From 0a1a5092f7c50ba1ebefd92245414dd6c8ef98cf Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 7 Oct 2026 17:03:47 +0200 Subject: [PATCH] fix(ui): the context menu answers the keyboard and closes when the page moves The shared context menu closed only on a click elsewhere: Escape did nothing, the arrow keys did nothing, and a scroll left the fixed menu hanging over content that had moved on. It now takes focus on its first item, moves with the arrow keys, and closes on Escape, scroll and resize. rules-of-hooks is an error again; its one hit was a test helper named like a hook. --- client/eslint.config.mjs | 4 +-- .../shared/ContextMenu.extra.test.tsx | 20 ++++++++++++++ client/src/components/shared/ContextMenu.tsx | 27 ++++++++++++++++++- client/src/pages/atlas/useAtlas.test.tsx | 10 +++---- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/client/eslint.config.mjs b/client/eslint.config.mjs index f007649a9..8887a2f44 100644 --- a/client/eslint.config.mjs +++ b/client/eslint.config.mjs @@ -54,8 +54,8 @@ export default tseslint.config( // --- Severities tuned to keep CI green on a codebase that was never linted --- // (each rule below has pre-existing violations; surfaced as warnings, not blockers) - // rules-of-hooks has one conditional-hook violation in PlaceInspector.tsx -> warn (not error). - 'react-hooks/rules-of-hooks': 'warn', + // A hook called conditionally is a crash waiting for the render that skips it. + 'react-hooks/rules-of-hooks': 'error', 'react-hooks/exhaustive-deps': 'warn', '@typescript-eslint/no-explicit-any': 'warn', diff --git a/client/src/components/shared/ContextMenu.extra.test.tsx b/client/src/components/shared/ContextMenu.extra.test.tsx index a0a292779..37f347045 100644 --- a/client/src/components/shared/ContextMenu.extra.test.tsx +++ b/client/src/components/shared/ContextMenu.extra.test.tsx @@ -114,4 +114,24 @@ describe('ContextMenu', () => { expect(menu.style.left).toBe('192px') expect(menu.style.top).toBe('172px') }) + + it('FE-W4CTX-011: Escape, a scroll and a resize close it', () => { + const onClose = vi.fn() + render() + fireEvent.keyDown(document, { key: 'Escape' }) + fireEvent.scroll(window) + fireEvent(window, new Event('resize')) + expect(onClose).toHaveBeenCalledTimes(3) + }) + + it('FE-W4CTX-012: focus starts on the first usable item and the arrow keys move it, wrapping', () => { + render( {}} />) + expect(screen.getByRole('button', { name: 'Open' })).toHaveFocus() + fireEvent.keyDown(screen.getByRole('button', { name: 'Open' }), { key: 'ArrowDown' }) + expect(screen.getByRole('button', { name: 'Rename' })).toHaveFocus() + fireEvent.keyDown(screen.getByRole('button', { name: 'Rename' }), { key: 'ArrowDown' }) + expect(screen.getByRole('button', { name: 'Open' })).toHaveFocus() + fireEvent.keyDown(screen.getByRole('button', { name: 'Open' }), { key: 'ArrowUp' }) + expect(screen.getByRole('button', { name: 'Rename' })).toHaveFocus() + }) }) diff --git a/client/src/components/shared/ContextMenu.tsx b/client/src/components/shared/ContextMenu.tsx index 6cdb6a22f..6242de6b9 100644 --- a/client/src/components/shared/ContextMenu.tsx +++ b/client/src/components/shared/ContextMenu.tsx @@ -44,14 +44,39 @@ export function ContextMenu({ menu, onClose }: ContextMenuProps) { useEffect(() => { if (!menu) return const handler = () => onClose() + // A fixed menu stays where it opened, so it closes when the page under it + // moves or the window changes size, and Escape closes it like any menu. + const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() } document.addEventListener('click', handler) document.addEventListener('contextmenu', handler) + document.addEventListener('keydown', onKey) + window.addEventListener('scroll', handler, true) + window.addEventListener('resize', handler) return () => { document.removeEventListener('click', handler) document.removeEventListener('contextmenu', handler) + document.removeEventListener('keydown', onKey) + window.removeEventListener('scroll', handler, true) + window.removeEventListener('resize', handler) } }, [menu, onClose]) + // Keyboard users land on the first item and move with the arrow keys. + useEffect(() => { + if (!menu) return + ref.current?.querySelector('button:not(:disabled)')?.focus({ preventScroll: true }) + }, [menu]) + + const moveFocus = (e: React.KeyboardEvent) => { + if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp') return + e.preventDefault() + const items = [...(ref.current?.querySelectorAll('button:not(:disabled)') ?? [])] + if (items.length === 0) return + const at = items.indexOf(document.activeElement as HTMLButtonElement) + const next = e.key === 'ArrowDown' ? (at + 1) % items.length : (at - 1 + items.length) % items.length + items[next]?.focus() + } + useLayoutEffect(() => { if (!menu || !ref.current) return const el = ref.current @@ -69,7 +94,7 @@ export function ContextMenu({ menu, onClose }: ContextMenuProps) { if (!menu) return null return createPortal( -
> = {}) { +function installAtlasHandlers(over: Partial> = {}) { server.use( http.get('/api/addons/atlas/stats', () => HttpResponse.json(over.stats ?? statsResponse)), http.get('/api/addons/atlas/bucket-list', () => HttpResponse.json({ items: over.bucket ?? [] })), @@ -298,7 +298,7 @@ function regionCodesOf(layer: MockGeoJson | undefined): string[] { } async function mountAtlas(over: Partial> = {}, props: { withPanel?: boolean } = {}) { - useAtlasHandlers(over); + installAtlasHandlers(over); const view = render(); await waitFor(() => expect(atlas.loading).toBe(false)); return view; @@ -332,7 +332,7 @@ describe('useAtlas', () => { }); it('FE-HOOK-ATLAS-002: a failing stats request still ends the loading state', async () => { - useAtlasHandlers(); + installAtlasHandlers(); server.use(http.get('/api/addons/atlas/stats', () => HttpResponse.error())); render(); @@ -468,7 +468,7 @@ describe('useAtlas', () => { }); it('FE-HOOK-ATLAS-011b: unmarking survives atlas data that never loaded', async () => { - useAtlasHandlers(); + installAtlasHandlers(); server.use( http.get('/api/addons/atlas/stats', () => HttpResponse.error()), http.delete('/api/addons/atlas/country/:code/mark', () => HttpResponse.json({ ok: true })), @@ -836,7 +836,7 @@ describe('useAtlas', () => { }); it('FE-HOOK-ATLAS-028: a failing plugin request simply leaves the map untinted', async () => { - useAtlasHandlers({ geo: geoCountries }); + installAtlasHandlers({ geo: geoCountries }); server.use(http.get('/api/atlas-layers', () => HttpResponse.error())); render(); await waitFor(() => expect(atlas.loading).toBe(false));