Skip to content

Commit

Permalink
Fix NavigationBar ripple for non-default `NavigationDestinationLabe…
Browse files Browse the repository at this point in the history 
…lBehavior` (#116888)
  • Loading branch information
@TahaTesser
TahaTesser committed Dec 12, 2022
1 parent 84ed058 commit a8c9f9c
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 27 deletions.
45 changes: 36 additions & 9 deletions packages/flutter/lib/src/material/navigation_bar.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import 'text_theme.dart';
import 'theme.dart';
import 'tooltip.dart';

const double _kIndicatorHeight = 64;
const double _kIndicatorWidth = 32;
const double _kIndicatorHeight = 32;
const double _kIndicatorWidth = 64;

// Examples can assume:
// late BuildContext context;
Expand Down Expand Up @@ -212,6 +212,7 @@ class NavigationBar extends StatelessWidget {
builder: (BuildContext context, Animation<double> animation) {
return _NavigationDestinationInfo(
index: i,
selectedIndex: selectedIndex,
totalNumberOfDestinations: destinations.length,
selectedAnimation: animation,
labelBehavior: effectiveLabelBehavior,
Expand Down Expand Up @@ -435,10 +436,25 @@ class _NavigationDestinationBuilder extends StatelessWidget {
final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(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(
child: _NavigationBarDestinationTooltip(
message: tooltip ?? label,
child: _IndicatorInkWell(
key: UniqueKey(),
labelPadding: labelPadding,
customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape,
onTap: info.onTap,
child: Row(
Expand All @@ -459,24 +475,28 @@ class _NavigationDestinationBuilder extends StatelessWidget {

class _IndicatorInkWell extends InkResponse {
const _IndicatorInkWell({
super.child,
super.onTap,
super.key,
required this.labelPadding,
super.customBorder,
super.onTap,
super.child,
}) : super(
containedInkWell: true,
highlightColor: Colors.transparent,
);

final double labelPadding;

@override
RectCallback? getRectCallback(RenderBox referenceBox) {
final double indicatorOffsetX = referenceBox.size.width / 2;
const double indicatorOffsetY = 30.0;
final double indicatorOffsetY = referenceBox.size.height / 2 - labelPadding;

return () {
return Rect.fromCenter(
center: Offset(indicatorOffsetX, indicatorOffsetY),
width: _kIndicatorHeight,
height: _kIndicatorWidth,
width: _kIndicatorWidth,
height: _kIndicatorHeight,
);
};
}
Expand All @@ -492,6 +512,7 @@ class _NavigationDestinationInfo extends InheritedWidget {
/// [child] and descendants.
const _NavigationDestinationInfo({
required this.index,
required this.selectedIndex,
required this.totalNumberOfDestinations,
required this.selectedAnimation,
required this.labelBehavior,
Expand Down Expand Up @@ -529,6 +550,12 @@ class _NavigationDestinationInfo extends InheritedWidget {
/// "Tab 1 of 3", for example.
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.
///
/// This is required for semantics, so that each destination can have a label
Expand Down Expand Up @@ -593,8 +620,8 @@ class NavigationIndicator extends StatelessWidget {
super.key,
required this.animation,
this.color,
this.width = _kIndicatorHeight,
this.height = _kIndicatorWidth,
this.width = _kIndicatorWidth,
this.height = _kIndicatorHeight,
this.borderRadius = const BorderRadius.all(Radius.circular(16)),
this.shape,
});
Expand Down
167 changes: 149 additions & 18 deletions packages/flutter/test/material/navigation_bar_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,34 +558,165 @@ void main() {
});

testWidgets('Navigation indicator renders ripple', (WidgetTester tester) async {
final Widget widget = _buildWidget(
NavigationBar(
destinations: const <Widget>[
NavigationDestination(
icon: Icon(Icons.ac_unit),
label: 'AC',
),
NavigationDestination(
icon: Icon(Icons.access_alarm),
label: 'Alarm',
),
],
onDestinationSelected: (int i) {
},
),
);
// This is a regression test for https://github.com/flutter/flutter/issues/116751.
int selectedIndex = 0;

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);
await gesture.addPointer();
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.access_alarm)));
await tester.pumpAndSettle();

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 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(
inkFeatures,
paints
Expand Down

0 comments on commit a8c9f9c

Please sign in to comment.