Unverified Commit 2a7ad01e authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub

Fix navigation rail hover misplaced when direction is RTL and extended is true (#134815)

## Description

This PR fixes `NavigationRail` hover position when text direction is set to RTL and `NavigationRail.extended` is true.

## Related Issue

Fixes https://github.com/flutter/flutter/issues/134361.

## Tests

Adds 1 test.
parent 0b540a87
...@@ -581,9 +581,9 @@ class _RailDestination extends StatelessWidget { ...@@ -581,9 +581,9 @@ class _RailDestination extends StatelessWidget {
); );
final ThemeData theme = Theme.of(context); final ThemeData theme = Theme.of(context);
final TextDirection textDirection = Directionality.of(context);
final bool material3 = theme.useMaterial3; final bool material3 = theme.useMaterial3;
final EdgeInsets destinationPadding = (padding ?? EdgeInsets.zero).resolve(Directionality.of(context)); final EdgeInsets destinationPadding = (padding ?? EdgeInsets.zero).resolve(textDirection);
Offset indicatorOffset; Offset indicatorOffset;
bool applyXOffset = false; bool applyXOffset = false;
...@@ -798,6 +798,7 @@ class _RailDestination extends StatelessWidget { ...@@ -798,6 +798,7 @@ class _RailDestination extends StatelessWidget {
useMaterial3: material3, useMaterial3: material3,
indicatorOffset: indicatorOffset, indicatorOffset: indicatorOffset,
applyXOffset: applyXOffset, applyXOffset: applyXOffset,
textDirection: textDirection,
child: content, child: content,
), ),
), ),
...@@ -821,6 +822,7 @@ class _IndicatorInkWell extends InkResponse { ...@@ -821,6 +822,7 @@ class _IndicatorInkWell extends InkResponse {
required this.useMaterial3, required this.useMaterial3,
required this.indicatorOffset, required this.indicatorOffset,
required this.applyXOffset, required this.applyXOffset,
required this.textDirection,
}) : super( }) : super(
containedInkWell: true, containedInkWell: true,
highlightShape: BoxShape.rectangle, highlightShape: BoxShape.rectangle,
...@@ -829,16 +831,25 @@ class _IndicatorInkWell extends InkResponse { ...@@ -829,16 +831,25 @@ class _IndicatorInkWell extends InkResponse {
); );
final bool useMaterial3; final bool useMaterial3;
// The offset used to position Ink highlight. // The offset used to position Ink highlight.
final Offset indicatorOffset; final Offset indicatorOffset;
// Whether the horizontal offset from indicatorOffset should be used to position Ink highlight. // 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. // If true, Ink highlight uses the indicator horizontal offset. If false, Ink highlight is centered horizontally.
final bool applyXOffset; final bool applyXOffset;
// The text direction used to adjust the indicator horizontal offset.
final TextDirection textDirection;
@override @override
RectCallback? getRectCallback(RenderBox referenceBox) { RectCallback? getRectCallback(RenderBox referenceBox) {
if (useMaterial3) { if (useMaterial3) {
final double indicatorHorizontalCenter = applyXOffset ? indicatorOffset.dx : referenceBox.size.width / 2; final double boxWidth = referenceBox.size.width;
double indicatorHorizontalCenter = applyXOffset ? indicatorOffset.dx : boxWidth / 2;
if (textDirection == TextDirection.rtl) {
indicatorHorizontalCenter = boxWidth - indicatorHorizontalCenter;
}
return () { return () {
return Rect.fromLTWH( return Rect.fromLTWH(
indicatorHorizontalCenter - (_kCircularIndicatorDiameter / 2), indicatorHorizontalCenter - (_kCircularIndicatorDiameter / 2),
......
...@@ -3276,6 +3276,105 @@ void main() { ...@@ -3276,6 +3276,105 @@ void main() {
); );
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/99933 }, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/99933
testWidgetsWithLeakTracking('NavigationRail indicator renders properly when text direction is rtl', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/134361.
await tester.pumpWidget(_buildWidget(
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'),
),
],
),
isRTL: true,
));
// Hover the first destination.
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 railMinExtendedWidth = 256.0;
const double indicatorHeight = 32.0;
const double destinationWidth = 72.0;
const double destinationHorizontalPadding = 8.0;
const double indicatorWidth = destinationWidth - 2 * destinationHorizontalPadding; // 56.0
const double verticalSpacer = 8.0;
const double verticalDestinationSpacingM3 = 12.0;
// The navigation rail width is the default one because labels are short.
final double railWidth = tester.getSize(find.byType(NavigationRail)).width;
expect(railWidth, railMinExtendedWidth);
// Expected indicator position.
final double indicatorLeft = railWidth - (destinationWidth - destinationHorizontalPadding / 2);
final double indicatorRight = indicatorLeft + indicatorWidth;
final Rect indicatorRect = Rect.fromLTRB(
indicatorLeft,
verticalDestinationSpacingM3 / 2,
indicatorRight,
verticalDestinationSpacingM3 / 2 + indicatorHeight,
);
final Rect includedRect = indicatorRect;
final Rect excludedRect = includedRect.inflate(10);
// Compute the vertical position for the selected destination (the one with 'bookmark' icon).
const double destinationHeight = indicatorHeight + verticalDestinationSpacingM3;
const double secondDestinationVerticalOffset = verticalSpacer + destinationHeight;
const double secondIndicatorVerticalOffset = secondDestinationVerticalOffset + verticalDestinationSpacingM3 / 2;
const double secondDestinationHorizontalOffset = 800 - railMinExtendedWidth; // RTL.
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,
],
),
)
// Hover highlight for the hovered destination (the one with 'favorite' icon).
..rect(
rect: indicatorRect,
color: const Color(0x0a6750a4),
)
// Indicator for the selected destination (the one with 'bookmark' icon).
..rrect(
rrect: RRect.fromLTRBR(
secondDestinationHorizontalOffset + indicatorLeft,
secondIndicatorVerticalOffset,
secondDestinationHorizontalOffset + indicatorRight,
secondIndicatorVerticalOffset + indicatorHeight,
const Radius.circular(16),
),
color: const Color(0xffe8def8),
),
);
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/99933
testWidgetsWithLeakTracking('NavigationRail indicator scale transform', (WidgetTester tester) async { testWidgetsWithLeakTracking('NavigationRail indicator scale transform', (WidgetTester tester) async {
int selectedIndex = 0; int selectedIndex = 0;
Future<void> buildWidget() async { Future<void> buildWidget() async {
......
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