Skip to content

Commit

Permalink
Fix NavigationRail highlight (#117320) (#117344)
Browse files Browse the repository at this point in the history
Co-authored-by: Taha Tesser <tessertaha@gmail.com>
Co-authored-by: Kevin Chisholm <kevinjchisholm@google.com>
  • Loading branch information
3 people committed Dec 19, 2022
1 parent e86981e commit 13f24e5
Show file tree
Hide file tree
Showing 2 changed files with 286 additions and 15 deletions.
43 changes: 28 additions & 15 deletions packages/flutter/lib/src/material/navigation_rail.dart
Expand Up @@ -16,6 +16,7 @@ import 'text_theme.dart';
import 'theme.dart';

const double _kCircularIndicatorDiameter = 56;
const double _kIndicatorHeight = 32;

/// A Material Design widget that is meant to be displayed at the left or right of an
/// app to navigate between a small number of views, typically between three and
Expand Down Expand Up @@ -579,7 +580,8 @@ class _RailDestination extends StatelessWidget {
);

final bool material3 = Theme.of(context).useMaterial3;
final double indicatorInkOffsetY;
final EdgeInsets destionationPadding = (padding ?? EdgeInsets.zero).resolve(Directionality.of(context));
Offset indicatorOffset;

final Widget themedIcon = IconTheme(
data: iconTheme,
Expand All @@ -596,8 +598,10 @@ class _RailDestination extends StatelessWidget {
case NavigationRailLabelType.none:
// Split the destination spacing across the top and bottom to keep the icon centered.
final Widget? spacing = material3 ? const SizedBox(height: _verticalDestinationSpacingM3 / 2) : null;
indicatorInkOffsetY = _verticalDestinationPaddingNoLabel - (_verticalIconLabelSpacingM3 / 2);

indicatorOffset = Offset(
minWidth / 2 + destionationPadding.left,
_verticalDestinationSpacingM3 / 2 + destionationPadding.top,
);
final Widget iconPart = Column(
children: <Widget>[
if (spacing != null) spacing,
Expand Down Expand Up @@ -675,8 +679,12 @@ class _RailDestination extends StatelessWidget {
final Widget topSpacing = SizedBox(height: material3 ? 0 : verticalPadding);
final Widget labelSpacing = SizedBox(height: material3 ? lerpDouble(0, _verticalIconLabelSpacingM3, appearingAnimationValue)! : 0);
final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : verticalPadding);
indicatorInkOffsetY = _verticalDestinationPaddingWithLabel;

final double indicatorHorizontalPadding = (destionationPadding.left / 2) - (destionationPadding.right / 2);
final double indicatorVerticalPadding = destionationPadding.top;
indicatorOffset = Offset(minWidth / 2 + indicatorHorizontalPadding, indicatorVerticalPadding);
if (minWidth < _NavigationRailDefaultsM2(context).minWidth!) {
indicatorOffset = Offset(minWidth / 2 + _horizontalDestinationSpacingM3, indicatorVerticalPadding);
}
content = Container(
constraints: BoxConstraints(
minWidth: minWidth,
Expand Down Expand Up @@ -719,7 +727,12 @@ class _RailDestination extends StatelessWidget {
final Widget topSpacing = SizedBox(height: material3 ? 0 : _verticalDestinationPaddingWithLabel);
final Widget labelSpacing = SizedBox(height: material3 ? _verticalIconLabelSpacingM3 : 0);
final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : _verticalDestinationPaddingWithLabel);
indicatorInkOffsetY = _verticalDestinationPaddingWithLabel;
final double indicatorHorizontalPadding = (destionationPadding.left / 2) - (destionationPadding.right / 2);
final double indicatorVerticalPadding = destionationPadding.top;
indicatorOffset = Offset(minWidth / 2 + indicatorHorizontalPadding, indicatorVerticalPadding);
if (minWidth < _NavigationRailDefaultsM2(context).minWidth!) {
indicatorOffset = Offset(minWidth / 2 + _horizontalDestinationSpacingM3, indicatorVerticalPadding);
}
content = Container(
constraints: BoxConstraints(
minWidth: minWidth,
Expand Down Expand Up @@ -761,7 +774,7 @@ class _RailDestination extends StatelessWidget {
splashColor: colors.primary.withOpacity(0.12),
hoverColor: colors.primary.withOpacity(0.04),
useMaterial3: material3,
indicatorOffsetY: indicatorInkOffsetY,
indicatorOffset: indicatorOffset,
child: content,
),
),
Expand All @@ -783,7 +796,7 @@ class _IndicatorInkWell extends InkResponse {
super.splashColor,
super.hoverColor,
required this.useMaterial3,
required this.indicatorOffsetY,
required this.indicatorOffset,
}) : super(
containedInkWell: true,
highlightShape: BoxShape.rectangle,
Expand All @@ -792,18 +805,17 @@ class _IndicatorInkWell extends InkResponse {
);

final bool useMaterial3;
final double indicatorOffsetY;
final Offset indicatorOffset;

@override
RectCallback? getRectCallback(RenderBox referenceBox) {
final double indicatorOffsetX = referenceBox.size.width / 2;

if (useMaterial3) {
return () {
return Rect.fromCenter(
center: Offset(indicatorOffsetX, indicatorOffsetY),
width: _kCircularIndicatorDiameter,
height: 32,
return Rect.fromLTWH(
indicatorOffset.dx - (_kCircularIndicatorDiameter / 2),
indicatorOffset.dy,
_kCircularIndicatorDiameter,
_kIndicatorHeight,
);
};
}
Expand Down Expand Up @@ -965,6 +977,7 @@ const double _verticalDestinationPaddingWithLabel = 16.0;
const Widget _verticalSpacer = SizedBox(height: 8.0);
const double _verticalIconLabelSpacingM3 = 4.0;
const double _verticalDestinationSpacingM3 = 12.0;
const double _horizontalDestinationSpacingM3 = 12.0;

// Hand coded defaults based on Material Design 2.
class _NavigationRailDefaultsM2 extends NavigationRailThemeData {
Expand Down
258 changes: 258 additions & 0 deletions packages/flutter/test/material/navigation_rail_test.dart
Expand Up @@ -2802,6 +2802,264 @@ void main() {
);
});

testWidgets('NavigationRail indicator renders ripple - extended', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/117126
await _pumpNavigationRail(
tester,
navigationRail: NavigationRail(
selectedIndex: 1,
extended: true,
destinations: const <NavigationRailDestination>[
NavigationRailDestination(
icon: Icon(Icons.favorite_border),
selectedIcon: Icon(Icons.favorite),
label: Text('Abc'),
),
NavigationRailDestination(
icon: Icon(Icons.bookmark_border),
selectedIcon: Icon(Icons.bookmark),
label: Text('Def'),
),
],
labelType: NavigationRailLabelType.none,
),
);

final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.favorite_border)));
await tester.pumpAndSettle();

final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
const Rect indicatorRect = Rect.fromLTRB(12.0, 6.0, 68.0, 38.0);
const Rect includedRect = indicatorRect;
final Rect excludedRect = includedRect.inflate(10);

expect(
inkFeatures,
paints
..clipPath(
pathMatcher: isPathThat(
includes: <Offset>[
includedRect.centerLeft,
includedRect.topCenter,
includedRect.centerRight,
includedRect.bottomCenter,
],
excludes: <Offset>[
excludedRect.centerLeft,
excludedRect.topCenter,
excludedRect.centerRight,
excludedRect.bottomCenter,
],
),
)
..rect(
rect: indicatorRect,
color: const Color(0x0a6750a4),
)
..rrect(
rrect: RRect.fromLTRBR(12.0, 58.0, 68.0, 90.0, const Radius.circular(16)),
color: const Color(0xffe8def8),
),
);
});

testWidgets('NavigationRail indicator renders properly when padding is applied', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/117126
await _pumpNavigationRail(
tester,
navigationRail: NavigationRail(
selectedIndex: 1,
extended: true,
destinations: const <NavigationRailDestination>[
NavigationRailDestination(
padding: EdgeInsets.all(10),
icon: Icon(Icons.favorite_border),
selectedIcon: Icon(Icons.favorite),
label: Text('Abc'),
),
NavigationRailDestination(
padding: EdgeInsets.all(18),
icon: Icon(Icons.bookmark_border),
selectedIcon: Icon(Icons.bookmark),
label: Text('Def'),
),
],
labelType: NavigationRailLabelType.none,
),
);

final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.favorite_border)));
await tester.pumpAndSettle();

final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
const Rect indicatorRect = Rect.fromLTRB(22.0, 16.0, 78.0, 48.0);
const Rect includedRect = indicatorRect;
final Rect excludedRect = includedRect.inflate(10);

expect(
inkFeatures,
paints
..clipPath(
pathMatcher: isPathThat(
includes: <Offset>[
includedRect.centerLeft,
includedRect.topCenter,
includedRect.centerRight,
includedRect.bottomCenter,
],
excludes: <Offset>[
excludedRect.centerLeft,
excludedRect.topCenter,
excludedRect.centerRight,
excludedRect.bottomCenter,
],
),
)
..rect(
rect: indicatorRect,
color: const Color(0x0a6750a4),
)
..rrect(
rrect: RRect.fromLTRBR(30.0, 96.0, 86.0, 128.0, const Radius.circular(16)),
color: const Color(0xffe8def8),
),
);
});

testWidgets('Indicator renders properly when NavigationRai.minWidth < default minWidth', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/117126
await _pumpNavigationRail(
tester,
navigationRail: NavigationRail(
minWidth: 50,
selectedIndex: 1,
extended: true,
destinations: const <NavigationRailDestination>[
NavigationRailDestination(
icon: Icon(Icons.favorite_border),
selectedIcon: Icon(Icons.favorite),
label: Text('Abc'),
),
NavigationRailDestination(
icon: Icon(Icons.bookmark_border),
selectedIcon: Icon(Icons.bookmark),
label: Text('Def'),
),
],
labelType: NavigationRailLabelType.none,
),
);

final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.favorite_border)));
await tester.pumpAndSettle();

