From 5fcb48d59884eed4073b200f10769f9944b57fc6 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Mon, 19 Dec 2022 20:45:30 +0200 Subject: [PATCH] Fix `NavigationRail` highlight (#117320) --- .../lib/src/material/navigation_rail.dart | 43 ++- .../test/material/navigation_rail_test.dart | 258 ++++++++++++++++++ 2 files changed, 286 insertions(+), 15 deletions(-) diff --git a/packages/flutter/lib/src/material/navigation_rail.dart b/packages/flutter/lib/src/material/navigation_rail.dart index 6beed46c555d..27375063d201 100644 --- a/packages/flutter/lib/src/material/navigation_rail.dart +++ b/packages/flutter/lib/src/material/navigation_rail.dart @@ -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 @@ -590,7 +591,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, @@ -607,8 +609,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: [ if (spacing != null) spacing, @@ -686,8 +690,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, @@ -730,7 +738,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, @@ -772,7 +785,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, ), ), @@ -794,7 +807,7 @@ class _IndicatorInkWell extends InkResponse { super.splashColor, super.hoverColor, required this.useMaterial3, - required this.indicatorOffsetY, + required this.indicatorOffset, }) : super( containedInkWell: true, highlightShape: BoxShape.rectangle, @@ -803,18 +816,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, ); }; } @@ -984,6 +996,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 { diff --git a/packages/flutter/test/material/navigation_rail_test.dart b/packages/flutter/test/material/navigation_rail_test.dart index cd040e7fda07..409227dcdf1a 100644 --- a/packages/flutter/test/material/navigation_rail_test.dart +++ b/packages/flutter/test/material/navigation_rail_test.dart @@ -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( + 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: [ + includedRect.centerLeft, + includedRect.topCenter, + includedRect.centerRight, + includedRect.bottomCenter, + ], + excludes: [ + 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( + 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: [ + includedRect.centerLeft, + includedRect.topCenter, + includedRect.centerRight, + includedRect.bottomCenter, + ], + excludes: [ + 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( + 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: [ + includedRect.centerLeft, + includedRect.topCenter, + includedRect.centerRight, + includedRect.bottomCenter, + ], + excludes: [ + 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( + 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: [ + includedRect.centerLeft, + includedRect.topCenter, + includedRect.centerRight, + includedRect.bottomCenter, + ], + excludes: [ + 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 buildWidget() async {