mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
@@ -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',
|
||||
|
||||
@@ -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(<ContextMenu menu={{ x: 0, y: 0, items: [{ label: 'Open' }] }} onClose={onClose} />)
|
||||
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(<ContextMenu menu={{ x: 0, y: 0, items: [{ label: 'Off', disabled: true }, { label: 'Open' }, { label: 'Rename' }] }} onClose={() => {}} />)
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<HTMLButtonElement>('button:not(:disabled)')?.focus({ preventScroll: true })
|
||||
}, [menu])
|
||||
|
||||
const moveFocus = (e: React.KeyboardEvent<HTMLDivElement>) => {
|
||||
if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp') return
|
||||
e.preventDefault()
|
||||
const items = [...(ref.current?.querySelectorAll<HTMLButtonElement>('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(
|
||||
<div ref={ref} className="trek-popover-enter" style={{
|
||||
<div ref={ref} onKeyDown={moveFocus} className="trek-popover-enter" style={{
|
||||
position: 'fixed', left: menu.x, top: menu.y, zIndex: 999999,
|
||||
background: 'var(--bg-card)', borderRadius: 10, padding: '4px',
|
||||
border: '1px solid var(--border-primary)',
|
||||
|
||||
@@ -267,7 +267,7 @@ function Harness({ withPanel = true }: { withPanel?: boolean }): React.ReactElem
|
||||
);
|
||||
}
|
||||
|
||||
function useAtlasHandlers(over: Partial<Record<string, unknown>> = {}) {
|
||||
function installAtlasHandlers(over: Partial<Record<string, unknown>> = {}) {
|
||||
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<Record<string, unknown>> = {}, props: { withPanel?: boolean } = {}) {
|
||||
useAtlasHandlers(over);
|
||||
installAtlasHandlers(over);
|
||||
const view = render(<Harness {...props} />);
|
||||
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(<Harness />);
|
||||
|
||||
@@ -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(<Harness />);
|
||||
await waitFor(() => expect(atlas.loading).toBe(false));
|
||||
|
||||
Reference in New Issue
Block a user