Make the barrier panel on the Material Drawer non-dismissible by romanejaquez · Pull Request #164810 · flutter/flutter

romanejaquez · 2025-03-07T19:56:17Z

This PR entails making the modal barrier behind the Material Drawer dismissible / non-dismissible via a Scaffold flag named "drawerBarrierDismissible", as in:

Scaffold(
   drawerBarrierDismissible: false, // defaults to true so it behaves as normal
   ....
)

This is so as to force the Drawer to be opened / closed programmatically using either:

Scaffold.of(context).openEndDrawer() / openDrawer()

or

Scaffold.of(Utils.adminNav.currentContext!).closeDrawer() / closeEndDrawer()

and the user not closing the Drawer upon tapping on the modal barrier.

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].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

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

…roperty named drawerDismissible

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

dkwingsmt · 2025-04-04T18:07:49Z

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

+  // If set to false, prevents the barrier behind the [Drawer] from being dismissed.
+  // Defaults to true. The value of this flag also gets inherited by the parent Scaffold,
+  // or can be overridden if a [DrawerController] is extended and this value set accordingly.
+  final bool drawerDismissible;


I wonder why we call it drawerDismissible here instead of simply dismissible, since this is already known as a DrawerController. (I know some other properties are also prefixed with "drawer", which also appears weird to me.)

I'd totally be open to call it dismissible - I was just trying to keep the existing convention which I also thought it was a bit redundant but didn't want to have to change the rest if there was a purpose for it.

Got it. Can you rename it to dismissible?

dkwingsmt · 2025-04-04T18:08:18Z

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

+  /// If false, and a [drawer] is specified, then the barrier behind the drawer should not
+  /// respond to a tap event and thus remain open. The default behavior is to close upon the
+  /// user tapping on the barrier.


Same doc style guide problem as above.

+1, this should be split into 2 paragraphs so that the first paragraph is a single sentence, per the styleguide.

dkwingsmt · 2025-04-04T18:15:39Z

dev/a11y_assessments/test/drawer_test.dart

    expect(findHeadingLevelOnes, findsOne);
  });
+
+  testWidgets('drawer is dismissible', (WidgetTester tester) async {


Unless it's intentional, test cases should reside in packages/flutter/test/material/drawer_test.dart, not a11y_assessments.

no, not intentional. I actually saw some of the other drawer tests there so I wanted to put them along with the rest, but these can easily be moved wherever is more appropriate.

Co-authored-by: Tong Mu <[email protected]>

…missible"; moved tests topackages/flutter/test/material/drawer_test.dart

dkwingsmt · 2025-04-17T17:31:06Z

Can you move the test to the unit test file? (packages/flutter/test/material/drawer_test.dart) Also there are some analyser issues.



   info • Missing a newline at the end of the file • dev/a11y_assessments/lib/use_cases/drawer.dart:80:2 • eol_at_end_of_file
   info • The value of the argument is redundant because it matches the default value • packages/flutter/test/material/drawer_test.dart:772:30 • avoid_redundant_argument_values
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:774:54 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:776:22 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:779:20 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:785:20 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:786:22 • prefer_const_constructors
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:794:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:823:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:826:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:830:5 • always_specify_types
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:830:32 • prefer_const_constructors
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:831:5 • always_specify_types
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:850:54 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:852:22 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:855:20 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:861:20 • prefer_const_constructors
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:862:22 • prefer_const_constructors
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:870:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:899:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:902:5 • always_specify_types
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:906:5 • always_specify_types
   info • Use 'const' with the constructor to improve performance • packages/flutter/test/material/drawer_test.dart:906:32 • prefer_const_constructors
   info • Missing type annotation • packages/flutter/test/material/drawer_test.dart:907:5 • always_specify_types

…ally_assessments; left the ones there as they were before

dkwingsmt · 2025-04-21T17:53:51Z

One analyzer issue missing

   info â�¢ Missing a newline at the end of the file â�¢ packages/flutter/test/material/drawer_test.dart:1005:2 â�¢ eol_at_end_of_file

dkwingsmt

LGTM

packages/flutter/test/material/drawer_test.dart

…omanejaquez/flutter into drawer_barrier_non_dismissible

justinmc

Just a few small things here, and it looks like the analyzer is failing. Thank you for the PR!

Is there an issue for this by the way?

justinmc · 2025-04-21T18:18:10Z

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

+  /// If false, and a [drawer] is specified, then the barrier behind the drawer should not
+  /// respond to a tap event and thus remain open. The default behavior is to close upon the
+  /// user tapping on the barrier.


+1, this should be split into 2 paragraphs so that the first paragraph is a single sentence, per the styleguide.

justinmc · 2025-04-21T18:18:51Z

packages/flutter/test/material/drawer_test.dart

+      ),
+    );
+
+    // check the flag is set at the Scaffold level


