fix MenuItemButton if child is null #147485

zeqinjie · 2024-04-28T06:58:14Z

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

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

mahmoudsaleh1997 · 2024-04-28T16:45:54Z

packages/flutter/lib/src/material/menu_anchor.dart

@@ -1141,7 +1141,7 @@ class _MenuItemButtonState extends State<MenuItemButton> {
        trailingIcon: widget.trailingIcon,
        hasSubmenu: false,
        overflowAxis: _anchor?._orientation ?? widget.overflowAxis,
-        child: widget.child!,
+        child: widget.child ?? const SizedBox(),


Using const SizedBox.shrink(); Would be better.
See https://api.flutter.dev/flutter/widgets/Visibility-class.html

@mahmoudsaleh1997 Agreed, thank you

nate-thegrate

Thanks a bunch for the contribution! Looking forward to getting this bug fixed.

nate-thegrate · 2024-05-02T21:42:01Z

packages/flutter/lib/src/material/menu_anchor.dart

-        child: widget.child!,
+        child: widget.child ?? const SizedBox.shrink(),


Rather than just a one-line change here, would you be able to update _MenuItemLabel so it has a nullable child property? Currently it's still documented as being required:

const _MenuItemLabel({ required this.hasSubmenu, this.showDecoration = true, this.leadingIcon, this.trailingIcon, this.shortcut, this.semanticsLabel, this.overflowAxis = Axis.vertical, required this.child, }); /// The required label child widget. final Widget child;

@nate-thegrate Thank you for your review，i have updated~

nate-thegrate · 2024-05-02T21:49:04Z

packages/flutter/test/material/menu_anchor_test.dart

@@ -2532,6 +2532,24 @@ void main() {
          ..rect(color: overlayColor.withOpacity(0.1)),
      );
    });
