Revert "Fix NavigationBar indicator overlay color (#164484)" (#169497)

## Description

This PR reverts the change from [Fix NavigationBar indicator overlay
color](#164484) as it leads to
several regressions.

The change was very small: replacing a Container (which obscured the
overlay) with an Ink. Unfortunately this leads to some rendering issues
which seem related to the Ink painting not being fully in sync with the
animation logic implemented by `NavigationIndicator`.
I investigated this but did not find an obvious solution. So I would
prefer to revert #164484 as the
issue it fixed has less impact than the regression.
cc @justinmc for review and also in case you have some clue about why an
Ink would cause such problems compared to a Container.

## Related Issue

Fixes [[Flutter 3.32.0] Active NavigationBar item not selected when
switching items
programmatically](#169249)
Fixes [NavigationDrawer active indicator offset with
SvgPicture](#169436)
Fixes
#164484 (comment)

Reopens #163871

Author: bleroux

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

@@ -16,7 +16,6 @@ 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';
@@ -869,7 +868,7 @@ class NavigationIndicator extends StatelessWidget {
             builder: (BuildContext context, Animation<double> fadeAnimation) {
               return FadeTransition(
                 opacity: fadeAnimation,
-                child: Ink(
+                child: Container(
                   width: width,
                   height: height,
                   decoration: ShapeDecoration(

packages/flutter/test/material/navigation_bar_test.dart

@@ -1716,8 +1716,8 @@ Material _getMaterial(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;

packages/flutter/test/material/navigation_bar_theme_test.dart

@@ -367,8 +367,8 @@ Material _barMaterial(WidgetTester tester) {
 
 ShapeDecoration? _indicator(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;

packages/flutter/test/material/navigation_drawer_test.dart

@@ -123,16 +123,14 @@ void main() {
     await tester.tap(find.text('AC'));
     await tester.pump();
 
-    // When no background color is set, only the non-visible indicator Ink is expected.
-    expect(findDestinationInk('AC'), findsOne);
+    expect(findDestinationInk('AC'), findsNothing);
 
     // 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'), findsNWidgets(2));
-    // The drawer destination Ink is the first one, the second is the indicator's one.
+    expect(findDestinationInk('Alarm'), findsOne);
     final BoxDecoration destinationDecoration =
         tester.firstWidget<Ink>(findDestinationInk('Alarm')).decoration! as BoxDecoration;
     expect(destinationDecoration.color, color);
@@ -551,8 +549,8 @@ InkWell? _getInkWell(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;

packages/flutter/test/material/navigation_drawer_theme_test.dart

@@ -285,8 +285,8 @@ Material _getMaterial(WidgetTester tester) {
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;

packages/flutter/test/material/navigation_rail_test.dart

@@ -3076,7 +3076,6 @@ 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);
@@ -3102,7 +3101,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(12.0, 0.0, 68.0, 32.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(12.0, 72.0, 68.0, 104.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3164,7 +3163,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(12.0, 6.0, 68.0, 38.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(12.0, 58.0, 68.0, 90.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3230,7 +3229,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(30.0, 24.0, 86.0, 56.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(30.0, 96.0, 86.0, 128.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3295,7 +3294,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(0.0, 6.0, 50.0, 38.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(0.0, 58.0, 50.0, 90.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3362,7 +3361,7 @@ void main() {
         )
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
-          rrect: RRect.fromLTRBR(140.0, 24.0, 196.0, 56.0, const Radius.circular(16)),
+          rrect: RRect.fromLTRBR(140.0, 96.0, 196.0, 128.0, const Radius.circular(16)),
           color: const Color(0xffe8def8),
         ),
     );
@@ -3402,11 +3401,13 @@ 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;
@@ -3417,7 +3418,13 @@ void main() {
     final Rect indicatorRect = Rect.fromLTRB(indicatorLeft, 0.0, indicatorRight, indicatorHeight);
     final Rect includedRect = indicatorRect;
     final Rect excludedRect = includedRect.inflate(10);
-    const double indicatorHorizontalPadding = (railMinWidth - indicatorWidth) / 2; // 12.0
+
+    // 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;
 
     expect(
       inkFeatures,
@@ -3441,10 +3448,10 @@ void main() {
         ..rect(rect: indicatorRect, color: const Color(0x0a6750a4))
         ..rrect(
           rrect: RRect.fromLTRBR(
-            indicatorHorizontalPadding,
-            0.0,
-            indicatorHorizontalPadding + indicatorWidth,
-            indicatorHeight,
+            indicatorLeft,
+            secondIndicatorVerticalOffset,
+            indicatorRight,
+            secondIndicatorVerticalOffset + indicatorHeight,
             const Radius.circular(16),
           ),
           color: const Color(0xffe8def8),
@@ -3497,6 +3504,9 @@ 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;
@@ -3516,8 +3526,13 @@ void main() {
     final Rect includedRect = indicatorRect;
     final Rect excludedRect = includedRect.inflate(10);
 
-    // Icon height is greater than indicator height so the indicator has a vertical offset.
-    const double secondIndicatorVerticalOffset = (iconSize - indicatorHeight) / 2;
+    // 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;
 
     expect(
       inkFeatures,
@@ -3596,6 +3611,7 @@ 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.
@@ -3615,7 +3631,11 @@ void main() {
     final Rect excludedRect = includedRect.inflate(10);
 
     // Compute the vertical position for the selected destination (the one with 'bookmark' icon).
-    const double secondIndicatorVerticalOffset = verticalDestinationSpacingM3 / 2;
+    const double destinationHeight = indicatorHeight + verticalDestinationSpacingM3;
+    const double secondDestinationVerticalOffset = verticalSpacer + destinationHeight;
+    const double secondIndicatorVerticalOffset =
+        secondDestinationVerticalOffset + verticalDestinationSpacingM3 / 2;
+    const double secondDestinationHorizontalOffset = 800 - railMinExtendedWidth; // RTL.
 
     expect(
       inkFeatures,
@@ -3641,9 +3661,9 @@ void main() {
         // Indicator for the selected destination (the one with 'bookmark' icon).
         ..rrect(
           rrect: RRect.fromLTRBR(
-            indicatorLeft,
+            secondDestinationHorizontalOffset + indicatorLeft,
             secondIndicatorVerticalOffset,
-            indicatorRight,
+            secondDestinationHorizontalOffset + indicatorRight,
             secondIndicatorVerticalOffset + indicatorHeight,
             const Radius.circular(16),
           ),
@@ -6150,8 +6170,8 @@ Widget _buildWidget(Widget child, {bool useMaterial3 = true, bool isRTL = false}
 
 ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(FadeTransition), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;

packages/flutter/test/material/navigation_rail_theme_test.dart

@@ -327,8 +327,8 @@ Material _railMaterial(WidgetTester tester) {
 
 ShapeDecoration? _indicatorDecoration(WidgetTester tester) {
   return tester
-          .firstWidget<Ink>(
-            find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Ink)),
+          .firstWidget<Container>(
+            find.descendant(of: find.byType(NavigationIndicator), matching: find.byType(Container)),
           )
           .decoration
       as ShapeDecoration?;