Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the NavigationDrawer 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 13, 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 NavigationDrawer does not crash when rendered in a zero-sized area. The test itself is a good idea, but in its current form, it's not effective because it doesn't include a NavigationDrawerDestination, which is necessary to trigger the code path that was causing the original crash. I've suggested a change to make the test correctly cover the intended scenario.

await tester.pumpWidget(
const MaterialApp(
home: Center(
child: SizedBox.shrink(child: NavigationDrawer(children: <Widget>[])),
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This test is intended to prevent a crash in NavigationDrawer when it has zero area. The original crash, as described in issue #6537, occurred within the NavigationIndicator widget. This widget is only built when there's at least one NavigationDrawerDestination in the children list of the NavigationDrawer.

By providing an empty list (children: <Widget>[]), this test doesn't exercise the code path that caused the crash, making it ineffective as a regression test. To make this test correctly cover the scenario, you should add at least one NavigationDrawerDestination to the children list.

Suggested change
child: SizedBox.shrink(child: NavigationDrawer(children: <Widget>[])),
child: SizedBox.shrink(child: NavigationDrawer(children: const <Widget>[const NavigationDrawerDestination(icon: Icon(Icons.inbox), label: Text('Inbox'))])),

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@victorsanni victorsanni requested a review from dkwingsmt October 20, 2025 21:54
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 20, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Oct 20, 2025
Merged via the queue into flutter:master with commit 92b1c87 Oct 21, 2025
78 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 21, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 21, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 21, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 21, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 22, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 22, 2025
Roll Flutter from 2d34167 to 75004a6 (39 revisions)

flutter/flutter@2d34167...75004a6

2025-10-22 jason-simmons@users.noreply.github.com [Impeller] Add the paint color to the key of the text shadow cache (flutter/flutter#177140)
2025-10-22 engine-flutter-autoroll@skia.org Roll Skia from 96e75ca8e24b to 928b5cf727c1 (2 revisions) (flutter/flutter#177387)
2025-10-22 jason-simmons@users.noreply.github.com Roll reclient to version 185 (flutter/flutter#177293)
2025-10-22 engine-flutter-autoroll@skia.org Roll Skia from b157f6b95f95 to 96e75ca8e24b (4 revisions) (flutter/flutter#177366)
2025-10-22 bruno.leroux@gmail.com Fix InputDatePickerFormField does not inherit local InputDecorationTheme (flutter/flutter#177090)
2025-10-22 engine-flutter-autoroll@skia.org Roll Skia from 2c6162c977db to b157f6b95f95 (2 revisions) (flutter/flutter#177362)
2025-10-22 engine-flutter-autoroll@skia.org Roll Skia from cadf8e7e6fca to 2c6162c977db (4 revisions) (flutter/flutter#177359)
2025-10-22 katelovett@google.com Cleanup after -news_toolkit, +google_fonts, and some leftover `team-go_router` (flutter/flutter#176841)
2025-10-21 eugene.makar@yandex.ru don't break sheet's snap from physics (flutter/flutter#171157)
2025-10-21 engine-flutter-autoroll@skia.org Roll Dart SDK from 913c2ae1b367 to c23010c4f9e6 (8 revisions) (flutter/flutter#177348)
2025-10-21 116356835+AbdeMohlbi@users.noreply.github.com Fix typo in comment about screen availibility  (flutter/flutter#177168)
2025-10-21 kazbek.sultanov.doc@gmail.com Fix(AnimatedScrollView): exclude outgoing items in removeAllItems (flutter/flutter#176452)
2025-10-21 jason-simmons@users.noreply.github.com Enable deprecated_member_use_from_same_package for all packages containing tests of Dart fixes defined within the package (flutter/flutter#177341)
2025-10-21 engine-flutter-autoroll@skia.org Roll Skia from 19bff385f7e8 to cadf8e7e6fca (3 revisions) (flutter/flutter#177331)
2025-10-21 jesswon@google.com Revert "[Android 16] Update `android_engine_vulkan_tests` to Test Against SDK 36 Emulator" (flutter/flutter#177292)
2025-10-21 108678139+manu-sncf@users.noreply.github.com Fix SliverMainAxisGroup.cacheOrigin (flutter/flutter#175760)
2025-10-21 engine-flutter-autoroll@skia.org Roll Skia from 75c756e029c9 to 19bff385f7e8 (3 revisions) (flutter/flutter#177316)
2025-10-21 59215665+davidhicks980@users.noreply.github.com Fix typo in overlay.dart documentation comment (flutter/flutter#176612)
2025-10-21 bkonyi@google.com [ Tool ] Output DTD URI for Flutter web applications (flutter/flutter#177310)
2025-10-21 engine-flutter-autoroll@skia.org Roll Skia from 982988b472a4 to 75c756e029c9 (1 revision) (flutter/flutter#177305)
2025-10-21 engine-flutter-autoroll@skia.org Roll Skia from 42ff13a91c80 to 982988b472a4 (8 revisions) (flutter/flutter#177300)
2025-10-21 bruno.leroux@gmail.com Fix DateRangePickerDialog does not inherit local InputDecorationTheme (flutter/flutter#177086)
2025-10-21 fishythefish@users.noreply.github.com Remove references to dart:_js_annotations (flutter/flutter#176698)
2025-10-20 31859944+LongCatIsLooong@users.noreply.github.com Make `FlutterSceneLifeCycleProvider.sceneLifeCycleDelegate` readonly (flutter/flutter#177240)
2025-10-20 ahmedsameha1@gmail.com Make sure that a CupertinoDesktopTextSelectionToolbar doesn't crash i… (flutter/flutter#173964)
2025-10-20 ahmedsameha1@gmail.com Make sure that a BottomSheet doesn't crash in 0x0 environment (flutter/flutter#172229)
2025-10-20 jason-simmons@users.noreply.github.com Move the Fuchsia SDK to //third_party/fuchsia-sdk (flutter/flutter#177118)
2025-10-20 engine-flutter-autoroll@skia.org Roll Skia from 641994569415 to 42ff13a91c80 (8 revisions) (flutter/flutter#177283)
2025-10-20 ahmedsameha1@gmail.com Make sure that a NavigationDrawer doesn't crash in 0x0 environment (flutter/flutter#176951)
2025-10-20 sokolovskyi.konstantin@gmail.com Fix ink features painting in TabBar. (flutter/flutter#177155)
2025-10-20 ahmedsameha1@gmail.com Make sure that SimpleDialog and SimpleDialogOption do not crash in 0x0 environment (flutter/flutter#174229)
2025-10-20 sokolovskyi.konstantin@gmail.com Fix ink features painting in YearPicker. (flutter/flutter#177014)
2025-10-20 116356835+AbdeMohlbi@users.noreply.github.com Update `image.error_builder.0.dart` to replace the emoji with some text  (flutter/flutter#176886)
2025-10-20 engine-flutter-autoroll@skia.org Roll Skia from ed4294faecde to 641994569415 (4 revisions) (flutter/flutter#177264)
2025-10-20 116356835+AbdeMohlbi@users.noreply.github.com Remove redundant name field form `TargetPlatform` and `XCDeviceEventInterface` enums (flutter/flutter#176890)
2025-10-20 55750689+AthulJoseph27@users.noreply.github.com Added support to pass in texture type while creating textures. (flutter/flutter#175376)
2025-10-20 engine-flutter-autoroll@skia.org Roll Packages from 3747006 to d113bbc (6 revisions) (flutter/flutter#177270)
2025-10-20 jason-simmons@users.noreply.github.com Roll Dart SDK from 2cd2106f2cef to 913c2ae1b367 (2 revisions) (flutter/flutter#177258)
2025-10-20 30870216+gaaclarke@users.noreply.github.com Added link to ClipRect from ImageFilter in the docstring (flutter/flutter#177196)

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 louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

...
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
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