Fix NavigationBar indicator overlay color (#164484)

bleroux · web-flow · commit c6c518653c39 · 2025-03-13T08:02:36.000Z
## 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: https://github.com/user-attachments/assets/a1e67dee-4a38-4711-ba90-bdcd9bed3226 ### After: The navigation indicator color changes (slightly darker): https://github.com/user-attachments/assets/1b1cc335-2cf4-4c41-9c53-696537707c72 ## Related Issue Fixes [NavigationBar lacks visual feedback when focused or hovered](flutter/flutter#163871) ## 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).
diff --git a/packages/flutter/lib/src/material/navigation_bar.dart b/packages/flutter/lib/src/material/navigation_bar.dart
@@ -13,6 +13,7 @@ import 'package:flutter/widgets.dart';
 import 'color_scheme.dart';
 import 'colors.dart';
 import 'elevation_overlay.dart';
+import 'ink_decoration.dart';
 import 'ink_well.dart';
 import 'material.dart';
 import 'material_localizations.dart';
@@ -853,7 +854,7 @@ class NavigationIndicator extends StatelessWidget {
             builder: (BuildContext context, Animation<double> fadeAnimation) {
               return FadeTransition(
                 opacity: fadeAnimation,
-                child: Container(
+                child: Ink(
                   width: width,
                   height: height,
                   decoration: ShapeDecoration(
diff --git a/packages/flutter/test/material/navigation_bar_test.dart b/packages/flutter/test/material/navigation_bar_test.dart
@@ -1663,8 +1663,8 @@ Material _getMaterial(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
diff --git a/packages/flutter/test/material/navigation_bar_theme_test.dart b/packages/flutter/test/material/navigation_bar_theme_test.dart
@@ -368,8 +368,8 @@ Material _barMaterial(WidgetTester tester) {
 
 ShapeDecoration? _indicator(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
diff --git a/packages/flutter/test/material/navigation_drawer_test.dart b/packages/flutter/test/material/navigation_drawer_test.dart
@@ -123,14 +123,16 @@ void main() {
     await tester.tap(find.text('AC'));
     await tester.pump();
 
-    expect(findDestinationInk('AC'), findsNothing);
+    // When no background color is set, only the non-visible indicator Ink is expected.
+    expect(findDestinationInk('AC'), findsOne);
 
     // Destination with a custom background color.
     await tester.tap(find.byIcon(Icons.access_alarm));
     await tester.pump();
 
     // A Material is added with the custom color.
-    expect(findDestinationInk('Alarm'), findsOne);
+    expect(findDestinationInk('Alarm'), findsNWidgets(2));
+    // The drawer destination Ink is the first one, the second is the indicator's one.
     final BoxDecoration destinationDecoration =
         tester.firstWidget<Ink>(findDestinationInk('Alarm')).decoration! as BoxDecoration;
     expect(destinationDecoration.color, color);
@@ -506,8 +508,8 @@ InkWell? _getInkWell(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
diff --git a/packages/flutter/test/material/navigation_drawer_theme_test.dart b/packages/flutter/test/material/navigation_drawer_theme_test.dart
@@ -285,8 +285,8 @@ Material _getMaterial(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
diff --git a/packages/flutter/test/material/navigation_rail_test.dart b/packages/flutter/test/material/navigation_rail_test.dart
@@ -3081,6 +3081,7 @@ void main() {
     final RenderObject inkFeatures = tester.allRenderObjects.firstWhere(
       (RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures',
     );
+
     const Rect indicatorRect = Rect.fromLTRB(12.0, 0.0, 68.0, 32.0);
     const Rect includedRect = indicatorRect;
     final Rect excludedRect = includedRect.inflate(10);
@@ -3106,7 +3107,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(12.0, 72.0, 68.0, 104.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(12.0, 0.0, 68.0, 32.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3168,7 +3169,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(12.0, 58.0, 68.0, 90.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(12.0, 6.0, 68.0, 38.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3234,7 +3235,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(30.0, 96.0, 86.0, 128.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(30.0, 24.0, 86.0, 56.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3299,7 +3300,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(0.0, 58.0, 50.0, 90.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(0.0, 6.0, 50.0, 38.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3366,7 +3367,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(140.0, 96.0, 196.0, 128.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(140.0, 24.0, 196.0, 56.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3406,13 +3407,11 @@ void main() {
     );
 
     // Default values from M3 specification.
+    const double railMinWidth = 80.0;
     const double indicatorHeight = 32.0;
     const double destinationWidth = 72.0;
     const double destinationHorizontalPadding = 8.0;
     const double indicatorWidth = destinationWidth - 2 * destinationHorizontalPadding; // 56.0
-    const double verticalSpacer = 8.0;
-    const double verticalIconLabelSpacing = 4.0;
-    const double verticalDestinationSpacing = 12.0;
 
     // The navigation rail width is larger than default because of the first destination long label.
     final double railWidth = tester.getSize(find.byType(NavigationRail)).width;
@@ -3423,13 +3422,7 @@ void main() {
     final Rect indicatorRect = Rect.fromLTRB(indicatorLeft, 0.0, indicatorRight, indicatorHeight);
     final Rect includedRect = indicatorRect;
     final Rect excludedRect = includedRect.inflate(10);
-
-    // Compute the vertical position for the selected destination (the one with 'bookmark' icon).
-    const double labelHeight = 16; // fontSize is 12 and height is 1.3.
-    const double destinationHeight =
-        indicatorHeight + verticalIconLabelSpacing + labelHeight + verticalDestinationSpacing;
-    const double secondDestinationVerticalOffset = verticalSpacer + destinationHeight;
-    const double secondIndicatorVerticalOffset = secondDestinationVerticalOffset;
+    const double indicatorHorizontalPadding = (railMinWidth - indicatorWidth) / 2; // 12.0
 
     expect(
       inkFeatures,
@@ -3453,10 +3446,10 @@ void main() {
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
           rrect: RRect.fromLTRBR(
-            indicatorLeft,
-            secondIndicatorVerticalOffset,
-            indicatorRight,
-            secondIndicatorVerticalOffset + indicatorHeight,
+            indicatorHorizontalPadding,
+            0.0,
+            indicatorHorizontalPadding + indicatorWidth,
+            indicatorHeight,
             const Radius.circular(16),
           ),
           color: const Color(0xffe8def8),
@@ -3509,9 +3502,6 @@ void main() {
     const double destinationWidth = 72.0;
     const double destinationHorizontalPadding = 8.0;
     const double indicatorWidth = destinationWidth - 2 * destinationHorizontalPadding; // 56.0
-    const double verticalSpacer = 8.0;
-    const double verticalIconLabelSpacing = 4.0;
-    const double verticalDestinationSpacing = 12.0;
 
     // The navigation rail width is the default one because labels are short.
     final double railWidth = tester.getSize(find.byType(NavigationRail)).width;
@@ -3531,13 +3521,8 @@ void main() {
     final Rect includedRect = indicatorRect;
     final Rect excludedRect = includedRect.inflate(10);
 
-    // Compute the vertical position for the selected destination (the one with 'bookmark' icon).
-    const double labelHeight = 16; // fontSize is 12 and height is 1.3.
-    const double destinationHeight =
-        iconSize + verticalIconLabelSpacing + labelHeight + verticalDestinationSpacing;
-    const double secondDestinationVerticalOffset = verticalSpacer + destinationHeight;
-    const double indicatorOffset = (iconSize - indicatorHeight) / 2;
-    const double secondIndicatorVerticalOffset = secondDestinationVerticalOffset + indicatorOffset;
+    // Icon height is greater than indicator height so the indicator has a vertical offset.
+    const double secondIndicatorVerticalOffset = (iconSize - indicatorHeight) / 2;
 
     expect(
       inkFeatures,
@@ -3616,7 +3601,6 @@ void main() {
     const double destinationWidth = 72.0;
     const double destinationHorizontalPadding = 8.0;
     const double indicatorWidth = destinationWidth - 2 * destinationHorizontalPadding; // 56.0
-    const double verticalSpacer = 8.0;
     const double verticalDestinationSpacingM3 = 12.0;
 
     // The navigation rail width is the default one because labels are short.
@@ -3636,11 +3620,7 @@ void main() {
     final Rect excludedRect = includedRect.inflate(10);
 
     // Compute the vertical position for the selected destination (the one with 'bookmark' icon).
-    const double destinationHeight = indicatorHeight + verticalDestinationSpacingM3;
-    const double secondDestinationVerticalOffset = verticalSpacer + destinationHeight;
-    const double secondIndicatorVerticalOffset =
-        secondDestinationVerticalOffset + verticalDestinationSpacingM3 / 2;
-    const double secondDestinationHorizontalOffset = 800 - railMinExtendedWidth; // RTL.
+    const double secondIndicatorVerticalOffset = verticalDestinationSpacingM3 / 2;
 
     expect(
       inkFeatures,
@@ -3666,9 +3646,9 @@ void main() {
         // Indicator for the selected destination (the one with 'bookmark' icon).
         ..rrect(
           rrect: RRect.fromLTRBR(
-            secondDestinationHorizontalOffset + indicatorLeft,
+            indicatorLeft,
             secondIndicatorVerticalOffset,
-            secondDestinationHorizontalOffset + indicatorRight,
+            indicatorRight,
             secondIndicatorVerticalOffset + indicatorHeight,
             const Radius.circular(16),
           ),
@@ -6175,8 +6155,8 @@ Widget _buildWidget(Widget child, {bool useMaterial3 = true, bool isRTL = false}
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
diff --git a/packages/flutter/test/material/navigation_rail_theme_test.dart b/packages/flutter/test/material/navigation_rail_theme_test.dart
@@ -327,8 +327,8 @@ Material _railMaterial(WidgetTester tester) {
 
 ShapeDecoration? _indicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Container>(
-            find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Container)),
+          .firstWidget<Ink>(
+            find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Ink)),
           )
           .decoration
       as ShapeDecoration?;
