From 392cd99e3b95405e16fa8b3f761c3c0626f1213c Mon Sep 17 00:00:00 2001 From: Viktor Lidholt Date: Tue, 20 Oct 2026 14:22:16 +0200 Subject: [PATCH] fix(ui): keep popup and context menus inside the window (T-3) Dropdowns, the menubar and submenus placed their menus with shad's manual ShadAnchor, which never moves an overlay back onto the screen: the sheet tab menu ran off the bottom edge and start-aligned menus off the right. WonkyMenuPlacement turns those anchors into auto anchors that open on the other side of the trigger when the menu does not fit and shift sideways at the window edges; menus taller than the room beside their trigger or pointer scroll. Write's own popup host is replaced by WonkyPopupMenuHost, and dropdown-like popovers anchored with ShadAnchor use the same placement. Co-Authored-By: Claude Opus 5.5 --- .../lib/src/board/pages/board_search.dart | 10 +- packages/wonky_app/lib/src/drive/top_bar.dart | 11 +- .../notifications/notifications_button.dart | 10 +- .../lib/src/widgets/menu/dropdown_menu.dart | 58 +++- .../lib/src/widgets/menu/menu_items.dart | 85 ++++-- .../lib/src/widgets/menu/menu_placement.dart | 103 +++++++ .../lib/src/widgets/menu/popup_menu_host.dart | 5 +- .../lib/src/widgets/menubar/menubar.dart | 20 ++ packages/wonky_ui/lib/wonky_ui.dart | 1 + .../test/widgets/menu_placement_test.dart | 282 ++++++++++++++++++ specs/02-design-system.md | 15 + .../features/pivot/ui/pivot_field_list.dart | 11 +- .../lib/features/screen/document_menus.dart | 103 ------- .../screen/write_document_screen.dart | 6 +- 14 files changed, 571 insertions(+), 149 deletions(-) create mode 100644 packages/wonky_ui/lib/src/widgets/menu/menu_placement.dart create mode 100644 packages/wonky_ui/test/widgets/menu_placement_test.dart diff --git a/packages/wonky_app/lib/src/board/pages/board_search.dart b/packages/wonky_app/lib/src/board/pages/board_search.dart index 50af5a99..5ebea1e3 100644 --- a/packages/wonky_app/lib/src/board/pages/board_search.dart +++ b/packages/wonky_app/lib/src/board/pages/board_search.dart @@ -38,10 +38,12 @@ class BoardSearchButton extends StatelessWidget { return ShadPopover( controller: controller, padding: EdgeInsets.zero, - anchor: const ShadAnchor( - childAlignment: AlignmentDirectional.bottomEnd, - overlayAlignment: AlignmentDirectional.topEnd, - offset: Offset(0, 6), + anchor: WonkyMenuPlacement.onScreen( + const ShadAnchor( + childAlignment: AlignmentDirectional.bottomEnd, + overlayAlignment: AlignmentDirectional.topEnd, + offset: Offset(0, 6), + ), ), popover: (context) => BoardSearchPanel( services: services, diff --git a/packages/wonky_app/lib/src/drive/top_bar.dart b/packages/wonky_app/lib/src/drive/top_bar.dart index cbda881d..f2c8c827 100644 --- a/packages/wonky_app/lib/src/drive/top_bar.dart +++ b/packages/wonky_app/lib/src/drive/top_bar.dart @@ -4,6 +4,7 @@ import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:shad/shad.dart'; import 'package:wonky_client/wonky_client.dart'; +import 'package:wonky_ui/wonky_ui.dart'; import '../bootstrap/app_urls.dart'; import '../branding/product_icon.dart'; @@ -219,10 +220,12 @@ class _DriveSearchFieldState extends State { return ShadPopover( controller: _popover, padding: const EdgeInsets.all(4), - anchor: const ShadAnchor( - childAlignment: AlignmentDirectional.bottomStart, - overlayAlignment: AlignmentDirectional.topStart, - offset: Offset(0, 4), + anchor: WonkyMenuPlacement.onScreen( + const ShadAnchor( + childAlignment: AlignmentDirectional.bottomStart, + overlayAlignment: AlignmentDirectional.topStart, + offset: Offset(0, 4), + ), ), popover: (context) { final q = _text.text.trim(); diff --git a/packages/wonky_app/lib/src/notifications/notifications_button.dart b/packages/wonky_app/lib/src/notifications/notifications_button.dart index 58d0b89f..2373cdba 100644 --- a/packages/wonky_app/lib/src/notifications/notifications_button.dart +++ b/packages/wonky_app/lib/src/notifications/notifications_button.dart @@ -125,10 +125,12 @@ class _NotificationsButtonState extends State { final l = WonkyAppLocalizations.of(context); return ShadPopover( controller: _popover, - anchor: const ShadAnchor( - childAlignment: AlignmentDirectional.bottomEnd, - overlayAlignment: AlignmentDirectional.topEnd, - offset: Offset(0, 6), + anchor: WonkyMenuPlacement.onScreen( + const ShadAnchor( + childAlignment: AlignmentDirectional.bottomEnd, + overlayAlignment: AlignmentDirectional.topEnd, + offset: Offset(0, 6), + ), ), popover: (context) => NotificationsPanel( controller: _ctl, diff --git a/packages/wonky_ui/lib/src/widgets/menu/dropdown_menu.dart b/packages/wonky_ui/lib/src/widgets/menu/dropdown_menu.dart index 41b7146e..5a2c1a5b 100644 --- a/packages/wonky_ui/lib/src/widgets/menu/dropdown_menu.dart +++ b/packages/wonky_ui/lib/src/widgets/menu/dropdown_menu.dart @@ -3,6 +3,7 @@ import 'package:shad/shad.dart'; import 'menu_entry.dart'; import 'menu_items.dart'; +import 'menu_placement.dart'; import 'open_menu.dart'; /// Builds the trigger of a [WonkyDropdownMenu]. @@ -30,7 +31,11 @@ class WonkyDropdownMenu extends StatefulWidget { final WonkyDropdownTriggerBuilder builder; final ShadContextMenuController? controller; - /// Defaults to below the trigger, start-aligned. + /// Defaults to below the trigger, start-aligned. A `ShadAnchor` is kept on + /// screen: the menu opens on the other side of the trigger when it does + /// not fit and shifts sideways at the window edges + /// ([WonkyMenuPlacement.onScreen]); menus taller than the room beside the + /// trigger scroll. final ShadAnchorBase? anchor; final BoxConstraints? constraints; final ValueChanged? onOpenChanged; @@ -69,6 +74,25 @@ class _WonkyDropdownMenuState extends State { super.dispose(); } + static const _defaultAnchor = ShadAnchor( + childAlignment: AlignmentDirectional.bottomStart, + overlayAlignment: AlignmentDirectional.topStart, + offset: Offset(0, 4), + ); + + /// The room beside the trigger: the menu opens below or above it, so it + /// may be as tall as the larger side. + double? _maxHeight(ShadAnchorBase anchor) { + final rect = WonkyMenuPlacement.rectOf(context); + if (rect == null) return null; + final gap = switch (anchor) { + ShadAnchor(:final offset) || + ShadAnchorAuto(:final offset) => offset.dy.abs(), + ShadGlobalAnchor() => 0.0, + }; + return WonkyMenuPlacement.itemsMaxHeightBeside(context, rect, gap: gap); + } + void _onChanged() { final open = _controller.isOpen; if (open != _wasOpen) { @@ -85,23 +109,22 @@ class _WonkyDropdownMenuState extends State { @override Widget build(BuildContext context) { + final anchor = widget.anchor ?? _defaultAnchor; final items = _controller.isOpen ? buildWonkyMenuItems( context, widget.entries(), onClose: _controller.hide, requestFocus: true, + maxHeight: () => _maxHeight(anchor), ) : const [SizedBox.shrink()]; return ShadContextMenu( controller: _controller, - anchor: - widget.anchor ?? - const ShadAnchor( - childAlignment: AlignmentDirectional.bottomStart, - overlayAlignment: AlignmentDirectional.topStart, - offset: Offset(0, 4), - ), + // A manual anchor never moves the menu back onto the screen. + anchor: anchor is ShadAnchor + ? WonkyMenuPlacement.onScreen(anchor) + : anchor, constraints: widget.constraints ?? const BoxConstraints(minWidth: 200), items: items, onTapOutside: (_) => _controller.hide(), @@ -159,6 +182,11 @@ class _LazyContextMenuRegion extends StatefulWidget { class _LazyContextMenuRegionState extends State<_LazyContextMenuRegion> { final _controller = ShadContextMenuController(); + /// Where the last pointer went down: the menu opens there (shad flips it + /// above the pointer when it does not fit below and keeps it on screen + /// horizontally), so it may be as tall as the larger side. + Offset? _pointer; + @override void initState() { super.initState(); @@ -181,6 +209,16 @@ class _LazyContextMenuRegionState extends State<_LazyContextMenuRegion> { super.dispose(); } + double? _maxHeight() { + final at = _pointer; + if (at == null || !mounted) return null; + return WonkyMenuPlacement.itemsMaxHeight( + MediaQuery.sizeOf(context).height, + below: at.dy, + above: at.dy, + ); + } + @override Widget build(BuildContext context) { return ShadContextMenuRegion( @@ -192,12 +230,16 @@ class _LazyContextMenuRegionState extends State<_LazyContextMenuRegion> { context, widget.entries(), onClose: _controller.hide, + maxHeight: _maxHeight, ) else const SizedBox.shrink(), ], child: Listener( + // Also over empty space, where shad's detector opens the menu too. + behavior: HitTestBehavior.translucent, onPointerDown: (e) { + _pointer = e.position; if (e.buttons == 2) widget.onOpen?.call(); }, child: widget.child, diff --git a/packages/wonky_ui/lib/src/widgets/menu/menu_items.dart b/packages/wonky_ui/lib/src/widgets/menu/menu_items.dart index ae40f044..16aa9cfa 100644 --- a/packages/wonky_ui/lib/src/widgets/menu/menu_items.dart +++ b/packages/wonky_ui/lib/src/widgets/menu/menu_items.dart @@ -3,6 +3,7 @@ import 'package:flutter/widgets.dart'; import 'package:shad/shad.dart'; import 'menu_entry.dart'; +import 'menu_placement.dart'; /// Renders [entries] as shad context-menu items (for `ShadContextMenu`, /// `ShadContextMenuRegion` and `ShadMenubarItem`). @@ -13,11 +14,30 @@ import 'menu_entry.dart'; /// /// With [requestFocus], the menu takes keyboard focus when it opens (↑/↓, /// Home/End move between items; Enter activates). +/// +/// [maxHeight], evaluated when the menu builds, limits the items' height and +/// taller menus scroll (`WonkyMenuPlacement.itemsMaxHeight`). Submenus are +/// limited to the room beside their item and open where they fit. List buildWonkyMenuItems( BuildContext context, List entries, { VoidCallback? onClose, bool requestFocus = false, + double? Function()? maxHeight, +}) => _menuItems( + entries, + onClose: onClose, + requestFocus: requestFocus, + maxHeight: maxHeight, + nested: false, +); + +List _menuItems( + List entries, { + required VoidCallback? onClose, + required bool requestFocus, + required double? Function()? maxHeight, + required bool nested, }) { final tidy = entries.tidy(); if (tidy.isEmpty) return const []; @@ -26,6 +46,8 @@ List buildWonkyMenuItems( entries: tidy, onClose: onClose, requestFocus: requestFocus, + maxHeight: maxHeight, + nested: nested, ), ]; } @@ -35,11 +57,17 @@ class _TypeAheadMenuBody extends StatefulWidget { required this.entries, this.onClose, this.requestFocus = false, + this.maxHeight, + this.nested = false, }); final List entries; final VoidCallback? onClose; final bool requestFocus; + final double? Function()? maxHeight; + + /// Whether this body is a submenu (its own submenus open nested). + final bool nested; @override State<_TypeAheadMenuBody> createState() => _TypeAheadMenuBodyState(); @@ -178,15 +206,19 @@ class _TypeAheadMenuBodyState extends State<_TypeAheadMenuBody> { ), ); } - return Focus( - focusNode: _root, - onKeyEvent: _onKey, - child: Column( - mainAxisSize: MainAxisSize.min, - crossAxisAlignment: CrossAxisAlignment.stretch, - children: children, - ), + Widget body = Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.stretch, + children: children, ); + final maxHeight = widget.maxHeight?.call(); + if (maxHeight != null) { + body = ConstrainedBox( + constraints: BoxConstraints(maxHeight: maxHeight), + child: SingleChildScrollView(child: body), + ); + } + return Focus(focusNode: _root, onKeyEvent: _onKey, child: body); } Widget _buildEntry(BuildContext context, WonkyMenuEntry e, bool leading) { @@ -220,17 +252,24 @@ class _TypeAheadMenuBodyState extends State<_TypeAheadMenuBody> { return _stableEnabled( e.key, e.enabled, - (key) => ShadContextMenuItem( + // The builder's context finds the item's box: the submenu is as + // tall as the room beside the item allows. + (key) => Builder( key: key, - enabled: e.enabled, - leading: lead(e.icon), - trailing: const Icon(LucideIcons.chevronRight, size: 16), - items: buildWonkyMenuItems( - context, - e.children, - onClose: widget.onClose, + builder: (itemContext) => ShadContextMenuItem( + enabled: e.enabled, + anchor: WonkyMenuPlacement.submenu(nested: widget.nested), + leading: lead(e.icon), + trailing: const Icon(LucideIcons.chevronRight, size: 16), + items: _menuItems( + e.children, + onClose: widget.onClose, + requestFocus: false, + maxHeight: () => _submenuMaxHeight(itemContext), + nested: true, + ), + child: Text(e.label), ), - child: Text(e.label), ), ); case WonkyMenuItem(): @@ -305,6 +344,18 @@ class _TypeAheadMenuBodyState extends State<_TypeAheadMenuBody> { } } +/// The tallest the items of the submenu of the item at [itemContext] may be: +/// it opens level with the item's top, or ends level with its bottom. +double? _submenuMaxHeight(BuildContext itemContext) { + final rect = WonkyMenuPlacement.rectOf(itemContext); + if (rect == null) return null; + return WonkyMenuPlacement.itemsMaxHeight( + MediaQuery.sizeOf(itemContext).height, + below: rect.top, + above: rect.bottom, + ); +} + /// Builds a shad menu item whose element is recreated when [enabled] /// changes. /// diff --git a/packages/wonky_ui/lib/src/widgets/menu/menu_placement.dart b/packages/wonky_ui/lib/src/widgets/menu/menu_placement.dart new file mode 100644 index 00000000..34393034 --- /dev/null +++ b/packages/wonky_ui/lib/src/widgets/menu/menu_placement.dart @@ -0,0 +1,103 @@ +import 'dart:math' as math; + +import 'package:flutter/widgets.dart'; +import 'package:shad/shad.dart'; + +/// Keeps menus (and dropdown-like popovers) inside the window (suite 02 +/// §4.2): a menu opens where its anchor says when it fits, opens on the +/// other side of its trigger (above instead of below) when it does not, +/// shifts sideways to stay on screen, and scrolls when it is taller than +/// the room beside its trigger. +abstract final class WonkyMenuPlacement { + /// Space kept between a menu and the screen edges. + static const margin = 8.0; + + /// shad's vertical menu padding (4 px top and bottom) plus its 1 px border. + static const chrome = 10.0; + + /// The least a menu's items may be limited to, so that tiny windows still + /// show a few items (and scroll). + static const minItemsHeight = 120.0; + + /// [anchor] as an anchor that keeps the overlay on screen. + /// + /// shad's `ShadAnchor` glues the overlay to its trigger and never moves it: + /// a menu below a trigger at the bottom of the window (the sheet tab menu) + /// or start-aligned near the right edge ran off the screen. The returned + /// anchor places the overlay as [anchor] does when it fits, mirrors it to + /// the other side of the trigger (with the offset mirrored too) when it + /// does not fit vertically, and shifts it horizontally to stay inside the + /// window. + static ShadAnchorAuto onScreen(ShadAnchor anchor) => ShadAnchorAuto( + targetAnchor: anchor.childAlignment, + // shad's auto anchor names the side the overlay extends to; the manual + // anchor names the overlay's point glued to the trigger: the opposite. + followerAnchor: -anchor.overlayAlignment, + offset: anchor.offset, + fallback: ShadAnchorAuto( + targetAnchor: _flipY(anchor.childAlignment), + followerAnchor: -_flipY(anchor.overlayAlignment), + offset: Offset(anchor.offset.dx, -anchor.offset.dy), + ), + ); + + /// The anchor of a submenu: beside its item's end (as shad's default), and + /// ending at the item's bottom instead when there is no room below. + /// [nested] is for a submenu of a submenu (shad lifts those 2 px more). + static ShadAnchorAuto submenu({bool nested = false}) { + final dy = nested ? 5.0 : 3.0; + return ShadAnchorAuto( + offset: Offset(-8, -dy), + targetAnchor: Alignment.topRight, + followerAnchor: Alignment.bottomRight, + fallback: ShadAnchorAuto( + offset: Offset(-8, dy), + targetAnchor: Alignment.bottomRight, + followerAnchor: Alignment.topRight, + ), + ); + } + + /// The tallest a menu's items may be on a screen of height [screen] when + /// the menu opens downwards from [below] or, when there is more room + /// there, upwards ending at [above] (global y coordinates). Taller menus + /// scroll, so that they always fit on one side of their trigger. + static double itemsMaxHeight( + double screen, { + required double below, + required double above, + }) => math.max( + minItemsHeight, + math.max(screen - below, above) - margin - chrome, + ); + + /// [itemsMaxHeight] for a menu opening beside [anchor] (a trigger's global + /// rect) with [gap] between the two. + static double itemsMaxHeightBeside( + BuildContext context, + Rect anchor, { + double gap = 0, + }) => itemsMaxHeight( + MediaQuery.sizeOf(context).height, + below: anchor.bottom + gap, + above: anchor.top - gap, + ); + + /// The global rect of [context]'s render box, or null before its first + /// layout. + static Rect? rectOf(BuildContext context) { + if (!context.mounted) return null; + final box = context.findRenderObject(); + if (box is! RenderBox || !box.attached || !box.hasSize) return null; + return box.localToGlobal(Offset.zero) & box.size; + } + + static AlignmentGeometry _flipY(AlignmentGeometry a) => switch (a) { + Alignment(:final x, :final y) => Alignment(x, -y), + AlignmentDirectional(:final start, :final y) => AlignmentDirectional( + start, + -y, + ), + _ => a, + }; +} diff --git a/packages/wonky_ui/lib/src/widgets/menu/popup_menu_host.dart b/packages/wonky_ui/lib/src/widgets/menu/popup_menu_host.dart index 6362b027..f1961a47 100644 --- a/packages/wonky_ui/lib/src/widgets/menu/popup_menu_host.dart +++ b/packages/wonky_ui/lib/src/widgets/menu/popup_menu_host.dart @@ -5,6 +5,7 @@ import 'package:shad/shad.dart'; import 'menu_entry.dart'; import 'menu_items.dart'; +import 'menu_placement.dart'; /// Hosts programmatic menus (context menus) anchored at a global position /// inside [child]: `WonkyPopupMenuHost.maybeOf(context)?.show(pos, entries)`. @@ -44,10 +45,10 @@ class WonkyPopupMenuHostState extends State { final _items = GlobalKey(debugLabel: 'WonkyPopupMenuHost.items'); /// Space kept between the menu and the screen edges. - static const _margin = 8.0; + static const _margin = WonkyMenuPlacement.margin; /// shad's vertical menu padding (4 px top and bottom) plus its 1 px border. - static const _chrome = 10.0; + static const _chrome = WonkyMenuPlacement.chrome; /// The top of a menu of height [height] asked for at [pointerY] on a screen /// of height [screen] (03 §10): below the pointer when it fits, else ending diff --git a/packages/wonky_ui/lib/src/widgets/menubar/menubar.dart b/packages/wonky_ui/lib/src/widgets/menubar/menubar.dart index 4043874b..af0f2c84 100644 --- a/packages/wonky_ui/lib/src/widgets/menubar/menubar.dart +++ b/packages/wonky_ui/lib/src/widgets/menubar/menubar.dart @@ -3,6 +3,7 @@ import 'package:shad/shad.dart'; import '../menu/menu_entry.dart'; import '../menu/menu_items.dart'; +import '../menu/menu_placement.dart'; import '../menu/open_menu.dart'; /// One top-level menu of a [WonkyMenubar]. @@ -178,6 +179,21 @@ class _WonkyMenubarState extends State { if (mounted) setState(() {}); } + static final _anchor = WonkyMenuPlacement.onScreen( + const ShadAnchor( + offset: Offset(-4, 8), + childAlignment: AlignmentDirectional.bottomStart, + overlayAlignment: AlignmentDirectional.topStart, + ), + ); + + /// The menus open below the strip; long ones scroll. + double? _maxHeight() { + final rect = WonkyMenuPlacement.rectOf(context); + if (rect == null) return null; + return WonkyMenuPlacement.itemsMaxHeightBeside(context, rect, gap: 4); + } + @override Widget build(BuildContext context) { final controller = _controller.._menus = widget.menus; @@ -207,6 +223,9 @@ class _WonkyMenubarState extends State { controller: controller._popoverAt(i), height: widget.height - 4, buttonPadding: const EdgeInsets.symmetric(horizontal: 8), + // shad's default anchor, kept on screen (a menu at the + // end of a narrow strip ran off the right edge). + anchor: _anchor, constraints: const BoxConstraints(minWidth: 240), items: selected == i ? buildWonkyMenuItems( @@ -217,6 +236,7 @@ class _WonkyMenubarState extends State { // the keys (Escape, ↑/↓, type-ahead) instead of a // focused document editor behind it. requestFocus: true, + maxHeight: _maxHeight, ) : const [SizedBox.shrink()], child: Text( diff --git a/packages/wonky_ui/lib/wonky_ui.dart b/packages/wonky_ui/lib/wonky_ui.dart index 097553ff..3e456fc8 100644 --- a/packages/wonky_ui/lib/wonky_ui.dart +++ b/packages/wonky_ui/lib/wonky_ui.dart @@ -19,6 +19,7 @@ export 'src/widgets/font_picker/font_picker.dart'; export 'src/widgets/menu/dropdown_menu.dart'; export 'src/widgets/menu/menu_entry.dart'; export 'src/widgets/menu/menu_items.dart'; +export 'src/widgets/menu/menu_placement.dart'; export 'src/widgets/menu/open_menu.dart'; export 'src/widgets/menu/popup_menu_host.dart'; export 'src/widgets/menubar/menubar.dart'; diff --git a/packages/wonky_ui/test/widgets/menu_placement_test.dart b/packages/wonky_ui/test/widgets/menu_placement_test.dart new file mode 100644 index 00000000..0708a775 --- /dev/null +++ b/packages/wonky_ui/test/widgets/menu_placement_test.dart @@ -0,0 +1,282 @@ +import 'package:flutter/gestures.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shad/shad.dart'; +import 'package:wonky_ui/testing.dart'; +import 'package:wonky_ui/wonky_ui.dart'; + +const _screen = Size(1280, 800); +const _trigger = ValueKey('trigger'); + +List _items(int n, {String prefix = 'm'}) => [ + for (var i = 0; i < n; i++) + WonkyMenuItem( + key: ValueKey('$prefix.item$i'), + label: 'A rather long menu item $i', + onSelected: () {}, + ), +]; + +Widget _dropdown( + List Function() entries, { + ShadAnchorBase? anchor, +}) => WonkyDropdownMenu( + anchor: anchor, + entries: entries, + builder: (context, c) => ShadButton( + key: _trigger, + size: ShadButtonSize.sm, + onPressed: c.toggle, + child: const Text('Menu'), + ), +); + +/// Pumps [child] at [alignment] in a window of [size]. +Future _pumpAt( + WidgetTester tester, + Widget child, + Alignment alignment, { + Size size = _screen, + Brightness brightness = Brightness.light, +}) => pumpWonky( + tester, + Align(alignment: alignment, child: child), + size: size, + brightness: brightness, +); + +Future _openDropdown(WidgetTester tester) async { + await tester.tap(find.byKey(_trigger)); + await tester.pumpAndSettle(); +} + +/// The rendered box of the open menu that shows [item]: its popover surface +/// (border and padding included). +Rect _menuOf(WidgetTester tester, Finder item) => tester.getRect( + find.ancestor(of: item, matching: find.byType(FocusScope)).first, +); + +void _expectOnScreen(Rect menu, Size screen) { + expect(menu.left, greaterThanOrEqualTo(0), reason: 'menu $menu'); + expect(menu.top, greaterThanOrEqualTo(0), reason: 'menu $menu'); + expect(menu.right, lessThanOrEqualTo(screen.width), reason: 'menu $menu'); + expect(menu.bottom, lessThanOrEqualTo(screen.height), reason: 'menu $menu'); +} + +void main() { + group('Given a dropdown whose trigger is in the bottom-right corner', () { + for (final b in Brightness.values) { + testWidgets('(${b.name}) when it opens then it opens above the trigger, ' + 'shifted left, fully on screen and in the theme\'s menu colours', ( + tester, + ) async { + await _pumpAt( + tester, + _dropdown(() => _items(6)), + Alignment.bottomRight, + brightness: b, + ); + await _openDropdown(tester); + + final item = find.byKey(const ValueKey('m.item0')); + final menu = _menuOf(tester, item); + final trigger = tester.getRect(find.byKey(_trigger)); + _expectOnScreen(menu, _screen); + expect(menu.bottom, lessThanOrEqualTo(trigger.top)); + + final popover = ShadTheme.of(tester.element(item)).colorScheme.popover; + expect( + find.ancestor( + of: item, + matching: find.byWidgetPredicate( + (w) => w is ShadDecorator && w.decoration?.color == popover, + ), + ), + findsWidgets, + ); + }); + } + }); + + testWidgets('Given a dropdown at the bottom-left (the sheet tab menu) when ' + 'it opens then it opens upwards and stays on screen (it ran off the ' + 'bottom edge)', (tester) async { + await _pumpAt(tester, _dropdown(() => _items(8)), Alignment.bottomLeft); + await _openDropdown(tester); + + final menu = _menuOf(tester, find.byKey(const ValueKey('m.item0'))); + final trigger = tester.getRect(find.byKey(_trigger)); + _expectOnScreen(menu, _screen); + expect(menu.bottom, lessThanOrEqualTo(trigger.top)); + expect(menu.left, trigger.left); + }); + + testWidgets('Given a start-aligned dropdown near the right edge when it ' + 'opens then it still opens below the trigger, shifted left to stay on ' + 'screen', (tester) async { + await _pumpAt(tester, _dropdown(() => _items(3)), Alignment.topRight); + await _openDropdown(tester); + + final menu = _menuOf(tester, find.byKey(const ValueKey('m.item0'))); + final trigger = tester.getRect(find.byKey(_trigger)); + _expectOnScreen(menu, _screen); + expect(menu.top, greaterThanOrEqualTo(trigger.bottom)); + expect(menu.width, greaterThan(trigger.width)); + }); + + testWidgets('Given an end-aligned dropdown near the left edge when it opens ' + 'then it shifts right to stay on screen', (tester) async { + await _pumpAt( + tester, + _dropdown( + () => _items(3), + anchor: const ShadAnchor( + childAlignment: AlignmentDirectional.bottomEnd, + overlayAlignment: AlignmentDirectional.topEnd, + offset: Offset(0, 4), + ), + ), + Alignment.topLeft, + ); + await _openDropdown(tester); + + _expectOnScreen( + _menuOf(tester, find.byKey(const ValueKey('m.item0'))), + _screen, + ); + }); + + testWidgets('Given a dropdown taller than the room above and below its ' + 'trigger when it opens then it fits beside the trigger and scrolls to ' + 'its last item', (tester) async { + const size = Size(800, 400); + await _pumpAt( + tester, + _dropdown(() => _items(40)), + const Alignment(0, 0.2), + size: size, + ); + await _openDropdown(tester); + + final menu = _menuOf(tester, find.byKey(const ValueKey('m.item0'))); + final trigger = tester.getRect(find.byKey(_trigger)); + _expectOnScreen(menu, size); + // The larger side, above the trigger. + expect(menu.bottom, lessThanOrEqualTo(trigger.top)); + + final last = find.byKey(const ValueKey('m.item39')); + await tester.scrollUntilVisible( + last, + 100, + scrollable: find.byType(Scrollable).last, + ); + expect(tester.getRect(last).bottom, lessThanOrEqualTo(size.height)); + }); + + testWidgets('Given a dropdown near the bottom-right corner when a submenu ' + 'opens then the submenu stays on screen', (tester) async { + await _pumpAt( + tester, + _dropdown( + () => [ + ..._items(2), + WonkyMenuSubmenu( + key: const ValueKey('m.sub'), + label: 'More', + children: _items(12, prefix: 's'), + ), + ], + ), + Alignment.bottomRight, + ); + await _openDropdown(tester); + await tester.tap(find.byKey(const ValueKey('m.sub'))); + await tester.pumpAndSettle(); + + final sub = _menuOf(tester, find.byKey(const ValueKey('s.item0'))); + _expectOnScreen(sub, _screen); + }); + + for (final corner in [Alignment.bottomRight, Alignment.bottomLeft]) { + testWidgets('Given a context menu region when right-clicked near the ' + '$corner corner then the menu stays on screen', (tester) async { + await pumpWonky( + tester, + WonkyContextMenuRegion( + entries: () => _items(12), + child: const SizedBox.expand(), + ), + size: _screen, + ); + final at = + corner.alongSize(_screen) - corner.alongOffset(const Offset(10, 10)); + await tester.tapAt(at, buttons: kSecondaryButton); + await tester.pumpAndSettle(); + + _expectOnScreen( + _menuOf(tester, find.byKey(const ValueKey('m.item0'))), + _screen, + ); + }); + } + + testWidgets('Given a context menu taller than the room above and below the ' + 'pointer when right-clicked then it fits on screen and scrolls', ( + tester, + ) async { + const size = Size(800, 400); + await pumpWonky( + tester, + WonkyContextMenuRegion( + entries: () => _items(40), + child: const SizedBox.expand(), + ), + size: size, + ); + await tester.tapAt(const Offset(400, 220), buttons: kSecondaryButton); + await tester.pumpAndSettle(); + + _expectOnScreen( + _menuOf(tester, find.byKey(const ValueKey('m.item0'))), + size, + ); + }); + + group('Given WonkyMenuPlacement', () { + test('when mirroring a below-the-trigger anchor then the fallback opens ' + 'above it with the gap mirrored', () { + final a = WonkyMenuPlacement.onScreen( + const ShadAnchor( + childAlignment: AlignmentDirectional.bottomStart, + overlayAlignment: AlignmentDirectional.topStart, + offset: Offset(0, 4), + ), + ); + expect(a.targetAnchor, AlignmentDirectional.bottomStart); + expect(a.followerAnchor, AlignmentDirectional.bottomEnd); + expect(a.offset, const Offset(0, 4)); + expect(a.fallback!.targetAnchor, AlignmentDirectional.topStart); + expect(a.fallback!.followerAnchor, AlignmentDirectional.topEnd); + expect(a.fallback!.offset, const Offset(0, -4)); + }); + + test('when sizing a menu then it may use the larger side of its ' + 'trigger', () { + // 800 - 100 - 8 margin - 10 chrome. + expect( + WonkyMenuPlacement.itemsMaxHeight(800, below: 100, above: 80), + 682, + ); + // 700 above the trigger is more than 800 - 720 below it. + expect( + WonkyMenuPlacement.itemsMaxHeight(800, below: 720, above: 700), + 682, + ); + // Tiny windows still show a few items. + expect( + WonkyMenuPlacement.itemsMaxHeight(100, below: 50, above: 50), + WonkyMenuPlacement.minItemsHeight, + ); + }); + }); +} diff --git a/specs/02-design-system.md b/specs/02-design-system.md index 884d99b6..511b0166 100644 --- a/specs/02-design-system.md +++ b/specs/02-design-system.md @@ -123,6 +123,21 @@ Thin wrapper over `ShadContextMenu` (controller + anchor) adding: `OpenMenu` slot when they open, which closes the menu that was open before (shad puts a menu, its trigger and its popover in one tap-region group, so a tap on another menu's trigger would otherwise not close it). +- **Menus stay inside the window** (`WonkyMenuPlacement`, T-3): shad's manual `ShadAnchor` + glues an overlay to its trigger and never moves it, so a menu below a trigger at the bottom of + the window (the sheet tab menu) or start-aligned near the right edge ran off the screen. + `WonkyDropdownMenu` turns a `ShadAnchor` into an auto anchor (`WonkyMenuPlacement.onScreen`): + the menu opens where the anchor says when it fits, on the other side of the trigger (above + instead of below, the gap mirrored) when it does not, and shifts sideways to stay on screen. + Menus taller than the larger side of the trigger scroll (`buildWonkyMenuItems(maxHeight:)`), so + they always fit on one side. The same applies to submenus (beside their item, or ending level + with its bottom), the `WonkyMenubar` menus, `WonkyContextMenuRegion` (at the pointer, flipped + above it, as tall as the larger side of the pointer) and `WonkyPopupMenuHost` (top-left corner + at the pointer, moved left/up to fit). Every app's programmatic menus go through + `WonkyPopupMenuHost` (Write's own copy is gone), and dropdown-like `ShadPopover`s anchored with + a `ShadAnchor` (search suggestions, notifications, the pivot *Add field* list) wrap it in + `WonkyMenuPlacement.onScreen` too. A menu keeps its surface (`popover` colours, border, + shadow) wherever it opens, in light and dark. ### 4.3 `WonkyMenubar` `ShadMenubar` whose items are built from a declarative `MenuSpec` tree diff --git a/wonky_sheets/lib/features/pivot/ui/pivot_field_list.dart b/wonky_sheets/lib/features/pivot/ui/pivot_field_list.dart index c4bd99e1..cb4815ae 100644 --- a/wonky_sheets/lib/features/pivot/ui/pivot_field_list.dart +++ b/wonky_sheets/lib/features/pivot/ui/pivot_field_list.dart @@ -1,6 +1,7 @@ import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:shad/shad.dart'; +import 'package:wonky_ui/wonky_ui.dart'; import '../../../l10n/sheets_l10n.dart'; import '../pivot_spec_edit.dart'; @@ -117,10 +118,12 @@ class _PivotAddFieldButtonState extends State { final l = sheetsL10n(context); return ShadPopover( controller: _popover, - anchor: const ShadAnchor( - childAlignment: AlignmentDirectional.bottomEnd, - overlayAlignment: AlignmentDirectional.topEnd, - offset: Offset(0, 4), + anchor: WonkyMenuPlacement.onScreen( + const ShadAnchor( + childAlignment: AlignmentDirectional.bottomEnd, + overlayAlignment: AlignmentDirectional.topEnd, + offset: Offset(0, 4), + ), ), padding: const EdgeInsets.all(6), popover: (context) => _list(context, l), diff --git a/wonky_write/lib/features/screen/document_menus.dart b/wonky_write/lib/features/screen/document_menus.dart index 5282d273..628a925f 100644 --- a/wonky_write/lib/features/screen/document_menus.dart +++ b/wonky_write/lib/features/screen/document_menus.dart @@ -9,109 +9,6 @@ import '../editor/model/code_languages.dart'; import 'write_document_controller.dart'; import 'write_toolbar.dart'; -/// Hosts programmatic menus (context menu, block menu, code language -/// picker) anchored at a global position inside the canvas. -class PopupMenuHost extends StatefulWidget { - const PopupMenuHost({super.key, required this.child}); - - final Widget child; - - static PopupMenuHostState? maybeOf(BuildContext context) => - context.findAncestorStateOfType(); - - @override - State createState() => PopupMenuHostState(); -} - -class PopupMenuHostState extends State { - final _controller = ShadContextMenuController(); - Offset? _at; - List Function()? _entries; - VoidCallback? _onClosed; - bool _wasOpen = false; - - @override - void initState() { - super.initState(); - _controller.addListener(_changed); - } - - void _changed() { - if (_wasOpen && !_controller.isOpen) { - final cb = _onClosed; - _onClosed = null; - cb?.call(); - } - _wasOpen = _controller.isOpen; - if (mounted) setState(() {}); - } - - /// Opens a menu of [entries] at the global [position]. - void show( - Offset position, - List Function() entries, { - VoidCallback? onClosed, - }) { - setState(() { - _at = position; - _entries = entries; - _onClosed = onClosed; - }); - WidgetsBinding.instance.addPostFrameCallback((_) { - if (mounted) _controller.show(); - }); - } - - void hide() => _controller.hide(); - - bool get isOpen => _controller.isOpen; - - @override - void dispose() { - _controller - ..removeListener(_changed) - ..dispose(); - super.dispose(); - } - - @override - Widget build(BuildContext context) { - final entries = _entries; - // The menu is anchored globally, so its trigger is an empty box beside - // the canvas rather than a wrapper around it: `ShadContextMenu` puts its - // child in the shared context-menu tap group, and a wrapped canvas would - // count as "inside" every other context menu (menubar menus would no - // longer close when clicking into the document). - return Stack( - fit: StackFit.passthrough, - children: [ - widget.child, - Positioned( - left: 0, - top: 0, - width: 0, - height: 0, - child: ShadContextMenu( - controller: _controller, - anchor: _at == null ? null : ShadGlobalAnchor(_at!), - constraints: const BoxConstraints(minWidth: 220), - onTapOutside: (_) => _controller.hide(), - items: _controller.isOpen && entries != null - ? buildWonkyMenuItems( - context, - entries(), - onClose: _controller.hide, - requestFocus: true, - ) - : const [SizedBox.shrink()], - child: const SizedBox.shrink(), - ), - ), - ], - ); - } -} - /// "Turn into" entries (paragraph styles + lists). List turnIntoEntries(CommandUi ui, BlockKind? current) => [ ...paragraphStyleEntries(ui, current), diff --git a/wonky_write/lib/features/screen/write_document_screen.dart b/wonky_write/lib/features/screen/write_document_screen.dart index c9505607..b129884c 100644 --- a/wonky_write/lib/features/screen/write_document_screen.dart +++ b/wonky_write/lib/features/screen/write_document_screen.dart @@ -364,7 +364,7 @@ class _WriteDocumentScreenState extends State { {'nodeId': nodeId, 'row': ?row, 'col': ?column}, ), onCodeLanguageTap: (nodeId, rect) => - PopupMenuHost.maybeOf(_menuContext ?? context)?.show( + WonkyPopupMenuHost.maybeOf(_menuContext ?? context)?.show( rect.bottomLeft, () => codeLanguageEntries(ui, doc, nodeId: nodeId), ), @@ -394,7 +394,7 @@ class _WriteDocumentScreenState extends State { WidgetsBinding.instance.addPostFrameCallback((_) { if (mounted) _attachPagination(); }); - return PopupMenuHost( + return WonkyPopupMenuHost( child: Builder( builder: (context) { _menuContext = context; @@ -568,7 +568,7 @@ class _WriteDocumentScreenState extends State { } } final ui = CommandUi(context, s); - PopupMenuHost.maybeOf(context)?.show( + WonkyPopupMenuHost.maybeOf(context)?.show( global, () => contextMenuEntries(ui, doc), onClosed: doc.focusEditor,