Fix NavigationBar indicator ripple doesn't account for label height (#117473)

packages/flutter/lib/src/material/navigation_bar.dart

@@ -462,26 +462,17 @@ class _NavigationDestinationBuilder extends StatelessWidget {
     final _NavigationDestinationInfo info = _NavigationDestinationInfo.of(context);
     final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context);
     final NavigationBarThemeData defaults = _defaultsFor(context);
+    final GlobalKey labelKey = GlobalKey();
 
     final bool selected = info.selectedIndex == info.index;
-    final double labelPadding;
-    switch (info.labelBehavior) {
-      case NavigationDestinationLabelBehavior.alwaysShow:
-        labelPadding = 8;
-        break;
-      case NavigationDestinationLabelBehavior.onlyShowSelected:
-        labelPadding = selected ? 8 : 0;
-        break;
-      case NavigationDestinationLabelBehavior.alwaysHide:
-        labelPadding = 0;
-        break;
-    }
     return _NavigationBarDestinationSemantics(
       child: _NavigationBarDestinationTooltip(
         message: tooltip ?? label,
         child: _IndicatorInkWell(
           key: UniqueKey(),
-          labelPadding: labelPadding,
+          labelKey: labelKey,
+          labelBehavior: info.labelBehavior,
+          selected: selected,
           customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape,
           onTap: info.onTap,
           child: Row(
@@ -489,6 +480,7 @@ class _NavigationDestinationBuilder extends StatelessWidget {
               Expanded(
                 child: _NavigationBarDestinationLayout(
                   icon: buildIcon(context),
+                  labelKey: labelKey,
                   label: buildLabel(context),
                 ),
               ),
@@ -503,7 +495,9 @@ class _NavigationDestinationBuilder extends StatelessWidget {
 class _IndicatorInkWell extends InkResponse {
   const _IndicatorInkWell({
     super.key,
-    required this.labelPadding,
+    required this.labelKey,
+    required this.labelBehavior,
+    required this.selected,
     super.customBorder,
     super.onTap,
     super.child,
@@ -512,10 +506,26 @@ class _IndicatorInkWell extends InkResponse {
     highlightColor: Colors.transparent,
   );
 
-  final double labelPadding;
+  final GlobalKey labelKey;
+  final NavigationDestinationLabelBehavior labelBehavior;
+  final bool selected;
 
   @override
   RectCallback? getRectCallback(RenderBox referenceBox) {
+    final RenderBox labelBox = labelKey.currentContext!.findRenderObject()! as RenderBox;
+    final Rect labelRect = labelBox.localToGlobal(Offset.zero) & labelBox.size;
+    final double labelPadding;
+    switch (labelBehavior) {
+      case NavigationDestinationLabelBehavior.alwaysShow:
+        labelPadding = labelRect.height / 2;
+        break;
+      case NavigationDestinationLabelBehavior.onlyShowSelected:
+        labelPadding = selected ? labelRect.height / 2 : 0;
+        break;
+      case NavigationDestinationLabelBehavior.alwaysHide:
+        labelPadding = 0;
+        break;
+    }
     final double indicatorOffsetX = referenceBox.size.width / 2;
     final double indicatorOffsetY = referenceBox.size.height / 2 - labelPadding;
 
@@ -765,6 +775,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
   /// 3 [NavigationBar].
   const _NavigationBarDestinationLayout({
     required this.icon,
+    required this.labelKey,
     required this.label,
   });
 
@@ -773,6 +784,11 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
   /// See [NavigationDestination.icon].
   final Widget icon;
 
+  /// The global key for the label of this destination.
+  ///
+  /// This is used to determine the position of the label relative to the icon.
+  final GlobalKey labelKey;
+
   /// The label widget that sits below the icon.
   ///
   /// This widget will sometimes be faded out, depending on
@@ -782,7 +798,6 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
   final Widget label;
 
   static final Key _iconKey = UniqueKey();
-  static final Key _labelKey = UniqueKey();
 
   @override
   Widget build(BuildContext context) {
@@ -806,7 +821,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
                 alwaysIncludeSemantics: true,
                 opacity: animation,
                 child: RepaintBoundary(
-                  key: _labelKey,
+                  key: labelKey,
                   child: label,
                 ),
               ),

packages/flutter/test/material/navigation_bar_test.dart

@@ -2,6 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This file is run as part of a reduced test set in CI on Mac and Windows
+// machines.
+@Tags(<String>['reduced-test-set'])
+library;
+
 import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
@@ -562,21 +567,26 @@ void main() {
     int selectedIndex = 0;
 
     Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
-      return _buildWidget(
+      return MaterialApp(
-        NavigationBar(
+        theme: ThemeData(useMaterial3: true),
-          selectedIndex: selectedIndex,
+        home: Scaffold(
-          labelBehavior: labelBehavior,
+          bottomNavigationBar: Center(
-          destinations: const <Widget>[
+            child: NavigationBar(
-            NavigationDestination(
+              selectedIndex: selectedIndex,
-              icon: Icon(Icons.ac_unit),
+              labelBehavior: labelBehavior,
-              label: 'AC',
+              destinations: const <Widget>[
+                NavigationDestination(
+                  icon: Icon(Icons.ac_unit),
+                  label: 'AC',
+                ),
+                NavigationDestination(
+                  icon: Icon(Icons.access_alarm),
+                  label: 'Alarm',
+                ),
+              ],
+              onDestinationSelected: (int i) { },
             ),
-            NavigationDestination(
+          ),
-              icon: Icon(Icons.access_alarm),
-              label: 'Alarm',
-            ),
-          ],
-          onDestinationSelected: (int i) { },
         ),
       );
     }
@@ -589,7 +599,7 @@ void main() {
     await tester.pumpAndSettle();
 
     final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
-    Offset indicatorCenter = const Offset(600, 32);
+    Offset indicatorCenter = const Offset(600, 30);
     const Size includedIndicatorSize = Size(64, 32);
     const Size excludedIndicatorSize = Size(74, 40);
 
@@ -715,7 +725,7 @@ void main() {
     selectedIndex = 1;
     await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
     await tester.pumpAndSettle();
-    indicatorCenter = const Offset(600, 32);
+    indicatorCenter = const Offset(600, 30);
 
     expect(
       inkFeatures,
@@ -753,6 +763,63 @@ void main() {
     );
   });
 
+  testWidgets('Navigation indicator ripple golden test', (WidgetTester tester) async {
+    // This is a regression test for https://github.com/flutter/flutter/issues/117420.
+
+    Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
+      return MaterialApp(
+        theme: ThemeData(useMaterial3: true),
+        home: Scaffold(
+          bottomNavigationBar: Center(
+            child: NavigationBar(
+              labelBehavior: labelBehavior,
+              destinations: const <Widget>[
+                NavigationDestination(
+                  icon: SizedBox(),
+                  label: 'AC',
+                ),
+                NavigationDestination(
+                  icon: SizedBox(),
+                  label: 'Alarm',
+                ),
+              ],
+              onDestinationSelected: (int i) { },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(buildWidget());
+
+    final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+    await gesture.addPointer();
+    await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+    await tester.pumpAndSettle();
+
+    // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysShow` (default).
+    await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_alwaysShow_m3.png'));
+
+    // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysHide`.
+    await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.alwaysHide));
+    await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+    await tester.pumpAndSettle();
+
+    await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_alwaysHide_m3.png'));
+
+    // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.onlyShowSelected`.
+    await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
+    await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).first));
+    await tester.pumpAndSettle();
+
+    await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_selected_m3.png'));
+
+    await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+    await tester.pumpAndSettle();
+
+    await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_unselected_m3.png'));
+  });
+
   testWidgets('Navigation indicator scale transform', (WidgetTester tester) async {
     int selectedIndex = 0;
 
@@ -892,6 +959,264 @@ void main() {
       expect(_getIndicatorDecoration(tester)?.color, color);
       expect(_getIndicatorDecoration(tester)?.shape, shape);
     });
+
+    testWidgets('Navigation indicator renders ripple', (WidgetTester tester) async {
+      // This is a regression test for https://github.com/flutter/flutter/issues/116751.
+      int selectedIndex = 0;
+
+      Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
+        return MaterialApp(
+          theme: ThemeData(useMaterial3: false),
+          home: Scaffold(
+            bottomNavigationBar: Center(
+              child: NavigationBar(
+              selectedIndex: selectedIndex,
+              labelBehavior: labelBehavior,
+              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(buildWidget());
+
+      final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+      await gesture.addPointer();
+      await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
+      await tester.pumpAndSettle();
+
+      final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
+      Offset indicatorCenter = const Offset(600, 33);
+      const Size includedIndicatorSize = Size(64, 32);
+      const Size excludedIndicatorSize = Size(74, 40);
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysShow` (default).
+      expect(
+        inkFeatures,
+        paints
+          ..clipPath(
+            pathMatcher: isPathThat(
+              includes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
+              ],
+              excludes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
+              ],
+            ),
+          )
+          ..circle(
+            x: indicatorCenter.dx,
+            y: indicatorCenter.dy,
+            radius: 35.0,
+            color: const Color(0x0a000000),
+          )
+      );
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysHide`.
+      await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.alwaysHide));
+      await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
+      await tester.pumpAndSettle();
+
+      indicatorCenter = const Offset(600, 40);
+
+      expect(
+        inkFeatures,
+        paints
+          ..clipPath(
+            pathMatcher: isPathThat(
+              includes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
+              ],
+              excludes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
+              ],
+            ),
+          )
+          ..circle(
+            x: indicatorCenter.dx,
+            y: indicatorCenter.dy,
+            radius: 35.0,
+            color: const Color(0x0a000000),
+          )
+      );
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.onlyShowSelected`.
+      await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
+      await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
+      await tester.pumpAndSettle();
+
+      expect(
+        inkFeatures,
+        paints
+          ..clipPath(
+            pathMatcher: isPathThat(
+              includes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
+              ],
+              excludes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
+              ],
+            ),
+          )
+          ..circle(
+            x: indicatorCenter.dx,
+            y: indicatorCenter.dy,
+            radius: 35.0,
+            color: const Color(0x0a000000),
+          )
+      );
+
+      // Make sure ripple is shifted when selectedIndex changes.
+      selectedIndex = 1;
+      await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
+      await tester.pumpAndSettle();
+      indicatorCenter = const Offset(600, 33);
+
+      expect(
+        inkFeatures,
+        paints
+          ..clipPath(
+            pathMatcher: isPathThat(
+              includes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
+              ],
+              excludes: <Offset>[
+                // Left center.
+                Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Top center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
+                // Right center.
+                Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
+                // Bottom center.
+                Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
+              ],
+            ),
+          )
+          ..circle(
+            x: indicatorCenter.dx,
+            y: indicatorCenter.dy,
+            radius: 35.0,
+            color: const Color(0x0a000000),
+          )
+      );
+    });
+
+    testWidgets('Navigation indicator ripple golden test', (WidgetTester tester) async {
+      // This is a regression test for https://github.com/flutter/flutter/issues/117420.
+
+      Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
+        return MaterialApp(
+          theme: ThemeData(useMaterial3: false),
+          home: Scaffold(
+            bottomNavigationBar: Center(
+              child: NavigationBar(
+                labelBehavior: labelBehavior,
+                destinations: const <Widget>[
+                  NavigationDestination(
+                    icon: SizedBox(),
+                    label: 'AC',
+                  ),
+                  NavigationDestination(
+                    icon: SizedBox(),
+                    label: 'Alarm',
+                  ),
+                ],
+                onDestinationSelected: (int i) { },
+              ),
+            ),
+          ),
+        );
+      }
+
+      await tester.pumpWidget(buildWidget());
+
+      final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+      await gesture.addPointer();
+      await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+      await tester.pumpAndSettle();
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysShow` (default).
+      await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_alwaysShow_m2.png'));
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysHide`.
+      await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.alwaysHide));
+      await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+      await tester.pumpAndSettle();
+
+      await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_alwaysHide_m2.png'));
+
+      // Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.onlyShowSelected`.
+      await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
+      await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).first));
+      await tester.pumpAndSettle();
+
+      await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_selected_m2.png'));
+
+      await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+      await tester.pumpAndSettle();
+
+      await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_unselected_m2.png'));
+    });
   });
 }
 

packages/flutter/test/material/navigation_bar_theme_test.dart

@@ -2,6 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This file is run as part of a reduced test set in CI on Mac and Windows
+// machines.
+@Tags(<String>['reduced-test-set'])
+library;
+
+import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -161,6 +167,48 @@ void main() {
     expect(_barMaterial(tester).elevation, elevation);
     expect(_labelBehavior(tester), labelBehavior);
   });
+
+  testWidgets('Custom label style renders ink ripple properly', (WidgetTester tester) async {
+    Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
+      return MaterialApp(
+        theme: ThemeData(
+          navigationBarTheme: const NavigationBarThemeData(
+            labelTextStyle: MaterialStatePropertyAll<TextStyle>(
+              TextStyle(fontSize: 25, color: Color(0xff0000ff)),
+            ),
+          ),
+          useMaterial3: true,
+        ),
+        home: Scaffold(
+          bottomNavigationBar: Center(
+            child: NavigationBar(
+              labelBehavior: labelBehavior,
+              destinations: const <Widget>[
+                NavigationDestination(
+                  icon: SizedBox(),
+                  label: 'AC',
+                ),
+                NavigationDestination(
+                  icon: SizedBox(),
+                  label: 'Alarm',
+                ),
+              ],
+              onDestinationSelected: (int i) { },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(buildWidget());
+
+    final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+    await gesture.addPointer();
+    await gesture.moveTo(tester.getCenter(find.byType(NavigationDestination).last));
+    await tester.pumpAndSettle();
+
+    await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_custom_label_style.png'));
+  });
 }
 
 List<NavigationDestination> _destinations() {