mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
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.
This commit is contained in:
parent
d903fa2d9d
commit
b5aef9c366
@ -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<double> 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: <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),
|
||||
),
|
||||
],
|
||||
);
|
||||
|
||||
@ -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 <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 @@ Material _getMaterial(WidgetTester tester) {
|
||||
|
||||
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?;
|
||||
|
||||
@ -365,8 +365,8 @@ Material _barMaterial(WidgetTester tester) {
|
||||
|
||||
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?;
|
||||
|
||||
@ -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<Ink>(findDestinationInk('Alarm')).decoration! as BoxDecoration;
|
||||
expect(destinationDecoration.color, color);
|
||||
@ -549,8 +550,8 @@ InkWell? _getInkWell(WidgetTester tester) {
|
||||
|
||||
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?;
|
||||
|
||||
@ -283,8 +283,8 @@ Material _getMaterial(WidgetTester tester) {
|
||||
|
||||
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?;
|
||||
|
||||
@ -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<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?;
|
||||
|
||||
@ -324,8 +324,8 @@ Material _railMaterial(WidgetTester tester) {
|
||||
|
||||
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?;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user