From 3fa82849a8aecd7606c49edb6408799853ba6af8 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:59:52 +0100 Subject: [PATCH] fix: open menus next to what opened them and show their keys apart from the label --- components/calendar/calendar-toolbar.tsx | 6 +- components/layout/phone-header-actions.tsx | 2 +- components/plugins/plugin-actions.tsx | 2 +- .../__tests__/context-menu-placement.test.tsx | 162 ++++++++++++++++++ components/ui/context-menu.tsx | 83 +++++---- hooks/use-context-menu.ts | 34 +--- .../__tests__/native-menu-keys.test.tsx | 98 +++++++++++ lib/desktop/native-menu.ts | 133 +++++++++++++- lib/desktop/types.ts | 5 + lib/menu-placement.ts | 83 +++++++++ 10 files changed, 539 insertions(+), 69 deletions(-) create mode 100644 components/ui/__tests__/context-menu-placement.test.tsx create mode 100644 lib/desktop/__tests__/native-menu-keys.test.tsx create mode 100644 lib/menu-placement.ts diff --git a/components/calendar/calendar-toolbar.tsx b/components/calendar/calendar-toolbar.tsx index d1957b882..fe6471667 100644 --- a/components/calendar/calendar-toolbar.tsx +++ b/components/calendar/calendar-toolbar.tsx @@ -6,7 +6,7 @@ import { Button } from "@/components/ui/button"; import { IconButton } from "@/components/ui/icon-button"; import { ToolbarButton } from "@/components/ui/toolbar"; import { ChevronLeft, ChevronRight, Plus, Upload, CalendarDays, Globe, ChevronDown, ArrowLeft, Menu, MoreVertical, RefreshCw, Search, Printer } from "@/components/icons"; -import { ContextMenu, ContextMenuItem, ContextMenuSeparator } from "@/components/ui/context-menu"; +import { ContextMenu, ContextMenuItem, ContextMenuSeparator, MenuShortcut } from "@/components/ui/context-menu"; import { useContextMenu } from "@/hooks/use-context-menu"; import { startOfWeek } from "date-fns"; import { PhoneHeaderActions } from "@/components/layout/phone-header-actions"; @@ -482,9 +482,9 @@ export function CalendarToolbar({ v === viewMode ? "bg-accent text-accent-foreground font-medium" : "hover:bg-muted text-foreground", )} > - {t(`views.${v}`)} + {t(`views.${v}`)} {/* The key, on devices with a keyboard (R8). */} - {VIEW_SHORTCUTS[v]} + ))} diff --git a/components/layout/phone-header-actions.tsx b/components/layout/phone-header-actions.tsx index e33815115..7e74c7074 100644 --- a/components/layout/phone-header-actions.tsx +++ b/components/layout/phone-header-actions.tsx @@ -92,7 +92,7 @@ export function PhoneHeaderMenu({ menuItems }: PhoneHeaderMenuProps) { 0 ? formatBadgeCount(unread) : undefined} + badge={unread > 0 ? formatBadgeCount(unread) : undefined} onClick={() => { close(); setShowNotifications(true); }} /> { void run(action); }} /> ))} diff --git a/components/ui/__tests__/context-menu-placement.test.tsx b/components/ui/__tests__/context-menu-placement.test.tsx new file mode 100644 index 000000000..a856a4268 --- /dev/null +++ b/components/ui/__tests__/context-menu-placement.test.tsx @@ -0,0 +1,162 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ContextMenu, ContextMenuItem, ContextMenuSubMenu, type ContextMenuPosition } from '../context-menu'; +import { useContextMenu } from '@/hooks/use-context-menu'; +import { placeMenu } from '@/lib/menu-placement'; +import type { BulwarkDesktopBridge } from '@/lib/desktop/types'; + +// Every menu opens next to what opened it: under a button, lined up with its +// edge, or at the pointer, and towards the other side only where there's no +// room. Never clamped somewhere away from the button. + +function rect(left: number, top: number, width: number, height: number): DOMRect { + return { left, top, width, height, right: left + width, bottom: top + height, x: left, y: top, toJSON: () => ({}) } as DOMRect; +} + +const MENU = { width: 220, height: 300 }; + +beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { value: 1280, configurable: true }); + Object.defineProperty(window, 'innerHeight', { value: 720, configurable: true }); + Object.defineProperty(window, 'outerWidth', { value: 1280, configurable: true }); + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { + return this.getAttribute('role') === 'menu' ? rect(0, 0, MENU.width, MENU.height) : rect(0, 0, 0, 0); + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + window.bulwarkDesktop = undefined; + document.documentElement.dir = ''; +}); + +function openAt(position: ContextMenuPosition) { + render( + {}}> + {}} /> + , + ); + const menu = screen.getByRole('menu'); + return { left: parseFloat(menu.style.left), top: parseFloat(menu.style.top) }; +} + +describe('a menu from a right-click', () => { + it('opens at the pointer where there is room', () => { + expect(openAt({ x: 300, y: 200 })).toEqual({ left: 300, top: 200 }); + }); + + it('opens to the left of the pointer near the right edge, its corner still at the pointer', () => { + // Clamped instead, the menu would sit under the pointer, away from where the click was. + expect(openAt({ x: 1200, y: 200 })).toEqual({ left: 1200 - MENU.width, top: 200 }); + }); + + it('opens upwards from the pointer near the bottom', () => { + expect(openAt({ x: 300, y: 650 })).toEqual({ left: 300, top: 650 - MENU.height }); + }); + + it('keeps the pointer where it was: the hook no longer guesses a size and moves it first', () => { + function Row() { + const { contextMenu, openContextMenu, closeContextMenu } = useContextMenu(); + return ( + <> +
openContextMenu(e, 'row')}>Row
+ + {}} /> + + + ); + } + render(); + // A short menu right-clicked at 640: it fits below the pointer. + MENU.height = 60; + try { + fireEvent.contextMenu(screen.getByTestId('row'), { clientX: 400, clientY: 640 }); + const menu = screen.getByRole('menu'); + expect(menu.style.top).toBe('640px'); + expect(menu.style.left).toBe('400px'); + } finally { + MENU.height = 300; + } + }); +}); + +describe('a menu from a button', () => { + it('sits under the button, lined up with its start edge', () => { + expect(openAt({ x: 0, y: 0, anchor: { left: 100, top: 40, right: 132, bottom: 72 } })).toEqual({ left: 100, top: 76 }); + }); + + it('lines up with the button\'s end edge for a button at the end of a toolbar', () => { + expect(openAt({ x: 0, y: 0, anchor: { left: 900, top: 40, right: 932, bottom: 72, align: 'end' } })).toEqual({ left: 932 - MENU.width, top: 76 }); + }); + + it('switches to the button\'s other edge when it doesn\'t fit, instead of sliding away from it', () => { + const at = openAt({ x: 0, y: 0, anchor: { left: 1200, top: 40, right: 1232, bottom: 72 } }); + expect(at.left + MENU.width).toBe(1232); + }); + + it('goes above the button when there is no room below', () => { + expect(openAt({ x: 0, y: 0, anchor: { left: 100, top: 600, right: 132, bottom: 632 } }).top).toBe(600 - 4 - MENU.height); + }); + + it('mirrors in RTL: start is the right edge', () => { + document.documentElement.dir = 'rtl'; + const at = openAt({ x: 0, y: 0, anchor: { left: 500, top: 40, right: 532, bottom: 72 } }); + // jsdom computes no direction from dir; the pure function covers RTL below. + expect([500, 532 - MENU.width]).toContain(at.left); + }); +}); + +describe('placeMenu', () => { + const viewport = { width: 1280, height: 720 }; + const size = { width: 220, height: 300 }; + + it('RTL: under a button, its right edge with the button\'s right edge', () => { + expect(placeMenu({ point: { x: 0, y: 0 }, anchor: { left: 500, top: 40, right: 532, bottom: 72 }, size, viewport, rtl: true })) + .toEqual({ x: 532 - 220, y: 76 }); + }); + + it('RTL: an end-aligned button gets the left edge', () => { + expect(placeMenu({ point: { x: 0, y: 0 }, anchor: { left: 40, top: 40, right: 72, bottom: 72, align: 'end' }, size, viewport, rtl: true })) + .toEqual({ x: 40, y: 76 }); + }); + + it('RTL: a right-click opens to the left of the pointer', () => { + expect(placeMenu({ point: { x: 600, y: 100 }, size, viewport, rtl: true })).toEqual({ x: 380, y: 100 }); + }); + + it('taller than the window: inside it, from the top margin, and the menu scrolls', () => { + expect(placeMenu({ point: { x: 600, y: 100 }, size: { width: 220, height: 900 }, viewport })).toEqual({ x: 600, y: 10 }); + }); + + it('wider than the room on both sides of a button: clamped inside the window', () => { + expect(placeMenu({ point: { x: 0, y: 0 }, anchor: { left: 100, top: 40, right: 132, bottom: 72 }, size: { width: 1270, height: 100 }, viewport }).x).toBe(10); + }); +}); + +describe('a native menu from a button', () => { + it('lines its right edge up with an end-aligned button, by the native menu\'s width, not the hidden copy\'s', async () => { + const popup = vi.fn(async () => null); + window.bulwarkDesktop = { version: 1, platform: 'win32', menu: { setState: () => {}, popup } } as unknown as BulwarkDesktopBridge; + // The hidden copy holds every submenu open, so it is far wider than the menu the OS draws. + MENU.width = 700; + try { + render( + {}}> + {}} /> + + {}} /> + + {}} /> + , + ); + await waitFor(() => expect(popup).toHaveBeenCalled()); + const at = (popup.mock.calls[0] as unknown[])[1] as { x: number; y: number }; + expect(at.y).toBe(76); + // Under the button: its left edge somewhere left of the button, not at the window's edge. + expect(at.x).toBeGreaterThan(200); + expect(at.x).toBeLessThan(468); + } finally { + MENU.width = 220; + } + }); +}); diff --git a/components/ui/context-menu.tsx b/components/ui/context-menu.tsx index 8241bd4d0..84e0c359c 100644 --- a/components/ui/context-menu.tsx +++ b/components/ui/context-menu.tsx @@ -18,7 +18,8 @@ import { createPortal } from "react-dom"; import { cn } from "@/lib/utils"; import { ChevronRight } from "@/components/icons"; import { useTranslations } from "next-intl"; -import { nativeMenusAvailable, readSettledMenu, showNativeMenu } from "@/lib/desktop/native-menu"; +import { estimateNativeMenuWidth, nativeMenuPoint, nativeMenusAvailable, readSettledMenu, showNativeMenu } from "@/lib/desktop/native-menu"; +import { placeMenu } from "@/lib/menu-placement"; import { toast } from "@/stores/toast-store"; interface Position { @@ -282,8 +283,11 @@ export const ContextMenu = forwardRef( } const { anchor: at } = positionRef.current; // From a right-click it opens at the pointer, which the OS knows best. + // From a button: under it, lined up with the edge the HTML menu would + // use. The hidden copy is as wide as its widest submenu row, not as the + // native menu, so the native menu's own width is estimated. const place = at - ? { x: at.align === "end" ? Math.max(0, at.right - root.getBoundingClientRect().width) : at.left, y: at.bottom + ANCHOR_GAP } + ? nativeMenuPoint(at, estimateNativeMenuWidth(read.items), getComputedStyle(root).direction === "rtl", ANCHOR_GAP) : undefined; const outcome = await showNativeMenu(read, root, place, tDesktopRef.current("from_plugins")); if (cancelled) return; @@ -322,32 +326,16 @@ export const ContextMenu = forwardRef( const clamp = () => { const rect = node.getBoundingClientRect(); - const vw = window.innerWidth; - const vh = window.innerHeight; - - let x = position.x; - let y = position.y; - - if (anchor) { - // Under the anchor, lined up with its start edge (end edge for - // align="end"); above it when there is no room below. - const rtl = getComputedStyle(node).direction === "rtl"; - const alignEnd = (anchor.align === "end") !== rtl; - x = alignEnd ? anchor.right - rect.width : anchor.left; - y = anchor.bottom + ANCHOR_GAP; - if (y + rect.height > vh - VIEWPORT_MARGIN && anchor.top - ANCHOR_GAP - rect.height >= VIEWPORT_MARGIN) { - y = anchor.top - ANCHOR_GAP - rect.height; - } - } - - if (x + rect.width > vw - VIEWPORT_MARGIN) { - x = vw - rect.width - VIEWPORT_MARGIN; - } - if (y + rect.height > vh - VIEWPORT_MARGIN) { - y = vh - rect.height - VIEWPORT_MARGIN; - } - x = Math.max(VIEWPORT_MARGIN, x); - y = Math.max(VIEWPORT_MARGIN, y); + // Next to the button it opened from, or at the pointer (lib/menu-placement.ts). + const { x, y } = placeMenu({ + point: { x: position.x, y: position.y }, + anchor, + size: { width: rect.width, height: rect.height }, + viewport: { width: window.innerWidth, height: window.innerHeight }, + rtl: getComputedStyle(node).direction === "rtl", + margin: VIEWPORT_MARGIN, + gap: ANCHOR_GAP, + }); setAdjustedPosition((prev) => (prev && prev.x === x && prev.y === y ? prev : { x, y })); }; @@ -574,7 +562,10 @@ interface ContextMenuItemProps { onClick: () => void; disabled?: boolean; destructive?: boolean; + /** The key that does the same, shown apart from the label. */ shortcut?: string; + /** A count at the end of the row, e.g. unread notifications. Not a key. */ + badge?: string; /** Stable hook for integration tests (not user-visible). */ testId?: string; /** Tooltip, e.g. the address behind a display name. */ @@ -590,6 +581,7 @@ export function ContextMenuItem({ disabled = false, destructive = false, shortcut, + badge, testId, title, hint, @@ -636,14 +628,41 @@ export function ContextMenuItem({ ) : ( {label} )} - {/* Key hints mean nothing without a keyboard: hidden on touch (R8). */} - {shortcut && ( - {shortcut} - )} + {shortcut && } + {badge && } ); } +/** + * The key that does what a menu row does, at the row's end, muted, as a keycap. The + * desktop app's native menu shows it as the item's accelerator, apart from the label + * (lib/desktop/native-menu.ts reads `data-menu-shortcut`). Key hints mean nothing + * without a keyboard: hidden on touch (R8). + */ +export function MenuShortcut({ keys, className }: { keys: string; className?: string }) { + return ( + + {keys} + + ); +} + +/** A count at a row's end ("3" unread). Not a key: a native menu puts it after the label. */ +export function MenuBadge({ text }: { text: string }) { + return ( + + {text} + + ); +} + export function ContextMenuSeparator() { return
; } diff --git a/hooks/use-context-menu.ts b/hooks/use-context-menu.ts index eb412b42d..d76555be2 100644 --- a/hooks/use-context-menu.ts +++ b/hooks/use-context-menu.ts @@ -24,10 +24,6 @@ export interface UseContextMenuReturn { menuRef: React.RefObject; } -const MENU_WIDTH = 200; -const MENU_HEIGHT = 320; // Approximate max height -const VIEWPORT_MARGIN = 10; - /** Shift+F10 or the Menu key: open the focused element's context menu. */ export function isContextMenuKey(e: { key: string; shiftKey: boolean }): boolean { return e.key === "ContextMenu" || (e.shiftKey && e.key === "F10"); @@ -71,30 +67,6 @@ export function useContextMenu(): UseContextMenuReturn { const menuRef = useRef(null); - const calculatePosition = useCallback((clientX: number, clientY: number): ContextMenuPosition => { - const viewportWidth = window.innerWidth; - const viewportHeight = window.innerHeight; - - let x = clientX; - let y = clientY; - - // Adjust for right edge - if (x + MENU_WIDTH > viewportWidth - VIEWPORT_MARGIN) { - x = viewportWidth - MENU_WIDTH - VIEWPORT_MARGIN; - } - - // Adjust for bottom edge - if (y + MENU_HEIGHT > viewportHeight - VIEWPORT_MARGIN) { - y = viewportHeight - MENU_HEIGHT - VIEWPORT_MARGIN; - } - - // Ensure minimum position - x = Math.max(VIEWPORT_MARGIN, x); - y = Math.max(VIEWPORT_MARGIN, y); - - return { x, y }; - }, []); - const openContextMenu = useCallback((e: React.MouseEvent, data: T) => { e.preventDefault(); e.stopPropagation(); @@ -103,14 +75,16 @@ export function useContextMenu(): UseContextMenuReturn { const trigger = target ? focusReturnTarget(target) : null; const position = target && openedByKeyboard(e) ? { ...anchorPosition(target), trigger } - : { ...calculatePosition(e.clientX, e.clientY), trigger }; + // At the pointer as it is: the menu places itself once it knows its size, + // opening up or to the side where there's no room (lib/menu-placement.ts). + : { x: e.clientX, y: e.clientY, trigger }; setContextMenu({ isOpen: true, position, data, }); - }, [calculatePosition]); + }, []); const openContextMenuFrom = useCallback((element: HTMLElement, data: T, align: "start" | "end" = "start") => { setContextMenu({ isOpen: true, position: anchorPosition(element, align), data }); diff --git a/lib/desktop/__tests__/native-menu-keys.test.tsx b/lib/desktop/__tests__/native-menu-keys.test.tsx new file mode 100644 index 000000000..3b4016619 --- /dev/null +++ b/lib/desktop/__tests__/native-menu-keys.test.tsx @@ -0,0 +1,98 @@ +import { render, screen } from '@testing-library/react'; +import { readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { ContextMenuItem } from '@/components/ui/context-menu'; +import { readMenu, toAccelerator } from '../native-menu'; + +// A native menu built from an HTML one shows a row's key apart from its label, +// as the native menus' own accelerators do. The calendar's view menu came out +// as "DayD", "WeekW": the key was read as part of the label. + +function menuOf(children: React.ReactNode) { + render(
{children}
); + return readMenu(screen.getByTestId('root')).items; +} + +describe('a row\'s key in a native menu', () => { + it('is not part of the label, and becomes the accelerator', () => { + const items = menuOf( + , + ); + expect(items[0].label).toBe('Day'); + expect(items[0].accelerator).toBe('D'); + }); + + it('a menu item\'s shortcut too', () => { + const items = menuOf( {}} />); + expect(items[0]).toMatchObject({ label: 'Cut', accelerator: 'Ctrl+X' }); + }); + + it('a count is no key: it goes after the label', () => { + const items = menuOf( {}} />); + expect(items[0].label).toBe('Notifications (3)'); + expect(items[0].accelerator).toBeUndefined(); + }); + + it('words in separate elements stay separate words', () => { + const items = menuOf( + , + ); + expect(items[0].label).toBe('Alice Example alice@example.net'); + }); +}); + +describe('toAccelerator', () => { + it.each([ + ['D', 'D'], + ['r', 'R'], + ['Shift + I', 'Shift+I'], + ['Ctrl+X', 'Ctrl+X'], + ['⌘X', 'Cmd+X'], + ['⌘⇧K', 'Cmd+Shift+K'], + ['Mod + Enter', 'CmdOrCtrl+Enter'], + ['Del', 'Delete'], + ['F2', 'F2'], + ['?', '?'], + ['#', '#'], + ['Ctrl++', 'Ctrl+Plus'], + ])('%s is %s', (hint, accelerator) => { + expect(toAccelerator(hint)).toBe(accelerator); + }); + + it.each(['j / ↓', '', 'Hyper+Q', 'twelve'])('%s is no single key', (hint) => { + expect(toAccelerator(hint)).toBeUndefined(); + }); +}); + +// Menu rows show their keys through MenuShortcut (or ContextMenuItem's +// shortcut), which the native menu knows. A raw in a file with menu +// rows is how "DayD" happened. +describe('menu rows show keys through MenuShortcut', () => { + const root = path.resolve(__dirname, '../../..'); + it('no raw next to role="menuitem…" rows', () => { + const files: string[] = []; + const walk = (dir: string) => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === 'node_modules' || entry.name === '__tests__' || entry.name.startsWith('.')) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) walk(full); + else if (entry.name.endsWith('.tsx')) files.push(full); + } + }; + walk(path.join(root, 'components')); + walk(path.join(root, 'app')); + const offenders = files.filter((file) => { + if (file.endsWith(path.join('ui', 'context-menu.tsx'))) return false; + const source = readFileSync(file, 'utf8'); + return /role="menuitem/.test(source) && /]/.test(source); + }); + expect(offenders.map((file) => path.relative(root, file))).toEqual([]); + }, 60_000); +}); diff --git a/lib/desktop/native-menu.ts b/lib/desktop/native-menu.ts index 5fac69c9e..2ae8e4bcf 100644 --- a/lib/desktop/native-menu.ts +++ b/lib/desktop/native-menu.ts @@ -35,9 +35,68 @@ function isDisabled(el: HTMLElement): boolean { return (el as HTMLButtonElement).disabled === true || el.getAttribute('aria-disabled') === 'true'; } +/** What a row shows apart from its label: the key, a count. Never part of the label. */ +const ASIDE = 'kbd, [data-menu-shortcut], [data-menu-badge]'; + +/** + * The words of an element, each element's apart: "Day" and "D" make "Day D", not + * "DayD", and a name over an address doesn't run into it. Keys and counts are left out. + */ +function wordsOf(node: Node): string { + if (node.nodeType === Node.TEXT_NODE) return node.textContent ?? ''; + if (!(node instanceof Element) || node.matches(ASIDE)) return ''; + return Array.from(node.childNodes).map(wordsOf).join(' '); +} + function labelOf(el: HTMLElement): string { - const named = el.getAttribute('aria-label') ?? el.querySelector('[data-menu-label]')?.textContent ?? el.textContent ?? ''; - return named.replace(/\s+/g, ' ').trim(); + const named = el.getAttribute('aria-label') ?? el.querySelector('[data-menu-label]')?.textContent ?? wordsOf(el); + const badge = el.querySelector('[data-menu-badge]')?.textContent?.trim(); + const label = named.replace(/\s+/g, ' ').trim(); + return badge && label ? `${label} (${badge})` : label; +} + +const KEY_NAMES: Record = { + del: 'Delete', delete: 'Delete', esc: 'Escape', escape: 'Escape', enter: 'Enter', return: 'Enter', + space: 'Space', tab: 'Tab', backspace: 'Backspace', insert: 'Insert', home: 'Home', end: 'End', + pageup: 'PageUp', pagedown: 'PageDown', up: 'Up', down: 'Down', left: 'Left', right: 'Right', + '↑': 'Up', '↓': 'Down', '←': 'Left', '→': 'Right', '+': 'Plus', '⌫': 'Backspace', '⏎': 'Enter', '↵': 'Enter', +}; +const MODIFIERS: Record = { + ctrl: 'Ctrl', control: 'Ctrl', mod: 'CmdOrCtrl', cmd: 'Cmd', command: 'Cmd', '⌘': 'Cmd', alt: 'Alt', option: 'Alt', + '⌥': 'Alt', shift: 'Shift', '⇧': 'Shift', '⌃': 'Ctrl', meta: 'Super', win: 'Super', +}; +const KEY = /^([A-Z0-9]|F([1-9]|1[0-9]|2[0-4])|[)!@#$%^&*(:;=<,_\-.>?/~`{\][|\\}"']|Plus|Space|Tab|Backspace|Delete|Insert|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape)$/; + +/** + * A key hint as the app shows it ("Shift + I", "⌘X", "Del") in Electron's accelerator + * syntax, or undefined when it isn't one key (two alternatives, a word). + */ +export function toAccelerator(hint: string): string | undefined { + let text = hint.trim(); + if (!text || text.includes(' / ')) return undefined; + const parts: string[] = []; + // macOS writes its modifiers as symbols with nothing between: ⌘⇧K. + while (text.length > 1 && /^[⌘⌥⇧⌃]/.test(text)) { + parts.push(MODIFIERS[text[0]]); + text = text.slice(1).replace(/^\s*\+?\s*/, ''); + } + const pieces = text === '+' ? ['+'] : text.split(/\s*\+\s*/).filter(Boolean); + if (text.endsWith('++')) pieces.push('+'); + for (let i = 0; i < pieces.length - 1; i++) { + const modifier = MODIFIERS[pieces[i].toLowerCase()]; + if (!modifier) return undefined; + parts.push(modifier); + } + const last = pieces[pieces.length - 1]; + if (!last) return undefined; + const key = KEY_NAMES[last.toLowerCase()] ?? (last.length === 1 ? last.toUpperCase() : last); + if (!KEY.test(key)) return undefined; + return [...new Set(parts), key].join('+'); +} + +function acceleratorOf(el: HTMLElement): string | undefined { + const hint = el.querySelector('[data-menu-shortcut], kbd')?.textContent; + return hint ? toAccelerator(hint) : undefined; } function isHidden(el: Element): boolean { @@ -100,10 +159,12 @@ export function readMenu(root: HTMLElement): ReadMenu { const id = `i${next++}`; elements.set(id, child); const role = child.getAttribute('role'); + const accelerator = acceleratorOf(child); into.push({ id, label, enabled: !isDisabled(child), + ...(accelerator ? { accelerator } : {}), ...(role === 'menuitemcheckbox' || role === 'menuitemradio' ? { type: role === 'menuitemcheckbox' ? 'checkbox' as const : 'radio' as const, checked: child.getAttribute('aria-checked') === 'true' } : {}), @@ -182,6 +243,74 @@ export async function showNativeMenu(read: ReadMenu, root: HTMLElement, at?: { x return 'done'; } +/** + * The system's menu font and the room a native menu adds around its words, in screen + * pixels: Chromium's menus on Windows and Linux, AppKit's on macOS. Measured off the + * real menus at 100 %; an estimate within a few pixels is all a placement needs. + */ +const NATIVE_METRICS = { + win32: { font: '12px "Segoe UI", system-ui, sans-serif', padding: 74, keyGap: 28, charWidth: 6.2 }, + darwin: { font: '14px -apple-system, system-ui, sans-serif', padding: 48, keyGap: 24, charWidth: 7.2 }, + linux: { font: '13px system-ui, sans-serif', padding: 70, keyGap: 28, charWidth: 6.8 }, +} as const; + +let measureContext: CanvasRenderingContext2D | null | undefined; + +function textWidth(text: string, font: string, charWidth: number): number { + if (measureContext === undefined) { + try { + // jsdom has no canvas, and says so on the console. + measureContext = typeof document === 'undefined' || /jsdom/i.test(navigator.userAgent) + ? null + : document.createElement('canvas').getContext('2d'); + } catch { + measureContext = null; + } + } + if (measureContext) { + measureContext.font = font; + const width = measureContext.measureText(text).width; + if (Number.isFinite(width) && width > 0) return width; + } + return text.length * charWidth; +} + +/** + * How wide the native menu for these items will be, in the page's CSS pixels. A native + * menu is drawn at the screen's scale, not the page's zoom: at 150 % zoom it takes up + * less of the page. outerWidth is in screen pixels and innerWidth in CSS pixels, which + * gives the zoom in a window with no frame. + */ +export function estimateNativeMenuWidth(items: DesktopNativeMenuItem[], platform = getDesktopBridge()?.platform ?? 'win32'): number { + const metrics = NATIVE_METRICS[platform] ?? NATIVE_METRICS.win32; + let widest = 0; + for (const item of items) { + if (item.type === 'separator' || !item.label) continue; + const key = item.accelerator ? metrics.keyGap + textWidth(item.accelerator, metrics.font, metrics.charWidth) : 0; + widest = Math.max(widest, textWidth(item.label, metrics.font, metrics.charWidth) + key); + } + const screenWidth = widest + metrics.padding; + const zoom = typeof window !== 'undefined' && window.innerWidth > 0 && window.outerWidth > 0 + ? Math.min(5, Math.max(0.25, window.outerWidth / window.innerWidth)) + : 1; + return Math.round(screenWidth / zoom); +} + +/** + * Where a native menu from a button opens, in page coordinates: its top left corner, + * under the button and lined up with the edge the HTML menu would use. Only that: the + * OS keeps a native menu on the screen itself, and it may reach past the window. + */ +export function nativeMenuPoint( + anchor: { left: number; right: number; bottom: number; align?: 'start' | 'end' }, + menuWidth: number, + rtl: boolean, + gap: number, +): { x: number; y: number } { + const alignEnd = (anchor.align === 'end') !== rtl; + return { x: Math.max(0, Math.round(alignEnd ? anchor.right - menuWidth : anchor.left)), y: Math.round(anchor.bottom + gap) }; +} + /** A native menu is up: the window's blur or a stray event says nothing about it. */ export function nativeMenuIsOpen(): boolean { return typeof document !== 'undefined' && document.documentElement.dataset.nativeMenuOpen !== undefined; diff --git a/lib/desktop/types.ts b/lib/desktop/types.ts index 69cbd18c0..1a36ccf10 100644 --- a/lib/desktop/types.ts +++ b/lib/desktop/types.ts @@ -84,6 +84,11 @@ export interface DesktopNativeMenuItem { type?: 'normal' | 'separator' | 'checkbox' | 'radio'; enabled?: boolean; checked?: boolean; + /** + * The key the page answers for this item, in Electron's accelerator syntax ("D", + * "Shift+I", "Ctrl+X"). Only shown, apart from the label: never registered. + */ + accelerator?: string; submenu?: DesktopNativeMenuItem[]; } diff --git a/lib/menu-placement.ts b/lib/menu-placement.ts new file mode 100644 index 000000000..a8397d1ee --- /dev/null +++ b/lib/menu-placement.ts @@ -0,0 +1,83 @@ +/** + * Where a menu goes: next to what opened it, inside the window. + * + * - Opened from a button (an anchor): under it, lined up with its start edge, or its + * end edge for a button at the end of a toolbar. In RTL start and end swap. When it + * doesn't fit on that side it lines up with the other edge, and when there's no room + * below it goes above. + * - Opened by a right-click (a point): its corner at the pointer, opening to the right + * and down (to the left in RTL), and towards the other side where there's no room, as + * the OS does with its own menus. + * + * Whatever is left over is clamped into the window. A menu taller than the window + * scrolls inside (the menu caps its own height). + */ + +export interface MenuAnchorRect { + left: number; + top: number; + right: number; + bottom: number; + /** Line up with the anchor's start (default) or end edge. */ + align?: "start" | "end"; +} + +export interface MenuPlacementInput { + /** Where a right-click happened. Ignored when there's an anchor. */ + point: { x: number; y: number }; + anchor?: MenuAnchorRect; + size: { width: number; height: number }; + viewport: { width: number; height: number }; + rtl?: boolean; + /** Room kept free along the window's edges. */ + margin?: number; + /** Between the anchor and the menu. */ + gap?: number; +} + +export const MENU_VIEWPORT_MARGIN = 10; +export const MENU_ANCHOR_GAP = 4; + +function clamp(value: number, size: number, room: number, margin: number): number { + return Math.max(margin, Math.min(value, room - size - margin)); +} + +export function placeMenu({ + point, + anchor, + size, + viewport, + rtl = false, + margin = MENU_VIEWPORT_MARGIN, + gap = MENU_ANCHOR_GAP, +}: MenuPlacementInput): { x: number; y: number } { + const { width: w, height: h } = size; + const { width: vw, height: vh } = viewport; + const fitsX = (x: number) => x >= margin && x + w <= vw - margin; + const fitsY = (y: number) => y >= margin && y + h <= vh - margin; + + let x: number; + let y: number; + if (anchor) { + const alignEnd = (anchor.align === "end") !== rtl; + const startEdge = anchor.left; + const endEdge = anchor.right - w; + const preferred = alignEnd ? endEdge : startEdge; + const other = alignEnd ? startEdge : endEdge; + x = fitsX(preferred) || !fitsX(other) ? preferred : other; + const below = anchor.bottom + gap; + const above = anchor.top - gap - h; + if (fitsY(below)) y = below; + else if (fitsY(above)) y = above; + // Neither fits whole: the side with more room, and the menu scrolls. + else y = vh - anchor.bottom >= anchor.top ? below : above; + } else { + const towardEnd = rtl ? point.x - w : point.x; + const towardStart = rtl ? point.x : point.x - w; + x = fitsX(towardEnd) || !fitsX(towardStart) ? towardEnd : towardStart; + const down = point.y; + const up = point.y - h; + y = fitsY(down) || !fitsY(up) ? down : up; + } + return { x: clamp(x, w, vw, margin), y: clamp(y, h, vh, margin) }; +}