diff --git a/packages/flutter/lib/src/material/navigation_bar.dart b/packages/flutter/lib/src/material/navigation_bar.dart index 213b0888cfa..6499dc6a62a 100644 --- a/packages/flutter/lib/src/material/navigation_bar.dart +++ b/packages/flutter/lib/src/material/navigation_bar.dart @@ -16,6 +16,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'; @@ -866,7 +867,7 @@ class NavigationIndicator extends StatelessWidget { builder: (BuildContext context, Animation fadeAnimation) { return FadeTransition( opacity: fadeAnimation, - child: Container( + child: Ink( width: width, height: height, decoration: ShapeDecoration( @@ -916,8 +917,6 @@ class _NavigationBarDestinationLayout extends StatelessWidget { /// See [NavigationDestination.label]. final Widget label; - static final Key _labelKey = UniqueKey(); - @override Widget build(BuildContext context) { return _DestinationLayoutAnimationBuilder( @@ -927,15 +926,12 @@ class _NavigationBarDestinationLayout extends StatelessWidget { children: [ 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 67d1e2f307d..dcbbdf6d41a 100644 --- a/packages/flutter/test/material/navigation_bar_test.dart +++ b/packages/flutter/test/material/navigation_bar_test.dart @@ -953,6 +953,47 @@ void main() { ); }); + // 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 [ + 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 @@ Material _getMaterial(WidgetTester tester) { ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)), + .firstWidget( + 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 e932e7d0420..8506b130589 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 @@ Material _barMaterial(WidgetTester tester) { ShapeDecoration? _indicator(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)), + .firstWidget( + 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 96ca683a746..585c87f5dd8 100644 --- a/packages/flutter/test/material/navigation_drawer_test.dart +++ b/packages/flutter/test/material/navigation_drawer_test.dart @@ -123,14 +123,15 @@ 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)); final BoxDecoration destinationDecoration = tester.firstWidget(findDestinationInk('Alarm')).decoration! as BoxDecoration; expect(destinationDecoration.color, color); @@ -549,8 +550,8 @@ InkWell? _getInkWell(WidgetTester tester) { ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Container)), + .firstWidget( + 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 3dcf1a95640..b81d82c279a 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 @@ Material _getMaterial(WidgetTester tester) { ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Container)), + .firstWidget( + 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 84058931d83..faf488b5633 100644 --- a/packages/flutter/test/material/navigation_rail_test.dart +++ b/packages/flutter/test/material/navigation_rail_test.dart @@ -3123,7 +3123,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), ), ); @@ -3185,7 +3185,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), ), ); @@ -3251,7 +3251,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), ), ); @@ -3316,7 +3316,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), ), ); @@ -3383,7 +3383,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), ), ); @@ -3423,13 +3423,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; @@ -3440,13 +3438,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, @@ -3470,10 +3462,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), @@ -3526,9 +3518,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; @@ -3548,13 +3537,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, @@ -3633,7 +3617,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. @@ -3653,11 +3636,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, @@ -3683,9 +3662,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), ), @@ -6369,8 +6348,8 @@ Widget _buildWidget(Widget child, {bool useMaterial3 = true, bool isRTL = false} ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)), + .firstWidget( + 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 52a055e4f73..d8c96f0a827 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 @@ Material _railMaterial(WidgetTester tester) { ShapeDecoration? _indicatorDecoration(WidgetTester tester) { return tester - .firstWidget( - find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Container)), + .firstWidget( + find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Ink)), ) .decoration as ShapeDecoration?;