Skip to content

Commit a8c9f9c

Browse files
TahaTesserTahaTesser
authored
Fix NavigationBar ripple for non-default NavigationDestinationLabelBehavior (#116888)
1 parent 84ed058 commit a8c9f9c

File tree

2 files changed

+185
-27
lines changed

2 files changed

+185
-27
lines changed

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

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import 'text_theme.dart';
1717
import 'theme.dart';
1818
import 'tooltip.dart';
1919

20-
const double _kIndicatorHeight = 64;
21-
const double _kIndicatorWidth = 32;
20+
const double _kIndicatorHeight = 32;
21+
const double _kIndicatorWidth = 64;
2222

2323
// Examples can assume:
2424
// late BuildContext context;
@@ -212,6 +212,7 @@ class NavigationBar extends StatelessWidget {
212212
builder: (BuildContext context, Animation<double> animation) {
213213
return _NavigationDestinationInfo(
214214
index: i,
215+
selectedIndex: selectedIndex,
215216
totalNumberOfDestinations: destinations.length,
216217
selectedAnimation: animation,
217218
labelBehavior: effectiveLabelBehavior,
@@ -435,10 +436,25 @@ class _NavigationDestinationBuilder extends StatelessWidget {
435436
final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context);
436437
final NavigationBarThemeData defaults = _defaultsFor(context);
437438

439+
final bool selected = info.selectedIndex == info.index;
440+
final double labelPadding;
441+
switch (info.labelBehavior) {
442+
case NavigationDestinationLabelBehavior.alwaysShow:
443+
labelPadding = 10;
444+
break;
445+
case NavigationDestinationLabelBehavior.onlyShowSelected:
446+
labelPadding = selected ? 10 : 0;
447+
break;
448+
case NavigationDestinationLabelBehavior.alwaysHide:
449+
labelPadding = 0;
450+
break;
451+
}
438452
return _NavigationBarDestinationSemantics(
439453
child: _NavigationBarDestinationTooltip(
440454
message: tooltip ?? label,
441455
child: _IndicatorInkWell(
456+
key: UniqueKey(),
457+
labelPadding: labelPadding,
442458
customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape,
443459
onTap: info.onTap,
444460
child: Row(
@@ -459,24 +475,28 @@ class _NavigationDestinationBuilder extends StatelessWidget {
459475

460476
class _IndicatorInkWell extends InkResponse {
461477
const _IndicatorInkWell({
462-
super.child,
463-
super.onTap,
478+
super.key,
479+
required this.labelPadding,
464480
super.customBorder,
481+
super.onTap,
482+
super.child,
465483
}) : super(
466484
containedInkWell: true,
467485
highlightColor: Colors.transparent,
468486
);
469487

488+
final double labelPadding;
489+
470490
@override
471491
RectCallback? getRectCallback(RenderBox referenceBox) {
472492
final double indicatorOffsetX = referenceBox.size.width / 2;
473-
const double indicatorOffsetY = 30.0;
493+
final double indicatorOffsetY = referenceBox.size.height / 2 - labelPadding;
474494

475495
return () {
476496
return Rect.fromCenter(
477497
center: Offset(indicatorOffsetX, indicatorOffsetY),
478-
width: _kIndicatorHeight,
479-
height: _kIndicatorWidth,
498+
width: _kIndicatorWidth,
499+
height: _kIndicatorHeight,
480500
);
481501
};
482502
}
@@ -492,6 +512,7 @@ class _NavigationDestinationInfo extends InheritedWidget {
492512
/// [child] and descendants.
493513
const _NavigationDestinationInfo({
494514
required this.index,
515+
required this.selectedIndex,
495516
required this.totalNumberOfDestinations,
496517
required this.selectedAnimation,
497518
required this.labelBehavior,
@@ -529,6 +550,12 @@ class _NavigationDestinationInfo extends InheritedWidget {
529550
/// "Tab 1 of 3", for example.
530551
final int index;
531552

553+
/// This is the index of the currently selected destination.
554+
///
555+
/// This is required for `_IndicatorInkWell` to apply label padding to ripple animations
556+
/// when label behavior is [NavigationDestinationLabelBehavior.onlyShowSelected].
557+
final int selectedIndex;
558+
532559
/// How many total destinations are are in this navigation bar.
533560
///
534561
/// This is required for semantics, so that each destination can have a label
@@ -593,8 +620,8 @@ class NavigationIndicator extends StatelessWidget {
593620
super.key,
594621
required this.animation,
595622
this.color,
596-
this.width = _kIndicatorHeight,
597-
this.height = _kIndicatorWidth,
623+
this.width = _kIndicatorWidth,
624+
this.height = _kIndicatorHeight,
598625
this.borderRadius = const BorderRadius.all(Radius.circular(16)),
599626
this.shape,
600627
});

‎packages/flutter/test/material/navigation_bar_test.dart

Lines changed: 149 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -558,34 +558,165 @@ void main() {
558558
});
559559

560560
testWidgets('Navigation indicator renders ripple', (WidgetTester tester) async {
561-
final Widget widget = _buildWidget(
562-
NavigationBar(
563-
destinations: const <Widget>[
564-
NavigationDestination(
565-
icon: Icon(Icons.ac_unit),
566-
label: 'AC',
567-
),
568-
NavigationDestination(
569-
icon: Icon(Icons.access_alarm),
570-
label: 'Alarm',
571-
),
572-
],
573-
onDestinationSelected: (int i) {
574-
},
575-
),
576-
);
561+
// This is a regression test for https://github.com/flutter/flutter/issues/116751.
562+
int selectedIndex = 0;
577563

578-
await tester.pumpWidget(widget);
564+
Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
565+
return _buildWidget(
566+
NavigationBar(
567+
selectedIndex: selectedIndex,
568+
labelBehavior: labelBehavior,
569+
destinations: const <Widget>[
570+
NavigationDestination(
571+
icon: Icon(Icons.ac_unit),
572+
label: 'AC',
573+
),
574+
NavigationDestination(
575+
icon: Icon(Icons.access_alarm),
576+
label: 'Alarm',
577+
),
578+
],
579+
onDestinationSelected: (int i) { },
580+
),
581+
);
582+
}
583+
584+
await tester.pumpWidget(buildWidget());
579585

580586
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
581587
await gesture.addPointer();
582588
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
583589
await tester.pumpAndSettle();
584590

585591
final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
586-
const Offset indicatorCenter = Offset(600, 30);
592+
Offset indicatorCenter = const Offset(600, 30);
587593
const Size includedIndicatorSize = Size(64, 32);
588594
const Size excludedIndicatorSize = Size(74, 40);
595+
596+
// Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysShow` (default).
597+
expect(
598+
inkFeatures,
599+
paints
600+
..clipPath(
601+
pathMatcher: isPathThat(
602+
includes: <Offset>[
603+
// Left center.
604+
Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
605+
// Top center.
606+
Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
607+
// Right center.
608+
Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
609+
// Bottom center.
610+
Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
611+
],
612+
excludes: <Offset>[
613+
// Left center.
614+
Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
615+
// Top center.
616+
Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
617+
// Right center.
618+
Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
619+
// Bottom center.
620+
Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
621+
],
622+
),
623+
)
624+
..circle(
625+
x: indicatorCenter.dx,
626+
y: indicatorCenter.dy,
627+
radius: 35.0,
628+
color: const Color(0x0a000000),
629+
)
630+
);
631+
632+
// Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.alwaysHide`.
633+
await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.alwaysHide));
634+
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
635+
await tester.pumpAndSettle();
636+
637+
indicatorCenter = const Offset(600, 40);
638+
639+
expect(
640+
inkFeatures,
641+
paints
642+
..clipPath(
643+
pathMatcher: isPathThat(
644+
includes: <Offset>[
645+
// Left center.
646+
Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
647+
// Top center.
648+
Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
649+
// Right center.
650+
Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
651+
// Bottom center.
652+
Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
653+
],
654+
excludes: <Offset>[
655+
// Left center.
656+
Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
657+
// Top center.
658+
Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
659+
// Right center.
660+
Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
661+
// Bottom center.
662+
Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
663+
],
664+
),
665+
)
666+
..circle(
667+
x: indicatorCenter.dx,
668+
y: indicatorCenter.dy,
669+
radius: 35.0,
670+
color: const Color(0x0a000000),
671+
)
672+
);
673+
674+
// Test ripple when NavigationBar is using `NavigationDestinationLabelBehavior.onlyShowSelected`.
675+
await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
676+
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
677+
await tester.pumpAndSettle();
678+
679+
expect(
680+
inkFeatures,
681+
paints
682+
..clipPath(
683+
pathMatcher: isPathThat(
684+
includes: <Offset>[
685+
// Left center.
686+
Offset(indicatorCenter.dx - (includedIndicatorSize.width / 2), indicatorCenter.dy),
687+
// Top center.
688+
Offset(indicatorCenter.dx, indicatorCenter.dy - (includedIndicatorSize.height / 2)),
689+
// Right center.
690+
Offset(indicatorCenter.dx + (includedIndicatorSize.width / 2), indicatorCenter.dy),
691+
// Bottom center.
692+
Offset(indicatorCenter.dx, indicatorCenter.dy + (includedIndicatorSize.height / 2)),
693+
],
694+
excludes: <Offset>[
695+
// Left center.
696+
Offset(indicatorCenter.dx - (excludedIndicatorSize.width / 2), indicatorCenter.dy),
697+
// Top center.
698+
Offset(indicatorCenter.dx, indicatorCenter.dy - (excludedIndicatorSize.height / 2)),
699+
// Right center.
700+
Offset(indicatorCenter.dx + (excludedIndicatorSize.width / 2), indicatorCenter.dy),
701+
// Bottom center.
702+
Offset(indicatorCenter.dx, indicatorCenter.dy + (excludedIndicatorSize.height / 2)),
703+
],
704+
),
705+
)
706+
..circle(
707+
x: indicatorCenter.dx,
708+
y: indicatorCenter.dy,
709+
radius: 35.0,
710+
color: const Color(0x0a000000),
711+
)
712+
);
713+
714+
// Make sure ripple is shifted when selectedIndex changes.
715+
selectedIndex = 1;
716+
await tester.pumpWidget(buildWidget(labelBehavior: NavigationDestinationLabelBehavior.onlyShowSelected));
717+
await tester.pumpAndSettle();
718+
indicatorCenter = const Offset(600, 30);
719+
589720
expect(
590721
inkFeatures,
591722
paints

0 commit comments

Comments
 (0)