-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Make sure that a DropdownMenu doesn't crash in 0x0 environment #174809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sure that a DropdownMenu doesn't crash in 0x0 environment #174809
Conversation
There was a problem hiding this 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 DropdownMenu does not crash when it is rendered in a zero-sized environment. The test correctly sets up the scenario and verifies the widget's size. I have suggested an improvement to make the test more robust by also checking for crashes when attempting to open the menu, as this could reveal further layout-related issues.
dkwingsmt
left a comment
There was a problem hiding this 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 is not a overlay
- Expect the target widget has a size of exactly
Size.zero
|
I've discussed with @LongCatIsLooong, who proposed that we could also set the view size to be zero. testWidgets('DropdownMenu does not crash at zero area', (WidgetTester tester) async {
tester.view.physicalSize = Size.zero;
final TextEditingController controller = TextEditingController(text: 'I');
addTearDown(controller.dispose);
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: DropdownMenu<TestMenu>(
dropdownMenuEntries: menuChildren,
controller: controller,
),
),
),
),
);
expect(tester.getSize(find.byType(DropdownMenu<TestMenu>)), Size.zero);
controller.selection = const TextSelection.collapsed(offset: 0);
await tester.pump();
expect(find.byType(MenuItemButton), findsWidgets);
});Notable points:
Either this way or the previous way (with |
|
If I understand correctly, it is meaningless to make any spatial interaction with a widget that occupies a zero area of the screen, and people cannot make any spatial interaction with the widget when starting up the framework in a 0x0 environment, as the original purpose of the issue had been stated in #6537 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Edited)
The idea is not to test whether a zero-size widget is interactable, but to verify that the content in the pop-up can also be rendered in a zero-size constraints, which matters because:
|
|
@dkwingsmt I see that you approved my changes. Do you want me to apply these changes: #174809 (comment)? Or what? |
|
@ahmedsameha1 Sorry I didn't meant to approve it. Yes, can you apply the change in the comment? |
dkwingsmt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
autosubmit label was removed for flutter/flutter/174809, because - The status or check suite Windows framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.
|
|
autosubmit label was removed for flutter/flutter/174809, because - The status or check suite Windows framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.
|
|
Hi @ahmedsameha1, can you resolve the merge conflicts? |
victorsanni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you ensure the view resets to it's normal default value after this test? Otherwise other tests will be run with a zero-sized view which can cause unrelated test failures.
e9bbe56 to
e8a9959
Compare
flutter/flutter@cb18290...4c91098 2025-10-26 [email protected] Roll ICU from ff35c4f9df23 to f27805b7d7d8 (1 revision) (flutter/flutter#177558) 2025-10-26 [email protected] Roll Skia from b85c7ae19092 to de52b3a7585a (1 revision) (flutter/flutter#177550) 2025-10-25 [email protected] Make sure that a DropdownMenu doesn't crash in 0x0 environment (flutter/flutter#174809) 2025-10-25 [email protected] Fix DropdownMenu filtering is broken (flutter/flutter#177450) 2025-10-25 [email protected] Roll Skia from f352da2d607f to b85c7ae19092 (1 revision) (flutter/flutter#177539) 2025-10-25 [email protected] Roll Skia from e91d238ce638 to f352da2d607f (4 revisions) (flutter/flutter#177525) 2025-10-24 [email protected] Add more docs to TextBaseline (flutter/flutter#177507) 2025-10-24 [email protected] Bump Templates To Correct Versions (flutter/flutter#177416) 2025-10-24 [email protected] [web] Use SkPathBuilder because SkPath is becoming immutable (flutter/flutter#177343) 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
…er#174809) This is my attempt to handle flutter#6537 for the DropdownMenu widget.
This is my attempt to handle #6537 for the DropdownMenu widget.