[cupertino.dart] Implement CupertinoMenuAnchor and CupertinoMenuItem using RawMenuAnchor#182036
Conversation
github-actions
Bot
added
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces CupertinoMenuAnchor, CupertinoMenuItem, and CupertinoMenuDivider, implementing them with RawMenuAnchor. The changes include the core implementation, new examples, and corresponding tests. The documentation for MenuAnchor's onOpen and onClose callbacks has also been clarified. The implementation appears robust and well-documented. I've identified one issue in an example file where a setState call is missing, which would prevent the UI from updating as expected.
| children: <Widget>[ | ||
| CupertinoMenuAnchor( | ||
| onAnimationStatusChanged: (AnimationStatus status) { | ||
| _status = status; |
There was a problem hiding this comment.
The _status update should be wrapped in a setState call to ensure the UI updates correctly when the menu's animation status changes. Without it, the CupertinoButton's text, which depends on _status, will not change from 'Open Menu' to 'Close Menu' when the menu is opened.
| _status = status; | |
| setState(() => _status = status); |
There was a problem hiding this comment.
This is actually not needed because the builder already is called on status changes, but it's probably better to include since it's a good practice.
|
everything looks smooth, just one notice, the shadow is too dark, on the native side it's more subtle. |
|
@MaherSafadii On dark or light mode? On light mode they seem pretty close: |
|
Oh I see it on dark mode. I'll update |
I was actually referring to light mode, the example you replied with looks good, but the original video recording maybe had some darker shadow, I did't try dark mode. |
|
@MaherSafadii You're correct. I'll update the video when I get a chance. Impeller is causing some problems that I need to sort out... |
|
no problem, it's not that big of a deal. |
|
didn't realize this reland exist. I also opened on pr #182103, will close in favor of this one |
|
|
||
| void _completeSwipe() { | ||
| _position = null; | ||
| widget.onDistanceChanged(0); |
There was a problem hiding this comment.
This should check for is mount in case the cancel is receive after dispose, it is a foot gun to call callback after the widget state is disposed.
There was a problem hiding this comment.
if it is not logically possible, at least put an assert is mounted
There was a problem hiding this comment.
Good point. There shouldn't be any logical reasons not to. Here's the final change:
void _completeSwipe() {
_recognizer?.dispose();
_recognizer = null;
_position = null;
if (mounted) {
widget.onDistanceChanged(0);
}
}
|
@chunhtai @dkwingsmt I made a few additional changes. Every change is shown on this gist: https://gist.github.com/davidhicks980/e608917541daecf4e55738a783c684a6/revisions . The changes are: menu_anchor.dart:
menu_anchor_test.dart
|
|
Thanks for the diff gist.
This seems like a bug of bounded blur. I'll look into it. Meanwhile feel free to change |
|
@dkwingsmt I split the bounded removal into a separate branch. I'll wait for it to merge + chuntai's review |
…er from ImageFilterConfig.blur (#182195) Removed 'bounded: true' from ImageFilterConfig.blur until #182066 is resolved. This should unblock #182036. See comment: #182066 (comment) @dkwingsmt Before: <img width="600" alt="Before" src="https://github.com/user-attachments/assets/29573ca2-eef7-4443-947c-58a7d30d060d" /> After: <img width="600" alt="After" src="https://github.com/user-attachments/assets/abae388a-ff40-494e-8137-13083e05ea81" /> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. // Goldens will need to be updated. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
one thing I also noticed while trying the latest iteration in DartPad is that there isn't any gap between the menu and button, on the native side there's a small gap by default 
here's how the small gap looks like on native |
…er from ImageFilterConfig.blur (flutter#182195) Removed 'bounded: true' from ImageFilterConfig.blur until flutter#182066 is resolved. This should unblock flutter#182036. See comment: flutter#182066 (comment) @dkwingsmt Before: <img width="600" alt="Before" src="https://github.com/user-attachments/assets/29573ca2-eef7-4443-947c-58a7d30d060d" /> After: <img width="600" alt="After" src="https://github.com/user-attachments/assets/abae388a-ff40-494e-8137-13083e05ea81" /> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. // Goldens will need to be updated. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
| /// The following constant represents a division in text scale factor beyond which | ||
| /// we want to change how the menu is laid out. | ||
| /// | ||
| /// This explanation was ported from cupertino/dialog.dart. |
There was a problem hiding this comment.
should also these constant be defined elsewhere to be share across similar to _DynamicTypeStyle? maybe add todo and group all these together some how?
There was a problem hiding this comment.
Forgot to comment -- this is done
| sizeStyle: CupertinoButtonSize.small, | ||
| focusNode: _buttonFocusNode, | ||
| onPressed: () { | ||
| if (_animationStatus.isForwardOrCompleted) { |
There was a problem hiding this comment.
why uses _animationStatus instead of controller.isopen
There was a problem hiding this comment.
controller.isOpen corresponds to whether the overlay is open, meaning it is true when the animation is AnimationStatus.forward, AnimationStatus.completed, and AnimationStatus.reverse. If we just check controller.isOpen, we have to wait until the menu is AnimationStatus.dismissed before it can be opened again. isForwardOrCompleted allows the menu to toggle open while it is closing.
| sizeStyle: CupertinoButtonSize.medium, | ||
| focusNode: _buttonFocusNode, | ||
| onPressed: () { | ||
| if (_status.isForwardOrCompleted) { |
| } | ||
| }, | ||
| child: Text( | ||
| _status.isForwardOrCompleted ? 'Close Menu' : 'Open Menu', |
There was a problem hiding this comment.
are we guaranteed this build method will be called when status change?
There was a problem hiding this comment.
looks like it will based on the code, should document on the builder method
There was a problem hiding this comment.
We're calling setState within onAnimationStatusChanged. However, as you mentioned, this isn't actually necessary. I'm only doing it so that devs aren't confused.
| /// | ||
| /// If null, the [CupertinoMenuAnchor] will be the size that its parent | ||
| /// allocates for it. | ||
| final RawMenuAnchorChildBuilder? builder; |
There was a problem hiding this comment.
should also mention this is called when animation status changes
There was a problem hiding this comment.
I added
/// The [builder] will rebuild when the menu's [AnimationStatus] changes.Let me know if more detail is needed.
| /// direction. | ||
| /// | ||
| /// Defaults to null. | ||
| final CupertinoMenuAnimationStatusChangedCallback? onAnimationStatusChanged; |
There was a problem hiding this comment.
I personally feel this is a weird api, but looks like this is mirroring the material version's. So I guess it is fine.
Personally I think if we want to expose this, it should pass into the builder.
There was a problem hiding this comment.
I agree. It would have been a breaking change on the material builder, so I opted for a callback. The other benefit is that users can call setState from the callback, whereas passing the animation status directly to a builder would have prevented that.
There was a problem hiding this comment.
Oh, and I try to avoid builders with more than 3 parameters. It's not very common across the framework (at least from what I've read).
There was a problem hiding this comment.
I don't think we have guideline on the limit of builder parameters. In this case, If choosing between one more parameter in builder versus a new parameter in constructor. I think it is better to be in builder instead.
anyway this doesn't block the merge
There was a problem hiding this comment.
I agree that it feels less intrusive to add a builder parameter, but I do think the fact that the builder can't call setState in response to animation status changes could prove to be frustrating. For example, I don't think there would be a way for the user to change a menu item's appearance in response to the AnimationStatus changing:
CupertinoMenuAnchor(
onAnimationStatusChanged: (AnimationStatus status) {
setState(() {
_animationStatus = status;
});
},
childFocusNode: _buttonFocusNode,
menuChildren: <Widget>[
CupertinoMenuItem(
onPressed: () {},
// This wouldn't be possible if the animation status was set from the builder.
child: _animationStatus.isForwardOrCompleted
? const Text('Opening Item')
: const Text('Closing Item'),
),
],As a side note, you're right that there aren't any guidelines against adding more than 3 params. It's a personal preference of mine because positional parameters don't retain documentation or their names when hovered. This can make it frustrating to track down how to use each parameter in a callback with a large definition. The example I always think of is PageRouteBuilder.transitionsBuilder:
There was a problem hiding this comment.
thanks for the reply, yup this sounds good to me
|
|
||
| MenuController get _menuController => widget.controller ?? _internalMenuController!; | ||
| MenuController? _internalMenuController; | ||
| bool get isOpening => _animationStatus.isForwardOrCompleted; |
There was a problem hiding this comment.
this name is a bit confusing, maybe just isOpen.
IsOpening implies the animation is still ongoing
There was a problem hiding this comment.
What about isOpenOrOpening?
| bool _resolveHasLeading() { | ||
| return widget.menuChildren.any((Widget element) { | ||
| return switch (element) { | ||
| final CupertinoMenuEntry entry => entry.hasLeading(context), |
There was a problem hiding this comment.
what is the use case for passing context to this method? I think this should be the root of the menu? can there be Anchor scope above?
There was a problem hiding this comment.
It's for submenus. IIRC, submenus on iPadOS display their arrow aligned to the end of a nested menu item, because menus open to the end of their parent. I'm using a media query to test for this in my project, but passing context is the safest way to allow for customization imo.
@override
bool hasLeading(BuildContext context) {
return switch (_computeMenuWidth(context)) {
CupertinoMenuWidth.iOS || CupertinoMenuWidth.iOSAccessible => true,
CupertinoMenuWidth.iPadOS || CupertinoMenuWidth.iPadOSAccessible => false,
};
}
|
Just wanted to say, super grateful for your work on this @davidhicks980! I've been a user of https://pub.dev/packages/pull_down_button for a while, and your work here is a worthy successor to that and a great improvement in several dimensions, so I'm excited to see this merged soon. I appreciate you! 🥳 Thank you also @chunhtai for your thoughtful reviewing here! 🙏🏻 |
dkwingsmt
force-pushed
the
cupertino_menu_anchor_redemption
branch
from
7f1ddc5 to
cec3132
Compare
flutter-dashboard
Bot
removed
the
autosubmit
…oMenuItem using RawMenuAnchor (flutter/flutter#182036)
…oMenuItem using RawMenuAnchor (flutter/flutter#182036)
flutter/flutter@a0924c7...05e0ae0 2026-04-08 [email protected] Fix Android engine flags defaulting to true for malformed values (flutter/flutter#184631) 2026-04-08 [email protected] Try one more again (flutter/flutter#184767) 2026-04-08 [email protected] Remove custom `analysis_options.yaml` from `imitation_game_flutter` (flutter/flutter#184717) 2026-04-08 [email protected] Add more error handling to unawaited callsites (flutter/flutter#184526) 2026-04-08 [email protected] Refactor: remove material from absorb_ponter_test, container_test, lookup_boundary_test, page_view_test, router_test, semantics_clipping_test, semantics_merge_test, shadow_test, text_test (flutter/flutter#183309) 2026-04-08 [email protected] Remove editable_text_utils cross-imports from material and cupertino … (flutter/flutter#184519) 2026-04-08 [email protected] Replace hard coded max path length with system defined one. (flutter/flutter#184697) 2026-04-08 [email protected] [Re-land] Add Support For Built-in Kotlin (flutter/flutter#184745) 2026-04-08 [email protected] Manually stop and continue LLDB breakpoints on Xcode 26.4+ (flutter/flutter#184690) 2026-04-08 [email protected] Code freeze workflow (flutter/flutter#184246) 2026-04-08 [email protected] [Dot shorthands] Migrate examples/api/lib/widgets (flutter/flutter#183965) 2026-04-08 [email protected] [cupertino.dart] Implement CupertinoMenuAnchor and CupertinoMenuItem using RawMenuAnchor (flutter/flutter#182036) 2026-04-08 [email protected] [Semantics] clarify Android header docs (flutter/flutter#183573) 2026-04-08 [email protected] [ci] mac build_test bringup false (flutter/flutter#184738) 2026-04-08 [email protected] Reland "Apply rect clipping to surface views" (flutter/flutter#184732) 2026-04-08 [email protected] Remove bringup label for resharded Windows tool_integration_tests shards (flutter/flutter#184721) 2026-04-08 [email protected] Tool: Add search and filtering to widget preview scaffold (flutter/flutter#184023) 2026-04-08 [email protected] Update localization from translation console (flutter/flutter#184742) 2026-04-07 [email protected] Revert "Add Support For Built-in Kotlin (#184227)" (flutter/flutter#184739) 2026-04-07 [email protected] Collect HCPP adoption analytics for flutter run/build apk/build appbundle (flutter/flutter#184225) 2026-04-07 [email protected] Add a github workflow for reverting PRs. (flutter/flutter#184593) 2026-04-07 [email protected] Add Support For Built-in Kotlin (flutter/flutter#184227) 2026-04-07 [email protected] Revert "Apply rect clipping to surface views (#184471)" (flutter/flutter#184728) 2026-04-07 [email protected] [Fix-forward] Added Compose plugin to Add-to-app Integration Test (flutter/flutter#184681) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…using RawMenuAnchor (flutter#182036) This PR implements `CupertinoMenuAnchor`, `CupertinoMenuItem`, and `CupertinoMenuDivider` using `RawMenuAnchor`. Resolves flutter#60298, notDmDrl/pull_down_button#26, flutter#137936. https://github.com/user-attachments/assets/cdaee8da-888b-4f64-8bf3-49c873f6d6e1 Dartpad: https://dartpad.dev/8c6bba779b6a00e95582b61b132292bc @dkwingsmt ---- Edit: Reland of flutter#174695 after revert flutter#182010 due to breakage when tests were randomized. Per @jason-simmons - > The error seen on CI can be reproduced by running: > flutter test --test-randomize-ordering-seed=20260206 test/cupertino/menu_anchor_test.dart Thanks to those who caught the error -- sorry I didn't notice. From what I can deduce, two errors were present. **Test 1: Menus do not close on root menu internal scroll (and others)** The first error occurred due to TestPointers being created with an existing pointer number. This was fixed by using tester.nextPointer instead of a fixed number. Code: https://github.com/davidhicks980/flutter/blob/054f3b03f2ab04822760c600e4259fbd828ebd4d/packages/flutter/test/cupertino/menu_anchor_test.dart#L968 ```dart final pointer = TestPointer( 1, // Fixed with tester.nextPointer ui.PointerDeviceKind.mouse ); await tester.sendEventToBinding(pointer.hover(tester.getCenter(find.text(Tag.a.text)))); await tester.pump(); ``` **Test 2: `Menu scale rebounds to full size when swipe gesture ends`** The second error occurred due to the _SwipeRegion's GestureRecognizer outliving its state and calling didSwipeLeave on a disposed CupertinoMenuItem. <s>This shouldn't be a problem -- swiping is supposed to continue so long as a user has their pointer down -- but the CupertinoMenuItem should have checked whether it was mounted before entering code that could schedule a rebuild (e.g. _handleActivation, isSwiped)</s> Nevermind -- I realize now there isn't much of a reason for letting the swipe outlive the region, so I just disposed of the swipe when the SwipeRegion is unmounted. I still kept the mounted check. Code: https://github.com/davidhicks980/flutter/blob/054f3b03f2ab04822760c600e4259fbd828ebd4d/packages/flutter/test/cupertino/menu_anchor_test.dart#L1285 ```dart @OverRide void didSwipeLeave({bool pointerUp = false}) { // This function could be called after disposal, but _handleActivation and isSwiped both could trigger rebuilds. // Fix: // if (!mounted) { // return // } if (isEnabled && pointerUp) { _handleActivation(); } isSwiped = false; } ``` I ran an additional 500 randomized menu_anchor_test.dart runs and did not experience another exception. I believe everything should be good-to-go, but I'll pay closer attention to cocoon. --- ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
6 participants
This PR implements
CupertinoMenuAnchor,CupertinoMenuItem, andCupertinoMenuDividerusingRawMenuAnchor.Resolves #60298, notDmDrl/pull_down_button#26, #137936.
Screen.Recording.2026-02-08.at.6.32.01.PM.mov
Dartpad: https://dartpad.dev/8c6bba779b6a00e95582b61b132292bc
@dkwingsmt
Edit: Reland of #174695 after revert #182010 due to breakage when tests were randomized.
Per @jason-simmons -
Thanks to those who caught the error -- sorry I didn't notice.
From what I can deduce, two errors were present.
Test 1: Menus do not close on root menu internal scroll (and others)
The first error occurred due to TestPointers being created with an existing pointer number. This was fixed by using tester.nextPointer instead of a fixed number.
Code: https://github.com/davidhicks980/flutter/blob/054f3b03f2ab04822760c600e4259fbd828ebd4d/packages/flutter/test/cupertino/menu_anchor_test.dart#L968
Test 2:
Menu scale rebounds to full size when swipe gesture endsThe second error occurred due to the _SwipeRegion's GestureRecognizer outliving its state and calling didSwipeLeave on a disposed CupertinoMenuItem.
This shouldn't be a problem -- swiping is supposed to continue so long as a user has their pointer down -- but the CupertinoMenuItem should have checked whether it was mounted before entering code that could schedule a rebuild (e.g. _handleActivation, isSwiped)Nevermind -- I realize now there isn't much of a reason for letting the swipe outlive the region, so I just disposed of the swipe when the SwipeRegion is unmounted. I still kept the mounted check.Code:
https://github.com/davidhicks980/flutter/blob/054f3b03f2ab04822760c600e4259fbd828ebd4d/packages/flutter/test/cupertino/menu_anchor_test.dart#L1285
I ran an additional 500 randomized menu_anchor_test.dart runs and did not experience another exception. I believe everything should be good-to-go, but I'll pay closer attention to cocoon.
Pre-launch 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-assistbot 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.