DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-15 23:33:36 +08:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
## Description

This PR reverts the change from [Fix NavigationBar indicator overlay
color](https://github.com/flutter/flutter/pull/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 https://github.com/flutter/flutter/pull/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](https://github.com/flutter/flutter/issues/169249)
Fixes [NavigationDrawer active indicator offset with
SvgPicture](https://github.com/flutter/flutter/issues/169436)
Fixes
https://github.com/flutter/flutter/pull/164484#issuecomment-2910505630

Reopens https://github.com/flutter/flutter/issues/163871
This commit is contained in:
Bruno Leroux 2025-06-02 10:00:46 +02:00 committed by GitHub
parent 8b22f67c85
commit 0b3ba809cc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 52 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/flutter/lib/src/material/navigation_bar.dart
View File

@ -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(

4
packages/flutter/test/material/navigation_bar_test.dart
View File

@ -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?;

4
packages/flutter/test/material/navigation_bar_theme_test.dart
View File

@ -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?;

10
packages/flutter/test/material/navigation_drawer_test.dart
View File

@ -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?;

4
packages/flutter/test/material/navigation_drawer_theme_test.dart
View File

@ -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?;

58
packages/flutter/test/material/navigation_rail_test.dart
View File

@ -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?;

4
packages/flutter/test/material/navigation_rail_theme_test.dart
View File

@ -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?;