Nit: Capital letter at the start and period at the end, here and elsewhere.

Took care of the comments above. However for the comment on the scaffold.dart, for the first sentence I had to split it into 2 because it would exceed the max length on a comment line.
Fixed the punctuation and capitalization on the other comments - thank you for catching that.

justinmc · 2025-04-21T18:19:30Z

packages/flutter/test/material/drawer_test.dart

    });
  });
 }
+


Remove these newlines.

gotcha - addressed the newline issue on the drawer_test.dart, but for some reason the Linux analyzer keeps barking on the newline not being added.

Co-authored-by: Tong Mu <[email protected]>

romanejaquez · 2025-04-21T18:36:44Z

Just a few small things here, and it looks like the analyzer is failing. Thank you for the PR!

Is there an issue for this by the way?

dkwingsmt · 2025-04-23T18:23:12Z

The analyzer errors seem to be only format errors, which can be resolved by running dart format . under the packages/flutter directory.

romanejaquez · 2025-04-23T18:24:40Z

The analyzer errors seem to be only format errors, which can be resolved by running dart format . under the packages/flutter directory.

will do 👍

…omanejaquez/flutter into drawer_barrier_non_dismissible

justinmc · 2025-05-09T23:09:36Z

This is probably good to go but I will give it a closer re-review on Monday. Sorry for the delay.

justinmc

LGTM but I see a few things that you seem to have intended to change but didn't. See my comments below. Maybe you forgot to push a commit.

justinmc · 2025-05-12T20:02:56Z

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

    this.scrimColor,
    this.edgeDragWidth,
    this.enableOpenDragGesture = true,
+    this.drawerBarrierDismissible = true,


I like the name drawerBarrierDismissible and I don't think it's too long 👍

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

justinmc · 2025-05-12T20:06:11Z

packages/flutter/test/material/drawer_test.dart

+    final ScaffoldState state = tester.firstState(find.byType(Scaffold));
+    state.openEndDrawer();
+
+    await tester.pump();


This pump is still here.

justinmc · 2025-05-12T20:06:22Z

packages/flutter/test/material/drawer_test.dart

+    state.openEndDrawer();
+
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1));


This hasn't been replaced by pumpAndSettle (also below).

justinmc · 2025-05-12T20:06:33Z

packages/flutter/test/material/drawer_test.dart

+
+    expect(find.byType(Drawer), findsExactly(1));
+
+    // Tap on the modal barrier.


This hasn't been removed.

Co-authored-by: Justin McCandless <[email protected]>

romanejaquez · 2025-05-12T20:50:49Z

LGTM but I see a few things that you seem to have intended to change but didn't. See my comments below. Maybe you forgot to push a commit.

Thanks for the comments. I was missing one additional commit where those remaining changes were - also I left the previous tests there as-is as I didn't want to disrupt anything else other than the ones I added (since I saw a bunch of other pumps and such, and whoever added those must've added them for a reason).

…flutter/flutter#164810)

