Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix NavigationBar indicator overlay color #164484

Merged
merged 2 commits into from
Mar 13, 2025
Merged

Fix NavigationBar indicator overlay color #164484

merged 2 commits into from
Mar 13, 2025
+35 −52

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Mar 3, 2025 •
edited
Loading

Description

This PR fixes NavigationBar lacking visual feedback on the active destination indicator.

Before:

The navigation indicator does not change color when hovered or focused:

NavigationBarIndicatorOverlayBefore.mp4

After:

The navigation indicator color changes (slightly darker):

NavigationBarIndicatorOverlayAfter.mp4

Related Issue

Fixes NavigationBar lacks visual feedback when focused or hovered

Tests

  • Updates several helper functions which are used by several tests.
  • Updates several test: adding an Ink widget changes the coordinates used in several tests because these coordinates are now relative to the Ink offset.
  • Will impact fours golden tests which were capturing the bad behavior (indicator color not changing when hovered or focused).

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 3, 2025
@bleroux bleroux force-pushed the fix_navigation_bar_indicator_overlay branch 3 times, most recently from d0de54c to 4a674ae Compare March 4, 2025 14:14
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #164484 at sha 4a674ae

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Mar 4, 2025
@bleroux bleroux force-pushed the fix_navigation_bar_indicator_overlay branch from 4a674ae to 3955195 Compare March 5, 2025 09:53
@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #164484 at sha 3955195

TahaTesser
TahaTesser approved these changes Mar 5, 2025
View reviewed changes
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! (To trigger Google testing)

@bleroux bleroux requested a review from justinmc March 10, 2025 07:47
justinmc
justinmc approved these changes Mar 12, 2025
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . The Google failures were all expected.

packages/flutter/lib/src/material/navigation_bar.dart
Comment on lines -856 to +857
child: Container(
child: Ink(
Copy link
Contributor

Choose a reason for hiding this comment

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

Simple fix 👍

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Mar 13, 2025
Merged via the queue into flutter:master with commit c6c5186 Mar 13, 2025
75 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
@bleroux bleroux deleted the fix_navigation_bar_indicator_overlay branch March 13, 2025 09:12
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
43cf5bc 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
db8ce11 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
575767e 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
dd339a0 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
2ff755e 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 13, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
5ce66ae 
…164484)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 13, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
22332c3 
…164484)
This was referenced Mar 13, 2025
Closed
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
6a3ad54 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
b0e7963 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
866c158 
…164484)
@Piinks Piinks mentioned this pull request Mar 14, 2025
Open
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
6f64693 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
0b9277a 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
3af88f6 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
524d7a4 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
e6e7ad6 
…164484)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 17, 2025
Open
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
cf2a34d 
…164484)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 20, 2025
@engine-flutter-autoroll
c6c518653 Fix NavigationBar indicator overlay color (flutter/flutter#…
8d2f442 
…164484)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinmc justinmc justinmc approved these changes

@TahaTesser TahaTesser TahaTesser approved these changes

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files
Projects
None yet
Milestone
No milestone
3 participants
@bleroux @justinmc @TahaTesser