Fix NavigatorBar lacks visual feedback (#175182)
## Description
This PR fixes NavigationBar lacking visual feedback on the active
destination indicator.
This is a reland of https://github.com/flutter/flutter/pull/164484 which
was reverted in https://github.com/flutter/flutter/pull/169497.
After investigation, I narrowed down the regression introduced in
https://github.com/flutter/flutter/pull/164484 to the usage of a
RepaintBoundary.
I added one test to verify that the regression which led to the revert
of https://github.com/flutter/flutter/pull/164484 is no more
reproducible.
### 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](https://github.com/flutter/flutter/issues/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.
- Add one test to check that active destination moves to the correct
destination (that was not the case after
https://github.com/flutter/flutter/pull/164484). This new test is a
golden test.
diff --git a/packages/flutter/lib/src/material/navigation_bar.dart b/packages/flutter/lib/src/material/navigation_bar.dart
index 213b088..6499dc6 100644
--- a/packages/flutter/lib/src/material/navigation_bar.dart
+++ b/packages/flutter/lib/src/material/navigation_bar.dart
@@ -16,6 +16,7 @@
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';
@@ -866,7 +867,7 @@
builder: (BuildContext context, Animation<double> fadeAnimation) {
return FadeTransition(
opacity: fadeAnimation,
- child: Container(
+ child: Ink(
width: width,
height: height,
decoration: ShapeDecoration(
@@ -916,8 +917,6 @@
/// See [NavigationDestination.label].
final Widget label;
- static final Key _labelKey = UniqueKey();
-
@override
Widget build(BuildContext context) {
return _DestinationLayoutAnimationBuilder(
@@ -927,15 +926,12 @@
children: <Widget>[
LayoutId(
id: _NavigationDestinationLayoutDelegate.iconId,
- child: RepaintBoundary(key: iconKey, child: icon),
+ // The key is used by the _IndicatorInkWell to query the icon position.
+ child: KeyedSubtree(key: iconKey, child: icon),
),
LayoutId(
id: _NavigationDestinationLayoutDelegate.labelId,
- child: FadeTransition(
- alwaysIncludeSemantics: true,
- opacity: animation,
- child: RepaintBoundary(key: _labelKey, child: label),
- ),
+ child: FadeTransition(alwaysIncludeSemantics: true, opacity: animation, child: label),
),
],
);
diff --git a/packages/flutter/test/material/navigation_bar_test.dart b/packages/flutter/test/material/navigation_bar_test.dart
index 67d1e2f..dcbbdf6 100644
--- a/packages/flutter/test/material/navigation_bar_test.dart
+++ b/packages/flutter/test/material/navigation_bar_test.dart
@@ -953,6 +953,47 @@
);
});
+ // Regression test for https://github.com/flutter/flutter/issues/169249.
+ testWidgets('Material3 - Navigation indicator moves to selected item', (
+ WidgetTester tester,
+ ) async {
+ final ThemeData theme = ThemeData();
+ int index = 0;
+
+ Widget buildNavigationBar({Color? indicatorColor, ShapeBorder? indicatorShape}) {
+ return MaterialApp(
+ theme: theme,
+ home: Scaffold(
+ bottomNavigationBar: RepaintBoundary(
+ child: NavigationBar(
+ indicatorColor: indicatorColor,
+ indicatorShape: indicatorShape,
+ selectedIndex: index,
+ destinations: const <Widget>[
+ NavigationDestination(icon: Icon(Icons.ac_unit), label: 'AC'),
+ NavigationDestination(icon: Icon(Icons.access_alarm), label: 'Alarm'),
+ ],
+ onDestinationSelected: (int i) {},
+ ),
+ ),
+ ),
+ );
+ }
+
+ await tester.pumpWidget(buildNavigationBar());
+
+ // Move the selection to the second destination.
+ index = 1;
+ await tester.pumpWidget(buildNavigationBar());
+ await tester.pumpAndSettle();
+
+ // The navigation indicator should be on the second item.
+ await expectLater(
+ find.byType(NavigationBar),
+ matchesGoldenFile('m3.navigation_bar.indicator.ink.position.png'),
+ );
+ });
+
testWidgets('Navigation indicator scale transform', (WidgetTester tester) async {
int selectedIndex = 0;
@@ -1718,8 +1759,8 @@
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
index e932e7d..8506b13 100644
--- a/packages/flutter/test/material/navigation_bar_theme_test.dart
+++ b/packages/flutter/test/material/navigation_bar_theme_test.dart
@@ -365,8 +365,8 @@
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
index 96ca683..585c87f 100644
--- a/packages/flutter/test/material/navigation_drawer_test.dart
+++ b/packages/flutter/test/material/navigation_drawer_test.dart
@@ -123,14 +123,15 @@
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));
final BoxDecoration destinationDecoration =
tester.firstWidget<Ink>(findDestinationInk('Alarm')).decoration! as BoxDecoration;
expect(destinationDecoration.color, color);
@@ -549,8 +550,8 @@
ShapeDecoration? _getIndicatorDecoration(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?;
diff --git a/packages/flutter/test/material/navigation_drawer_theme_test.dart b/packages/flutter/test/material/navigation_drawer_theme_test.dart
index 3dcf1a9..b81d82c 100644
--- a/packages/flutter/test/material/navigation_drawer_theme_test.dart
+++ b/packages/flutter/test/material/navigation_drawer_theme_test.dart
@@ -283,8 +283,8 @@
ShapeDecoration? _getIndicatorDecoration(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?;
diff --git a/packages/flutter/test/material/navigation_rail_test.dart b/packages/flutter/test/material/navigation_rail_test.dart
index 8405893..faf488b 100644
--- a/packages/flutter/test/material/navigation_rail_test.dart
+++ b/packages/flutter/test/material/navigation_rail_test.dart
@@ -3123,7 +3123,7 @@
)
..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),
),
);
@@ -3185,7 +3185,7 @@
)
..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),
),
);
@@ -3251,7 +3251,7 @@
)
..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),
),
);
@@ -3316,7 +3316,7 @@
)
..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),
),
);
@@ -3383,7 +3383,7 @@
)
..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),
),
);
@@ -3423,13 +3423,11 @@
);
// 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;
@@ -3440,13 +3438,7 @@
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,
@@ -3470,10 +3462,10 @@
..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),
@@ -3526,9 +3518,6 @@
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;
@@ -3548,13 +3537,8 @@
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,
@@ -3633,7 +3617,6 @@
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.
@@ -3653,11 +3636,7 @@
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,
@@ -3683,9 +3662,9 @@
// 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),
),
@@ -6369,8 +6348,8 @@
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
index 52a055e..d8c96f0 100644
--- a/packages/flutter/test/material/navigation_rail_theme_test.dart
+++ b/packages/flutter/test/material/navigation_rail_theme_test.dart
@@ -324,8 +324,8 @@
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?;
