Skip to content

Conversation

@rydmike
Copy link
Contributor

@rydmike rydmike commented Mar 23, 2023

NavigationDrawer indicator overlay does not use shape

If the NavigationDrawer shape is modified via indicatorShape by widget or theme, from its default M3 StadiumBorder shape, the indicator correctly gets the new shape, but InkWell used by overlay color for focus, hover and pressed state does not use the specified shape.

Before

Used in Drawer Used on main canvas
Screenshot 2023-03-23 at 12 03 19 Screenshot 2023-03-23 at 12 02 59

After

Used in Drawer Used on main canvas
Screenshot 2023-03-23 at 12 23 38 Screenshot 2023-03-23 at 12 23 23

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.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Mar 23, 2023
@rydmike rydmike changed the title FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape FIX: NavigationDrawer hover/focus/pressed do not use indicatorShape Mar 23, 2023
@TahaTesser TahaTesser self-requested a review March 23, 2023 11:31
Copy link
Member

@TahaTesser TahaTesser left a comment

Choose a reason for hiding this comment

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

LGTM!

@TahaTesser
Copy link
Member

Need a second review from @HansMuller

@TahaTesser TahaTesser requested a review from HansMuller March 23, 2023 14:45
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

highlightColor: Colors.transparent,
onTap: info.onTap,
borderRadius: const BorderRadius.all(Radius.circular(28.0)),
customBorder: info.indicatorShape ?? navigationDrawerTheme.indicatorShape ?? defaults.indicatorShape!,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, we really appear to have dropped the ball here. Nice fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @HansMuller, and naah probably on purpose, just to see if I'm awake enough to notice, and give me an easy fix 😃

@TahaTesser TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 23, 2023
@auto-submit auto-submit bot merged commit 9dec4fb into flutter:master Mar 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023
tarrinneal pushed a commit to flutter/packages that referenced this pull request Mar 24, 2023
* cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330)

* 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772)

* 5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

* b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282)

* af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336)

* 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325)

* 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340)

* 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342)

* 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141)

* 7b7af9f roll packages (flutter/flutter#123339)

* 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765)

* 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276)

* fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246)

* 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347)

* 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284)

* 100cf21 Prefer enum over class. (flutter/flutter#123312)

* 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474)

* d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
)

* cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330)

* 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772)

* 5a36bdd Stop serving Observatory by default (flutter/flutter#122419)

* b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282)

* af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336)

* 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325)

* 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340)

* 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342)

* 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141)

* 7b7af9f roll packages (flutter/flutter#123339)

* 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765)

* 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276)

* fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246)

* 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347)

* 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284)

* 100cf21 Prefer enum over class. (flutter/flutter#123312)

* 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474)

* d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)
@rydmike rydmike deleted the navigation_drawer_indicator_shape branch November 16, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App 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.

[Material3] NavigationDrawer indicator overlay does not use shape

3 participants