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));