final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
const Rect indicatorRect = Rect.fromLTRB(-3.0, 6.0, 53.0, 38.0);
const Rect includedRect = indicatorRect;
final Rect excludedRect = includedRect.inflate(10);

expect(
inkFeatures,
paints
..clipPath(
pathMatcher: isPathThat(
includes: <Offset>[
includedRect.centerLeft,
includedRect.topCenter,
includedRect.centerRight,
includedRect.bottomCenter,
],
excludes: <Offset>[
excludedRect.centerLeft,
excludedRect.topCenter,
excludedRect.centerRight,
excludedRect.bottomCenter,
],
),
)
..rect(
rect: indicatorRect,
color: const Color(0x0a6750a4),
)
..rrect(
rrect: RRect.fromLTRBR(0.0, 58.0, 50.0, 90.0, const Radius.circular(16)),
color: const Color(0xffe8def8),
),
);
});

testWidgets('NavigationRail indicator renders properly with custom padding and minWidth', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/117126
await _pumpNavigationRail(
tester,
navigationRail: NavigationRail(
minWidth: 300,
selectedIndex: 1,
extended: true,
destinations: const <NavigationRailDestination>[
NavigationRailDestination(
padding: EdgeInsets.all(10),
icon: Icon(Icons.favorite_border),
selectedIcon: Icon(Icons.favorite),
label: Text('Abc'),
),
NavigationRailDestination(
padding: EdgeInsets.all(18),
icon: Icon(Icons.bookmark_border),
selectedIcon: Icon(Icons.bookmark),
label: Text('Def'),
),
],
labelType: NavigationRailLabelType.none,
),
);

final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(tester.getCenter(find.byIcon(Icons.favorite_border)));
await tester.pumpAndSettle();

final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
const Rect indicatorRect = Rect.fromLTRB(132.0, 16.0, 188.0, 48.0);
const Rect includedRect = indicatorRect;
final Rect excludedRect = includedRect.inflate(10);

expect(
inkFeatures,
paints
..clipPath(
pathMatcher: isPathThat(
includes: <Offset>[
includedRect.centerLeft,
includedRect.topCenter,
includedRect.centerRight,
includedRect.bottomCenter,
],
excludes: <Offset>[
excludedRect.centerLeft,
excludedRect.topCenter,
excludedRect.centerRight,
excludedRect.bottomCenter,
],
),
)
..rect(
rect: indicatorRect,
color: const Color(0x0a6750a4),
)
..rrect(
rrect: RRect.fromLTRBR(140.0, 96.0, 196.0, 128.0, const Radius.circular(16)),
color: const Color(0xffe8def8),
),
);
});

testWidgets('NavigationRail indicator scale transform', (WidgetTester tester) async {
int selectedIndex = 0;
Future<void> buildWidget() async {
Expand Down

0 comments on commit 13f24e5

Please sign in to comment.