+
+    testWidgets('MenuItemButton is null check operator when it child is null', (WidgetTester tester) async {


Suggested change

testWidgets('MenuItemButton is null check operator when it child is null', (WidgetTester tester) async {

testWidgets('MenuItemButton can build when its child is null', (WidgetTester tester) async {

The rest of this test looks fantastic!

@nate-thegrate Thank you for your review，i got it~

nate-thegrate

LGTM 👍

Fantastic work here, thank you!

gspencergoog

packages/flutter/test/material/menu_anchor_test.dart

flutter/flutter@04424e1...7920a52 2024-05-07 sokolovskyi.konstantin@gmail.com Add tests for callback_shortcuts.0.dart API example. (flutter/flutter#147536) 2024-05-07 barpac02@gmail.com Improve Android SDK and NDK mistmatch warning message (flutter/flutter#147809) 2024-05-07 sokolovskyi.konstantin@gmail.com Add tests for shortcuts.dart API examples. (flutter/flutter#147433) 2024-05-07 sosamaalirizvi@gmail.com Added missing tests for ButtonStyle example (flutter/flutter#147457) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from ece1d686e3ba to 150f694a7816 (1 revision) (flutter/flutter#147912) 2024-05-07 leroux_bruno@yahoo.fr DropdownMenu cleanup (flutter/flutter#147860) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 80470584e1e1 to ece1d686e3ba (1 revision) (flutter/flutter#147910) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from b7bfd94af743 to 80470584e1e1 (1 revision) (flutter/flutter#147906) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from b64e2300bcd0 to b7bfd94af743 (1 revision) (flutter/flutter#147905) 2024-05-07 zeqinjie@qq.com fix MenuItemButton if child is null (flutter/flutter#147485) 2024-05-07 gspencergoog@users.noreply.github.com Fix document generation, eliminate template support from snippets tool. (flutter/flutter#147893) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4cb9e02c06ce to b64e2300bcd0 (1 revision) (flutter/flutter#147904) 2024-05-07 82763757+NobodyForNothing@users.noreply.github.com test focus example 0 (flutter/flutter#147564) 2024-05-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 422f92b992c5 to 4cb9e02c06ce (1 revision) (flutter/flutter#147899) 2024-05-06 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#147896) 2024-05-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from 463ff7d2d4d5 to 422f92b992c5 (5 revisions) (flutter/flutter#147895) 2024-05-06 59215665+davidhicks980@users.noreply.github.com MultiSelectableSelectionContainerDelegate documentation fixes. (flutter/flutter#147843) 2024-05-06 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#147891) 2024-05-06 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.4 to 4.1.5 (flutter/flutter#147888) 2024-05-06 32538273+ValentinVignal@users.noreply.github.com Fix janks and memory leaks in `CupertinoPageTransition` and `CupertinoFullscreenDialogTransition` (flutter/flutter#146999) 2024-05-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from 960a0c8fecbe to 463ff7d2d4d5 (1 revision) (flutter/flutter#147886) 2024-05-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from 70cc300f9ad8 to 960a0c8fecbe (1 revision) (flutter/flutter#147884) 2024-05-06 jonahwilliams@google.com [new gallery] Reisze gallery images (flutter/flutter#147882) 2024-05-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from d0f99b35eac6 to 70cc300f9ad8 (1 revision) (flutter/flutter#147880) 2024-05-06 polinach@google.com Fix leak in a test. (flutter/flutter#147846) 2024-05-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from a30ae7729c95 to d0f99b35eac6 (3 revisions) (flutter/flutter#147878) 2024-05-06 engine-flutter-autoroll@skia.org Roll Packages from f4719ca to 2dfe645 (3 revisions) (flutter/flutter#147866) 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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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

fix 147479

9c8f038

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Apr 28, 2024

zeqinjie added 2 commits April 28, 2024 15:04

update

52ef226

Merge branch 'flutter:master' into fix_147479

68d19f6

zeqinjie changed the title ~~fix 147479~~ fix MenuItemButton if child is null Apr 28, 2024

mahmoudsaleh1997 reviewed Apr 28, 2024

View reviewed changes

update

d895475

nate-thegrate reviewed May 2, 2024

View reviewed changes

zeqinjie added 2 commits May 3, 2024 09:30

Merge branch 'master' into fix_147479

12200dc

update _MenuItemLabel child required to optional

5bb8e98

nate-thegrate approved these changes May 3, 2024

View reviewed changes

Merge branch 'master' into fix_147479

e40640c

nate-thegrate requested a review from gspencergoog May 3, 2024 21:46

gspencergoog approved these changes May 7, 2024

View reviewed changes

packages/flutter/test/material/menu_anchor_test.dart Show resolved Hide resolved

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024

auto-submit bot merged commit 23ae246 into flutter:master May 7, 2024
72 checks passed

zeqinjie deleted the fix_147479 branch May 7, 2024 07:13

engine-flutter-autoroll mentioned this pull request May 7, 2024

Roll Flutter from 04424e10fe28 to 28e65b781105 (25 revisions) flutter/packages#6681

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2024

23ae246a2 fix MenuItemButton if child is null (flutter/flutter#147485)

a81b000

engine-flutter-autoroll mentioned this pull request May 7, 2024

Roll Flutter from 04424e10fe28 to 7920a52dd568 (27 revisions) flutter/packages#6683

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2024

23ae246a2 fix MenuItemButton if child is null (flutter/flutter#147485)

82e8ccb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix MenuItemButton if child is null #147485

fix MenuItemButton if child is null #147485

zeqinjie commented Apr 28, 2024 •

edited

mahmoudsaleh1997 Apr 28, 2024

zeqinjie Apr 29, 2024

nate-thegrate left a comment

nate-thegrate May 2, 2024

zeqinjie May 3, 2024

nate-thegrate May 2, 2024

zeqinjie May 3, 2024

nate-thegrate left a comment

gspencergoog left a comment

		child: widget.child!,
		child: widget.child ?? const SizedBox.shrink(),

	testWidgets('MenuItemButton is null check operator when it child is null', (WidgetTester tester) async {
	testWidgets('MenuItemButton can build when its child is null', (WidgetTester tester) async {

fix MenuItemButton if child is null #147485

fix MenuItemButton if child is null #147485

Conversation

zeqinjie commented Apr 28, 2024 • edited

mahmoudsaleh1997 Apr 28, 2024

Choose a reason for hiding this comment

zeqinjie Apr 29, 2024

Choose a reason for hiding this comment

nate-thegrate left a comment

Choose a reason for hiding this comment

nate-thegrate May 2, 2024

Choose a reason for hiding this comment

zeqinjie May 3, 2024

Choose a reason for hiding this comment

nate-thegrate May 2, 2024

Choose a reason for hiding this comment

zeqinjie May 3, 2024

Choose a reason for hiding this comment

nate-thegrate left a comment

Choose a reason for hiding this comment

gspencergoog left a comment

Choose a reason for hiding this comment

zeqinjie commented Apr 28, 2024 •

edited