Skip to content

Commit 5ac7a6d

Browse files
Copilotbdlukaa
Copilot
and
bdlukaa
authored
fix: MenuFlyout sub-item tree direction and icon in RTL mode (#1313)
* Initial plan * fix: MenuFlyout sub-item tree uses correct direction in RTL mode Co-authored-by: bdlukaa <45696119+bdlukaa@users.noreply.github.com> * fix: remove reverse transition animation when closing MenuFlyout sub-menus Co-authored-by: bdlukaa <45696119+bdlukaa@users.noreply.github.com> Agent-Logs-Url: https://github.com/bdlukaa/fluent_ui/sessions/e65edcbe-e60d-4113-8f23-869dc2327c91 * fix: Disable reverse transitions by default --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bdlukaa <45696119+bdlukaa@users.noreply.github.com> Co-authored-by: Bruno D'Luka <brunodlukaa@gmail.com>
1 parent 6bc5bb6 commit 5ac7a6d

4 files changed

Lines changed: 226 additions & 24 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## 4.15.0
22

33
- fix: `MenuFlyout` no longer throws `TypeError` on sub-items ([#1337](https://github.com/bdlukaa/fluent_ui/issues/1337))
4+
- fix: `MenuFlyout` sub-item tree now correctly expands to the left and shows a `chevron_left` icon when right-to-left directionality is enabled ([#1342](https://github.com/bdlukaa/fluent_ui/issues/1342))
45
- feat: Controls now respond to `VisualDensity` from `FluentThemeData` for compact sizing. Use `FluentThemeData(visualDensity: VisualDensity.compact)` to enable compact mode ([#1175](https://github.com/bdlukaa/fluent_ui/issues/1175))
56
- fix: `NavigationView` no longer throws `BoxConstraints has a negative minimum height` when header and menu button are both absent ([#1334](https://github.com/bdlukaa/fluent_ui/issues/1334))
67
- fix: `ProgressBar` chooses the correct direction when directionality is right-to-left ([#1291](https://github.com/bdlukaa/fluent_ui/issues/1291))

‎lib/src/controls/flyouts/flyout.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ class FlyoutController with ChangeNotifier, WidgetsBindingObserver {
789789
NavigatorState? navigatorKey,
790790
FlyoutTransitionBuilder? transitionBuilder,
791791
Duration? transitionDuration,
792-
Duration? reverseTransitionDuration,
792+
Duration? reverseTransitionDuration = Duration.zero,
793793
Curve transitionCurve = Curves.linear,
794794
Offset? position,
795795
RouteSettings? settings,

‎lib/src/controls/flyouts/menu_flyout.dart‎

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -504,14 +504,31 @@ typedef MenuItemsBuilder =
504504
/// change between two states, checked or unchecked
505505
/// * [RadioMenuFlyoutItem], which represents a menu item that is mutually
506506
/// exclusive with other radio menu items in its group
507+
508+
/// The default trailing widget for [MenuFlyoutSubItem].
509+
///
510+
/// It shows a [WindowsIcons.chevron_right] icon in left-to-right mode and a
511+
/// [WindowsIcons.chevron_left] icon in right-to-left mode.
512+
class _MenuFlyoutSubItemChevron extends StatelessWidget {
513+
const _MenuFlyoutSubItemChevron();
514+
515+
@override
516+
Widget build(BuildContext context) {
517+
final isRtl = Directionality.of(context) == TextDirection.rtl;
518+
return WindowsIcon(
519+
isRtl ? WindowsIcons.chevron_left : WindowsIcons.chevron_right,
520+
);
521+
}
522+
}
523+
507524
class MenuFlyoutSubItem extends MenuFlyoutItem {
508525
/// Creates a menu flyout sub item
509526
MenuFlyoutSubItem({
510527
required super.text,
511528
required this.items,
512529
super.key,
513530
super.leading,
514-
super.trailing = const WindowsIcon(WindowsIcons.chevron_right),
531+
super.trailing = const _MenuFlyoutSubItemChevron(),
515532
this.showBehavior = SubItemShowAction.hover,
516533
this.showHoverDelay = const Duration(milliseconds: 450),
517534
}) : super(onPressed: null);
@@ -657,6 +674,7 @@ class _MenuFlyoutSubItemState extends State<_MenuFlyoutSubItem>
657674
parentRect: itemRect,
658675
parentSize: itemBox.size,
659676
margin: parent.margin,
677+
textDirection: Directionality.of(context),
660678
),
661679
child: Flyout(
662680
rootFlyout: parent.rootFlyout,
@@ -720,11 +738,13 @@ class _SubItemPositionDelegate extends SingleChildLayoutDelegate {
720738
final Rect parentRect;
721739
final Size parentSize;
722740
final double margin;
741+
final TextDirection textDirection;
723742

724743
const _SubItemPositionDelegate({
725744
required this.parentRect,
726745
required this.parentSize,
727746
required this.margin,
747+
required this.textDirection,
728748
});
729749

730750
@override
@@ -736,27 +756,51 @@ class _SubItemPositionDelegate extends SingleChildLayoutDelegate {
736756

737757
@override
738758
Offset getPositionForChild(Size rootSize, Size flyoutSize) {
739-
var x = parentRect.left + parentRect.size.width;
740-
741-
// if the flyout will overflow the screen on the right
742-
final willOverflowX = x + flyoutSize.width + margin > rootSize.width;
743-
744-
// if overflow x on the right, we check for some cases
745-
//
746-
// if the space available on the right is greater than the space available on
747-
// the left, use the right.
748-
//
749-
// otherwise, we position the flyout at the end of the screen
750-
if (willOverflowX) {
751-
final rightX = parentRect.left - flyoutSize.width;
752-
if (rightX > margin) {
753-
x = rightX;
754-
} else {
755-
x = clampDouble(
756-
rootSize.width - flyoutSize.width - margin,
757-
0,
758-
rootSize.width,
759-
);
759+
final isRtl = textDirection == TextDirection.rtl;
760+
double x;
761+
762+
if (isRtl) {
763+
// In RTL, the sub-menu should open to the left of the parent item by
764+
// default.
765+
x = parentRect.left - flyoutSize.width;
766+
767+
// if the flyout will overflow the screen on the left
768+
final willOverflowX = x < margin;
769+
770+
if (willOverflowX) {
771+
// try to the right of the parent item
772+
final rightX = parentRect.left + parentRect.size.width;
773+
if (rightX + flyoutSize.width + margin <= rootSize.width) {
774+
x = rightX;
775+
} else {
776+
x = clampDouble(margin, 0, rootSize.width);
777+
}
778+
}
779+
} else {
780+
// In LTR, the sub-menu should open to the right of the parent item by
781+
// default.
782+
x = parentRect.left + parentRect.size.width;
783+
784+
// if the flyout will overflow the screen on the right
785+
final willOverflowX = x + flyoutSize.width + margin > rootSize.width;
786+
787+
// if overflow x on the right, we check for some cases
788+
//
789+
// if the space available on the right is greater than the space available
790+
// on the left, use the right.
791+
//
792+
// otherwise, we position the flyout at the end of the screen
793+
if (willOverflowX) {
794+
final leftX = parentRect.left - flyoutSize.width;
795+
if (leftX > margin) {
796+
x = leftX;
797+
} else {
798+
x = clampDouble(
799+
rootSize.width - flyoutSize.width - margin,
800+
0,
801+
rootSize.width,
802+
);
803+
}
760804
}
761805
}
762806

@@ -775,6 +819,7 @@ class _SubItemPositionDelegate extends SingleChildLayoutDelegate {
775819
bool shouldRelayout(covariant _SubItemPositionDelegate oldDelegate) {
776820
return oldDelegate.parentRect != parentRect ||
777821
oldDelegate.parentSize != parentSize ||
778-
oldDelegate.margin != margin;
822+
oldDelegate.margin != margin ||
823+
oldDelegate.textDirection != textDirection;
779824
}
780825
}

‎test/flyout_test.dart‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,4 +287,160 @@ void main() {
287287
await gesture.removePointer();
288288
},
289289
);
290+
291+
testWidgets(
292+
'MenuFlyoutSubItem shows chevron_right trailing icon in LTR context',
293+
(tester) async {
294+
final controller = FlyoutController();
295+
296+
await tester.pumpWidget(
297+
wrapApp(
298+
child: Center(
299+
child: FlyoutTarget(
300+
controller: controller,
301+
child: const Text('Target'),
302+
),
303+
),
304+
),
305+
);
306+
307+
controller.showFlyout<void>(
308+
builder: (context) => MenuFlyout(
309+
items: [
310+
MenuFlyoutSubItem(
311+
text: const Text('Sub Menu'),
312+
items: (context) => [],
313+
),
314+
],
315+
),
316+
);
317+
await tester.pumpAndSettle();
318+
319+
// Find all WindowsIcon widgets in the tree and verify the sub-item
320+
// trailing uses chevron_right in LTR mode.
321+
final icons = tester.widgetList<WindowsIcon>(find.byType(WindowsIcon));
322+
expect(
323+
icons.any((icon) => icon.icon == WindowsIcons.chevron_right),
324+
isTrue,
325+
reason:
326+
'MenuFlyoutSubItem should show chevron_right trailing icon in LTR',
327+
);
328+
expect(
329+
icons.any((icon) => icon.icon == WindowsIcons.chevron_left),
330+
isFalse,
331+
reason:
332+
'MenuFlyoutSubItem should not show chevron_left trailing icon in LTR',
333+
);
334+
},
335+
);
336+
337+
testWidgets(
338+
'MenuFlyoutSubItem shows chevron_left trailing icon in RTL context',
339+
(tester) async {
340+
final controller = FlyoutController();
341+
342+
await tester.pumpWidget(
343+
FluentApp(
344+
home: Directionality(
345+
textDirection: TextDirection.rtl,
346+
child: Center(
347+
child: FlyoutTarget(
348+
controller: controller,
349+
child: const Text('Target'),
350+
),
351+
),
352+
),
353+
),
354+
);
355+
356+
controller.showFlyout<void>(
357+
builder: (context) => MenuFlyout(
358+
items: [
359+
MenuFlyoutSubItem(
360+
text: const Text('Sub Menu'),
361+
items: (context) => [],
362+
),
363+
],
364+
),
365+
);
366+
await tester.pumpAndSettle();
367+
368+
// Find all WindowsIcon widgets in the tree and verify the sub-item
369+
// trailing uses chevron_left in RTL mode.
370+
final icons = tester.widgetList<WindowsIcon>(find.byType(WindowsIcon));
371+
expect(
372+
icons.any((icon) => icon.icon == WindowsIcons.chevron_left),
373+
isTrue,
374+
reason:
375+
'MenuFlyoutSubItem should show chevron_left trailing icon in RTL',
376+
);
377+
expect(
378+
icons.any((icon) => icon.icon == WindowsIcons.chevron_right),
379+
isFalse,
380+
reason:
381+
'MenuFlyoutSubItem should not show chevron_right trailing icon in RTL',
382+
);
383+
},
384+
);
385+
386+
testWidgets(
387+
'MenuFlyoutSubItem sub-menu opens to the left in RTL context',
388+
(tester) async {
389+
final controller = FlyoutController();
390+
391+
await tester.pumpWidget(
392+
FluentApp(
393+
home: Directionality(
394+
textDirection: TextDirection.rtl,
395+
child: Center(
396+
child: FlyoutTarget(
397+
controller: controller,
398+
child: const Text('Target'),
399+
),
400+
),
401+
),
402+
),
403+
);
404+
405+
controller.showFlyout<void>(
406+
builder: (context) => MenuFlyout(
407+
items: [
408+
MenuFlyoutSubItem(
409+
text: const Text('Sub Menu'),
410+
items: (context) => [
411+
MenuFlyoutItem(
412+
text: const Text('Sub Item 1'),
413+
onPressed: () {},
414+
),
415+
],
416+
),
417+
],
418+
),
419+
);
420+
await tester.pumpAndSettle();
421+
422+
expect(find.text('Sub Menu'), findsOneWidget);
423+
final subMenuParentRect = tester.getRect(find.text('Sub Menu'));
424+
425+
// Hover over the sub-menu item to trigger its display
426+
final gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
427+
await gesture.addPointer(location: Offset.zero);
428+
await gesture.moveTo(tester.getCenter(find.text('Sub Menu')));
429+
await tester.pumpAndSettle(const Duration(milliseconds: 500));
430+
431+
// The sub-menu should now be showing
432+
expect(find.text('Sub Item 1'), findsOneWidget);
433+
434+
// In RTL mode, the sub-menu should appear to the LEFT of the parent item.
435+
final subItemRect = tester.getRect(find.text('Sub Item 1'));
436+
expect(
437+
subItemRect.left,
438+
lessThan(subMenuParentRect.left),
439+
reason:
440+
'In RTL, sub-menu should open to the left of the parent item',
441+
);
442+
443+
await gesture.removePointer();
444+
},
445+
);
290446
}

0 commit comments

Comments
 (0)