Skip to content

[material_ui, cupertino_ui] Migrate several doc directives for demonstration#11975

Open
dkwingsmt wants to merge 3 commits into
flutter:mainfrom
dkwingsmt:migrate-doc-try
Open

[material_ui, cupertino_ui] Migrate several doc directives for demonstration#11975
dkwingsmt wants to merge 3 commits into
flutter:mainfrom
dkwingsmt:migrate-doc-try

Conversation

@dkwingsmt

@dkwingsmt dkwingsmt commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

This PR migrates several doc directives to reach agreement before I perform a mass search-and-replace throughout the codebase.

To be exact, this PR migrates one {@tool snippet} directive, one {@tool dartpad} directive, and two {@tool sample} directives (which are actually all sample usages throughout the codebase).

The TODO comments are left for features to be implemented by the Dart team or unit tests that do not block the initial release.

Result

The result looks like this:
image

and

image

Known issue 1: No max-height for code snippets

It is a known flaw that the code blocks do not have a max height. The API doc page will display full source code, which is harder to read.

Solve it requires extra CSS rules, which is included in a previous proposal dart-lang/dartdoc#4243.

Known issue 2: dart analyze does not recognize the example directive

Even though dartdoc has supported the example directive, dart analyze still does not recognize it, causing warnings in IDE and CI. A PR dart-lang/sdk#63646 has been opened to solve this. Before the fix is landed in (hopefully) the next Dart version, we'll have to ignore this rule.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 24, 2026
@github-actions github-actions Bot added triage-framework Should be looked at in framework triage p: material_ui labels Jun 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the documentation in menu_anchor.dart and carousel.dart by replacing old tool blocks with {@example} directives and adding several TODO comments. The review feedback highlights that using regular comments (//) instead of doc comments (///) within documentation blocks breaks their contiguity, which can detach the API documentation. Additionally, the reviewer notes that the {@example} paths should not start with a leading slash to prevent resolution failures.

Comment thread packages/cupertino_ui/lib/src/menu_anchor.dart
Comment thread packages/material_ui/lib/src/carousel.dart
Comment thread packages/material_ui/lib/src/carousel.dart
Comment thread packages/material_ui/lib/src/carousel.dart Outdated
Comment thread packages/material_ui/lib/src/carousel.dart
@Piinks Piinks changed the title Migrate several doc directives for demonstration [material_ui, cupertino_ui] Migrate several doc directives for demonstration Jun 24, 2026
Comment thread packages/cupertino_ui/lib/src/menu_anchor.dart Outdated
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 24, 2026
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 24, 2026

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as long as we have a way to know which examples need a live dartpad example in the future and which don't.

For the record here are how the 2 screenshots you showed above look today on the docs site. I think the new format looks good enough to me!

Image Image

// TODO(dkwingsmt): Replace the following block with a blue example container
// when it's supported. https://github.com/dart-lang/dartdoc/issues/4243
///
/// {@tool dartpad}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dartpads that you have removed appear to now be exactly the same as the samples that you removed (if I'm not overlooking something). Don't you need to distinguish the two so that we can add back the dartpads later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD p: material_ui triage-framework Should be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants