PlatformMenuBar changes to bring it into line with upcoming MenuBar implementation #104565
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d5d7078 to
840addf
Compare
goderbauer
approved these changes
May 25, 2022
| @@ -383,7 +459,12 @@ class DefaultPlatformMenuDelegate extends PlatformMenuDelegate { | |||
| } | |||
| final MenuItem item = _idMap[id]!; | |||
| if (call.method == _kMenuSelectedCallbackMethod) { | |||
| assert(item.onSelected == null || item.onSelectedIntent == null, | |||
There was a problem hiding this comment.
Could we assert this earlier closer to the creation point of the MenuItem? Currently, you'd only see the assert if you actually selected the item in the menu
There was a problem hiding this comment.
Oh, I see below that this is also asserted earlier. Never mind this comment then.
There was a problem hiding this comment.
Yes, I did both because they're in separate classes. It's probably overkill.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 25, 2022
…enuBar implementation (flutter/flutter#104565)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/plugins
that referenced
this pull request
Jun 3, 2022
…enuBar implementation (flutter/flutter#104565)
camsim99
pushed a commit
to camsim99/flutter
that referenced
this pull request
Aug 10, 2022
…mplementation (flutter#104565) When I was doing the MenuBar implementation, I made some changes to the PlatformMenuBar to allow it to understand shortcuts a little more, and to deprecate the body parameter rename it to child to match most other widgets. These are those changes, separated out because they are separable, and I'm trying to make the MenuBar PR smaller.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 30, 2022
…enuBar implementation (flutter/flutter#104565)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/plugins
that referenced
this pull request
Aug 30, 2022
…enuBar implementation (flutter/flutter#104565)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When I was doing the
MenuBarimplementation, I made some changes to thePlatformMenuBarto allow it to understand shortcuts a little more, and to deprecate thebodyparameter rename it tochildto match most other widgets.These are those changes, separated out because they are separable, and I'm trying to make the
MenuBarPR smaller.