Unverified Commit a8c9f9c6 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `NavigationBar` ripple for non-default `NavigationDestinationLabelBehavior` (#116888)

parent 84ed058b
...@@ -17,8 +17,8 @@ import 'text_theme.dart'; ...@@ -17,8 +17,8 @@ import 'text_theme.dart';
import 'theme.dart'; import 'theme.dart';
import 'tooltip.dart'; import 'tooltip.dart';
const double _kIndicatorHeight = 64; const double _kIndicatorHeight = 32;
const double _kIndicatorWidth = 32; const double _kIndicatorWidth = 64;
// Examples can assume: // Examples can assume:
// late BuildContext context; // late BuildContext context;
...@@ -212,6 +212,7 @@ class NavigationBar extends StatelessWidget { ...@@ -212,6 +212,7 @@ class NavigationBar extends StatelessWidget {
builder: (BuildContext context, Animation<double> animation) { builder: (BuildContext context, Animation<double> animation) {
return _NavigationDestinationInfo( return _NavigationDestinationInfo(
index: i, index: i,
selectedIndex: selectedIndex,
totalNumberOfDestinations: destinations.length, totalNumberOfDestinations: destinations.length,
selectedAnimation: animation, selectedAnimation: animation,
labelBehavior: effectiveLabelBehavior, labelBehavior: effectiveLabelBehavior,
...@@ -435,10 +436,25 @@ class _NavigationDestinationBuilder extends StatelessWidget { ...@@ -435,10 +436,25 @@ class _NavigationDestinationBuilder extends StatelessWidget {
final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context); final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context);
final NavigationBarThemeData defaults = _defaultsFor(context); final NavigationBarThemeData defaults = _defaultsFor(context);
final bool selected = info.selectedIndex == info.index;
final double labelPadding;
switch (info.labelBehavior) {
case NavigationDestinationLabelBehavior.alwaysShow:
labelPadding = 10;
break;
case NavigationDestinationLabelBehavior.onlyShowSelected:
labelPadding = selected ? 10 : 0;
break;
case NavigationDestinationLabelBehavior.alwaysHide:
labelPadding = 0;
break;
}
return _NavigationBarDestinationSemantics( return _NavigationBarDestinationSemantics(
child: _NavigationBarDestinationTooltip( child: _NavigationBarDestinationTooltip(
message: tooltip ?? label, message: tooltip ?? label,
child: _IndicatorInkWell( child: _IndicatorInkWell(
key: UniqueKey(),
labelPadding: labelPadding,
customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape, customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape,
onTap: info.onTap, onTap: info.onTap,
child: Row( child: Row(
...@@ -459,24 +475,28 @@ class _NavigationDestinationBuilder extends StatelessWidget { ...@@ -459,24 +475,28 @@ class _NavigationDestinationBuilder extends StatelessWidget {
class _IndicatorInkWell extends InkResponse { class _IndicatorInkWell extends InkResponse {
const _IndicatorInkWell({ const _IndicatorInkWell({
super.child, super.key,
super.onTap, required this.labelPadding,
super.customBorder, super.customBorder,
super.onTap,
super.child,
}) : super( }) : super(
containedInkWell: true, containedInkWell: true,
highlightColor: Colors.transparent, highlightColor: Colors.transparent,
); );
final double labelPadding;
@override @override
RectCallback? getRectCallback(RenderBox referenceBox) { RectCallback? getRectCallback(RenderBox referenceBox) {
final double indicatorOffsetX = referenceBox.size.width / 2; final double indicatorOffsetX = referenceBox.size.width / 2;
const double indicatorOffsetY = 30.0; final double indicatorOffsetY = referenceBox.size.height / 2 - labelPadding;
return () { return () {
return Rect.fromCenter( return Rect.fromCenter(
center: Offset(indicatorOffsetX, indicatorOffsetY), center: Offset(indicatorOffsetX, indicatorOffsetY),
width: _kIndicatorHeight, width: _kIndicatorWidth,
height: _kIndicatorWidth, height: _kIndicatorHeight,
); );
}; };
} }
...@@ -492,6 +512,7 @@ class _NavigationDestinationInfo extends InheritedWidget { ...@@ -492,6 +512,7 @@ class _NavigationDestinationInfo extends InheritedWidget {
/// [child] and descendants. /// [child] and descendants.
const _NavigationDestinationInfo({ const _NavigationDestinationInfo({
required this.index, required this.index,
required this.selectedIndex,
required this.totalNumberOfDestinations, required this.totalNumberOfDestinations,
required this.selectedAnimation, required this.selectedAnimation,
required this.labelBehavior, required this.labelBehavior,
...@@ -529,6 +550,12 @@ class _NavigationDestinationInfo extends InheritedWidget { ...@@ -529,6 +550,12 @@ class _NavigationDestinationInfo extends InheritedWidget {
/// "Tab 1 of 3", for example. /// "Tab 1 of 3", for example.
final int index; final int index;
/// This is the index of the currently selected destination.
///
/// This is required for `_IndicatorInkWell` to apply label padding to ripple animations
/// when label behavior is [NavigationDestinationLabelBehavior.onlyShowSelected].
final int selectedIndex;
/// How many total destinations are are in this navigation bar. /// How many total destinations are are in this navigation bar.
/// ///
/// This is required for semantics, so that each destination can have a label /// This is required for semantics, so that each destination can have a label
...@@ -593,8 +620,8 @@ class NavigationIndicator extends StatelessWidget { ...@@ -593,8 +620,8 @@ class NavigationIndicator extends StatelessWidget {
super.key, super.key,
required this.animation, required this.animation,
this.color, this.color,
this.width = _kIndicatorHeight, this.width = _kIndicatorWidth,
this.height = _kIndicatorWidth, this.height = _kIndicatorHeight,
this.borderRadius = const BorderRadius.all(Radius.circular(16)), this.borderRadius = const BorderRadius.all(Radius.circular(16)),
this.shape, this.shape,
}); });
......
...@@ -558,24 +558,30 @@ void main() { ...@@ -558,24 +558,30 @@ void main() {
}); });
testWidgets('Navigation indicator renders ripple', (WidgetTester tester) async { testWidgets('Navigation indicator renders ripple', (WidgetTester tester) async {
final Widget widget = _buildWidget( // This is a regression test for https://github.com/flutter/flutter/issues/116751.
NavigationBar( int selectedIndex = 0;
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(widget); Widget buildWidget({ NavigationDestinationLabelBehavior? labelBehavior }) {
return _buildWidget(
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); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer(); await gesture.addPointer();
...@@ -583,9 +589,134 @@ void main() { ...@@ -583,9 +589,134 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
const Offset indicatorCenter = Offset(600, 30); Offset indicatorCenter = const Offset(600, 30);
const Size includedIndicatorSize = Size(64, 32); const Size includedIndicatorSize = Size(64, 32);
const Size excludedIndicatorSize = Size(74, 40); 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, 30);
expect( expect(
inkFeatures, inkFeatures,
paints paints
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment