Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the MenuItemButton widget.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Oct 2, 2025
Copy link
Contributor

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

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 adds a regression test to ensure that MenuItemButton does not crash when rendered in a zero-sized area. The new test case wraps a MenuItemButton within a SizedBox.shrink() to simulate a zero-area environment and asserts that the widget's size is indeed zero, ensuring that no crash occurs under these conditions. The test is well-written and accurately covers the reported issue. I have one minor suggestion to improve code style.

testWidgets('MenuItemButton does not crash at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(child: SizedBox.shrink(child: MenuItemButton())),
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better performance and adherence to Dart best practices, it's recommended to use const for widgets that can be constant. The Center widget and its children can be declared as const here.1

Suggested change
home: Center(child: SizedBox.shrink(child: MenuItemButton())),
home: const Center(child: SizedBox.shrink(child: MenuItemButton())),

Style Guide References

Footnotes

  1. The Flutter style guide recommends following 'Effective Dart: Style', which prefers using const for constructors where possible to improve performance. (link)

Copy link
Contributor

Choose a reason for hiding this comment

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

The MaterialApp is already const.

testWidgets('MenuItemButton does not crash at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(child: SizedBox.shrink(child: MenuItemButton())),
Copy link
Contributor

Choose a reason for hiding this comment

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

The MaterialApp is already const.

@victorsanni victorsanni requested a review from dkwingsmt October 3, 2025 18:28
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Checklist:

  • The test is in the correct file
  • The test name goes “does not crash at zero area”
  • The target widget is wrapped by Center (or is fullscreen)
  • The target widget does not have an overlay, or the overlay is tested
  • The target widget is expected to have a size of exactly Size.zero

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 8, 2025

autosubmit label was removed for flutter/flutter/176419, because - The status or check suite Linux_android_emu native_assets_android has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2025
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 9, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 9, 2025

autosubmit label was removed for flutter/flutter/176419, because - The status or check suite Linux_android_emu native_assets_android has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 9, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 9, 2025

autosubmit label was removed for flutter/flutter/176419, because Pull request flutter/flutter/176419 is not in a mergeable state.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 9, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Oct 9, 2025
Merged via the queue into flutter:master with commit 2949612 Oct 9, 2025
77 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 15, 2025
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…tter#176419)

This is my attempt to handle
flutter#6537 for the MenuItemButton
widget.

Co-authored-by: Tong Mu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants