From ceeaf98e64703a50cf3cfcf0bd103de98b979a45 Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Thu, 8 Jun 2023 22:32:27 +0200 Subject: [PATCH] Fix navigation rail with long labels misplaced highlights (#128324) --- .../lib/src/material/navigation_rail.dart | 11 ++- .../test/material/navigation_rail_test.dart | 75 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/material/navigation_rail.dart b/packages/flutter/lib/src/material/navigation_rail.dart index 646e6eaefbe..590da9ec2ab 100644 --- a/packages/flutter/lib/src/material/navigation_rail.dart +++ b/packages/flutter/lib/src/material/navigation_rail.dart @@ -585,6 +585,7 @@ class _RailDestination extends StatelessWidget { final bool material3 = theme.useMaterial3; final EdgeInsets destinationPadding = (padding ?? EdgeInsets.zero).resolve(Directionality.of(context)); Offset indicatorOffset; + bool applyXOffset = false; final Widget themedIcon = IconTheme( data: disabled @@ -645,6 +646,7 @@ class _RailDestination extends StatelessWidget { ); } else { final Animation labelFadeAnimation = extendedTransitionAnimation.drive(CurveTween(curve: const Interval(0.0, 0.25))); + applyXOffset = true; content = Padding( padding: padding ?? EdgeInsets.zero, child: ConstrainedBox( @@ -775,6 +777,7 @@ class _RailDestination extends StatelessWidget { hoverColor: colors.primary.withOpacity(0.04), useMaterial3: material3, indicatorOffset: indicatorOffset, + applyXOffset: applyXOffset, child: content, ), ), @@ -797,6 +800,7 @@ class _IndicatorInkWell extends InkResponse { super.hoverColor, required this.useMaterial3, required this.indicatorOffset, + required this.applyXOffset, }) : super( containedInkWell: true, highlightShape: BoxShape.rectangle, @@ -805,14 +809,19 @@ class _IndicatorInkWell extends InkResponse { ); final bool useMaterial3; + // The offset used to position Ink highlight. final Offset indicatorOffset; + // Whether the horizontal offset from indicatorOffset should be used to position Ink highlight. + // If true, Ink highlight uses the indicator horizontal offset. If false, Ink highlight is centered horizontally. + final bool applyXOffset; @override RectCallback? getRectCallback(RenderBox referenceBox) { if (useMaterial3) { + final double indicatorHorizontalCenter = applyXOffset ? indicatorOffset.dx : referenceBox.size.width / 2; return () { return Rect.fromLTWH( - indicatorOffset.dx - (_kCircularIndicatorDiameter / 2), + indicatorHorizontalCenter - (_kCircularIndicatorDiameter / 2), indicatorOffset.dy, _kCircularIndicatorDiameter, _kIndicatorHeight, diff --git a/packages/flutter/test/material/navigation_rail_test.dart b/packages/flutter/test/material/navigation_rail_test.dart index ccfaa9947aa..fb0086b9d24 100644 --- a/packages/flutter/test/material/navigation_rail_test.dart +++ b/packages/flutter/test/material/navigation_rail_test.dart @@ -3048,6 +3048,81 @@ void main() { ); }); + testWidgets('NavigationRail indicator renders properly with long labels', (WidgetTester tester) async { + // This is a regression test for https://github.com/flutter/flutter/issues/128005. + await _pumpNavigationRail( + tester, + navigationRail: NavigationRail( + selectedIndex: 1, + destinations: const [ + NavigationRailDestination( + icon: Icon(Icons.favorite_border), + selectedIcon: Icon(Icons.favorite), + label: Text('ABCDEFGHIJKLMNOPQRSTUVWXYZ'), + ), + NavigationRailDestination( + icon: Icon(Icons.bookmark_border), + selectedIcon: Icon(Icons.bookmark), + label: Text('ABC'), + ), + ], + labelType: NavigationRailLabelType.all, + ), + ); + + 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'); + + // Default values from M3 specification. + const double indicatorHeight = 32.0; + const double destinationWidth = 72.0; + const double destinationHorizontalPadding = 8.0; + const double indicatorWidth = destinationWidth - 2 * destinationHorizontalPadding; // 56.0 + + // The navigation rail width is larger than default because of the first destination long label. + final double railWidth = tester.getSize(find.byType(NavigationRail)).width; + + // Expected indicator position. + final double indicatorLeft = (railWidth - indicatorWidth) / 2; + final double indicatorRight = (railWidth + indicatorWidth) / 2; + final Rect indicatorRect = Rect.fromLTRB(indicatorLeft, 0.0, indicatorRight, indicatorHeight); + final 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(indicatorLeft, 72.0, indicatorRight, 104.0, const Radius.circular(16)), + color: const Color(0xffe8def8), + ), + ); + }); + testWidgets('NavigationRail indicator scale transform', (WidgetTester tester) async { int selectedIndex = 0; Future buildWidget() async {