flutter/flutter@54de32d...336a7ec 2025-05-13 [email protected] Add assert for index parameter in IndexedStack. (flutter/flutter#167757) 2025-05-13 [email protected] Fixed Android Lint Errors (flutter/flutter#168613) 2025-05-13 [email protected] Tab bar theme and dialog theme documentation cleanup (flutter/flutter#168724) 2025-05-13 [email protected] Roll Fuchsia Linux SDK from 6vjKe0bfYDVaECqBL... to 6J81agNhuK4q255Jj... (flutter/flutter#168712) 2025-05-12 [email protected] Make the barrier panel on the Material Drawer non-dismissible (flutter/flutter#164810) 2025-05-12 [email protected] Roll ICU from c9fb4b3a6fb5 to b929596baebf (5 revisions) (flutter/flutter#168691) 2025-05-12 [email protected] Fix DropdownButtonFormField icon not vertically centered (flutter/flutter#163205) 2025-05-12 [email protected] Android home/end keyboard shortcut support (flutter/flutter#168184) 2025-05-12 [email protected] Update `TESTOWNERS` for Android PV tests (flutter/flutter#168694) 2025-05-12 [email protected] Roll Dart SDK from 0bea6379f654 to 645d04a7a964 (14 revisions) (flutter/flutter#168679) 2025-05-12 [email protected] [Impeller] libImpeller: Allow setting text decorations. (flutter/flutter#168408) 2025-05-12 [email protected] Error when trying to use old HC mode when HCPP is enabled (flutter/flutter#168027) 2025-05-12 [email protected] Nav bar back label is not clipped mid-transition (flutter/flutter#168194) 2025-05-12 [email protected] Add test for stack trace mapping and test expression eval tests using DDC library bundle format (flutter/flutter#168017) 2025-05-12 [email protected] Remove CupertinoSliverNavigationBar background box when large title is extended (flutter/flutter#168407) 2025-05-12 [email protected] Label platform view modes using the unified naming scheme (flutter/flutter#168670) 2025-05-12 [email protected] Roll Packages from 7814fab to 6a28ad9 (2 revisions) (flutter/flutter#168669) 2025-05-12 [email protected] Update `Engine-artifacts.md` to reflect the new `engine.version` verifier (flutter/flutter#168413) 2025-05-12 [email protected] Roll Skia from 9f9e1f37917e to c97451da059f (1 revision) (flutter/flutter#168671) 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] 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

talski · 2025-08-04T23:15:23Z

Should the "Esc" key still close the drawer?

I'm testing and I can't close it by clicking outside, but I think the "Esc" key shouldn't close the drawer either, but it currently can.

romanejaquez · 2025-08-05T00:01:23Z

Should the "Esc" key still close the drawer?

I'm testing and I can't close it by clicking outside, but I think the "Esc" key shouldn't close the drawer either, but it currently can.

You are correct - ESC shouldn't close it. I'll submit a PR to fix that. Thanks for flagging it!

dkwingsmt · 2025-08-05T00:02:30Z

Good call, and I agree! Feel free to let me review it.

romanejaquez · 2025-08-06T00:49:00Z

Good call, and I agree! Feel free to let me review it.

here's the PR for review! => #173263

…flutter/flutter#164810)

make the barrier panel on the Material Drawer non-dismissible via a p…

cbc5f5b

…roperty named drawerDismissible

romanejaquez requested a review from matanlurey as a code owner March 7, 2025 19:56

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 7, 2025

dkwingsmt self-requested a review March 19, 2025 18:21

dkwingsmt reviewed Apr 4, 2025

View reviewed changes

romanejaquez and others added 2 commits April 4, 2025 14:31

Update packages/flutter/lib/src/material/drawer.dart

80a5cfd

Co-authored-by: Tong Mu <[email protected]>

renamed DrawerController property "dismissible" instead of "drawerDis…

e108277

…missible"; moved tests topackages/flutter/test/material/drawer_test.dart

matanlurey removed their request for review April 9, 2025 18:56

Merge branch 'master' into drawer_barrier_non_dismissible

60e7a23

romanejaquez added 2 commits April 17, 2025 15:41

fixed analyzer issues with drawer tests; removed previous tests from …

bf9dd97

…ally_assessments; left the ones there as they were before

Merge branch 'master' into drawer_barrier_non_dismissible

6194cbc

dkwingsmt approved these changes Apr 21, 2025

View reviewed changes

packages/flutter/test/material/drawer_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/drawer_test.dart Outdated Show resolved Hide resolved

romanejaquez added 2 commits April 21, 2025 14:06

fixing one more analyzer issue

0e932f6

Merge branch 'drawer_barrier_non_dismissible' of https://github.com/r…

4fc0072

…omanejaquez/flutter into drawer_barrier_non_dismissible

justinmc reviewed Apr 21, 2025

View reviewed changes

romanejaquez and others added 3 commits April 21, 2025 14:35

addressed PR comments

8ce48c7

Update packages/flutter/test/material/drawer_test.dart

6cbecca

Co-authored-by: Tong Mu <[email protected]>

Update packages/flutter/test/material/drawer_test.dart

626a7fd

Co-authored-by: Tong Mu <[email protected]>

Merge branch 'master' into drawer_barrier_non_dismissible

31a86d7

romanejaquez added 2 commits April 23, 2025 16:03

ran dart format on drawer_test file to fix linux analyzer issues

7be3d63

Merge branch 'drawer_barrier_non_dismissible' of https://github.com/r…

7b18c89

…omanejaquez/flutter into drawer_barrier_non_dismissible

dkwingsmt requested a review from justinmc April 23, 2025 22:24

romanejaquez added 2 commits April 24, 2025 09:36

Merge branch 'master' into drawer_barrier_non_dismissible

f9566d9

Merge branch 'master' into drawer_barrier_non_dismissible

74166c3

romanejaquez added 2 commits May 4, 2025 12:51

applied review comments

c40f9aa

updated reference to drawerBarrierDismissible

3aeb46c

justinmc approved these changes May 12, 2025

View reviewed changes

romanejaquez and others added 2 commits May 12, 2025 16:21

Update packages/flutter/lib/src/material/scaffold.dart

b317431

Co-authored-by: Justin McCandless <[email protected]>

apply provided review comments on the tests

76cb624

dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2025

auto-submit bot added this pull request to the merge queue May 12, 2025

Merged via the queue into flutter:master with commit 5c80840 May 13, 2025
76 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 13, 2025

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

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

adf5e78

…flutter/flutter#164810)

engine-flutter-autoroll mentioned this pull request May 13, 2025

Roll Flutter from 54de32d6e159 to 358b07268828 (18 revisions) flutter/packages#9244

Closed

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

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

e02da09

…flutter/flutter#164810)

engine-flutter-autoroll mentioned this pull request May 13, 2025

Roll Flutter from 54de32d6e159 to 336a7ecf7b40 (19 revisions) flutter/packages#9246

Merged

Piinks mentioned this pull request May 16, 2025

Flutter Notable Commits - 2025 #161460

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

92eb297

…flutter/flutter#164810)

justinmc mentioned this pull request Aug 14, 2025

Drawer barrier non dismissible with the escape key #173263

Merged

9 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

7bc334f

…flutter/flutter#164810)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

f9ba92c

…flutter/flutter#164810)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

dffa183

…flutter/flutter#164810)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025

5c80840 Make the barrier panel on the Material Drawer non-dismissible (…

60b2236

…flutter/flutter#164810)


		expect(find.byType(Drawer), findsExactly(1));

		// Tap on the modal barrier.

Conversation

romanejaquez commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Apr 17, 2025

Uh oh!

dkwingsmt commented Apr 21, 2025

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanejaquez commented Apr 21, 2025

Uh oh!

dkwingsmt commented Apr 23, 2025

Uh oh!

romanejaquez commented Apr 23, 2025

Uh oh!

justinmc commented May 9, 2025

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanejaquez commented May 12, 2025

Uh oh!

Uh oh!

talski commented Aug 4, 2025

Uh oh!

romanejaquez commented Aug 5, 2025

Uh oh!

dkwingsmt commented Aug 5, 2025

Uh oh!

romanejaquez commented Aug 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

romanejaquez commented Mar 7, 2025 •

edited

Loading