From bfc5d9a871abf70ab4e9003d860453420ed49221 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 21 Jul 2026 23:20:47 -0400 Subject: [PATCH 1/2] Make the canvas keyboard-reachable + trap focus in dialogs (#84) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The arrow-key nudge handler existed but only ever acted on a POINTER selection, and nothing could select without a mouse — so the feature was unusable by exactly the keyboard users it's for. This is the scoped first slice: give the canvas a keyboard path in, and fix the Modal focus trap that every destructive confirmation goes through. Canvas: - The gets role="application" + an aria-label describing the controls, and a naming the garden — a screen reader now announces an interactive canvas rather than an empty graphic. - Each object is a focusable role="button" with an aria-label (name + kind) and aria-pressed reflecting selection. Enter/Space selects it — the step that was missing — which makes the existing arrow-key nudge reachable. - A :focus-visible CSS rule draws a dashed accent ring on keyboard focus (and NOT on a mouse click, which is the point of :focus-visible). CSS rather than React state because onFocus on an SVG is unreliable, and a CSS rule cleanly overrides the shape's inline stroke. Modal (blast radius: DeleteGarden/ClearBed/DeletePlant/DeleteSeedLot/Share): - Tab is trapped inside the dialog and wraps at the ends, instead of walking out into the page behind the backdrop. - On close, focus returns to the element that opened the dialog rather than landing on . Verified live against the built binary with real keyboard input: Tab focuses an object (SVG genuinely takes focus), Enter flips aria-pressed false→true, the focus-visible dash renders (computed stroke-dasharray "5px, 4px"), the dialog traps focus through 5 Tabs, and Escape closes it and restores focus to the opener. Follow-ups noted, not done here: object dimensions in the aria-label (needs the garden's unit context this component doesn't hold), roving-tabindex between plops inside a focused bed, and the EditorRail tablist semantics. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- web/src/components/ui/Modal.tsx | 45 ++++++++++++++++++++++++++++++--- web/src/editor/GardenCanvas.tsx | 6 +++++ web/src/editor/ObjectShape.tsx | 38 ++++++++++++++++++++++++++-- web/src/styles/index.css | 13 ++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/web/src/components/ui/Modal.tsx b/web/src/components/ui/Modal.tsx index e671552..6a4874a 100644 --- a/web/src/components/ui/Modal.tsx +++ b/web/src/components/ui/Modal.tsx @@ -27,12 +27,51 @@ export function Modal({ busyRef.current = busy useEffect(() => { - cardRef.current?.focus() + const card = cardRef.current + // Remember who opened the dialog so focus can return there on close — + // otherwise it lands on and a keyboard user loses their place. + const opener = document.activeElement as HTMLElement | null + card?.focus() + + // The dialog's own focusable controls, in DOM order, skipping disabled ones. + const focusable = () => + Array.from( + card?.querySelectorAll( + 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])', + ) ?? [], + ) + function onKey(e: KeyboardEvent) { - if (e.key === 'Escape' && !busyRef.current) onCloseRef.current() + if (e.key === 'Escape' && !busyRef.current) { + onCloseRef.current() + return + } + if (e.key !== 'Tab') return + // Trap Tab inside the dialog: wrap at the ends, and pull a stray focus + // (e.g. starting from the card itself) back to a real control. Without + // this, Tab walks straight out into the page behind the backdrop. + const items = focusable() + if (items.length === 0) { + e.preventDefault() + card?.focus() + return + } + const first = items[0] + const last = items[items.length - 1] + const active = document.activeElement + if (e.shiftKey && (active === first || active === card)) { + e.preventDefault() + last.focus() + } else if (!e.shiftKey && active === last) { + e.preventDefault() + first.focus() + } } document.addEventListener('keydown', onKey) - return () => document.removeEventListener('keydown', onKey) + return () => { + document.removeEventListener('keydown', onKey) + opener?.focus?.() + } }, []) return ( diff --git a/web/src/editor/GardenCanvas.tsx b/web/src/editor/GardenCanvas.tsx index 78ba6f1..94ed51b 100644 --- a/web/src/editor/GardenCanvas.tsx +++ b/web/src/editor/GardenCanvas.tsx @@ -235,7 +235,13 @@ export function GardenCanvas({ className="h-full w-full select-none" style={{ touchAction: 'none' }} onPointerDown={onCanvasPointerDown} + // role="application" tells a screen reader this is an interactive canvas + // to operate, not a document to read linearly. The names it, and + // objects inside are individually focusable buttons (see ObjectShape). + role="application" + aria-label={`${garden.name} — garden layout. Tab between objects; Enter selects; arrow keys nudge a selection.`} > + <title>{garden.name} garden layout {drawnGridCm != null && ( <> diff --git a/web/src/editor/ObjectShape.tsx b/web/src/editor/ObjectShape.tsx index 4ba32bd..a51ddb9 100644 --- a/web/src/editor/ObjectShape.tsx +++ b/web/src/editor/ObjectShape.tsx @@ -1,5 +1,6 @@ -import { memo, type PointerEvent } from 'react' +import { memo, type KeyboardEvent, type PointerEvent } from 'react' import { objectTransform } from './shared' +import { objectDisplayName } from './kinds' import type { EditorObject } from './types' const DEFAULT_FILL = '#8a8a8a' @@ -57,11 +58,44 @@ export const ObjectShape = memo(function ObjectShape({ onSelect(object.id) } + // Keyboard path into selection (#84): the arrow-key nudge handler already + // exists but only ever acted on a pointer selection, so it was unreachable + // without a mouse. Enter/Space on a focused object selects it, which is the + // step that was missing. + function handleKey(e: KeyboardEvent) { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + e.stopPropagation() + onSelect(object.id) + } + } + const stroke = selected ? '#2f7a3e' : '#00000033' const strokeWidth = selected ? 2 : 1 + // A concise accessible name: the object's label plus its kind, e.g. + // "North Bed, raised bed". The dimensions aren't included — they need the + // garden's unit context this component doesn't hold — so they're a follow-up. + const label = `${objectDisplayName(object)}, ${object.kind.replace(/_/g, ' ')}` + + // Keyboard focus needs to be VISIBLE — that's the point of making the canvas + // keyboard-reachable. The `object-shape` class carries a :focus-visible rule + // (styles/index.css) that draws a dashed ring; :focus-visible means it shows + // for keyboard focus but NOT a mouse click, which is exactly what we want. CSS + // rather than React state because onFocus on an SVG is unreliable and a + // presentation attribute is overridden by any CSS rule. return ( - + {object.shape === 'circle' ? ( Date: Tue, 21 Jul 2026 23:33:38 -0400 Subject: [PATCH 2/2] Address Gadfly findings on #84 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Object aria-label uses kindDef().label (the canonical "In-ground") instead of an ad-hoc kind.replace() that produced "in ground" and diverged from the UI. 4+ models flagged this. - aria-current, not aria-pressed, for the selected object — selection isn't a toggle, which is what aria-pressed means; aria-current marks the active item. - Modal focus trap made robust: if focus is NOT inside the dialog (fell to because the focused control was removed — ShareGardenModal's remove-share button — or disabled while busy, or externally stolen), Tab now pulls it back in instead of escaping. The previous branches only handled focus being exactly at a known boundary. - Focus restore checks opener.isConnected before calling focus(): the delete/ clear flows this targets often remove the element that opened the dialog, and a disconnected node's focus() silently no-ops. - Hoisted the focusable-element selector to a module constant, and excluded input[type="hidden"] (it matched input:not([disabled]) and, at a boundary, broke the wrap). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- web/src/components/ui/Modal.tsx | 33 +++++++++++++++++++++++---------- web/src/editor/ObjectShape.tsx | 18 ++++++++++++------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/web/src/components/ui/Modal.tsx b/web/src/components/ui/Modal.tsx index 6a4874a..67313cd 100644 --- a/web/src/components/ui/Modal.tsx +++ b/web/src/components/ui/Modal.tsx @@ -1,5 +1,12 @@ import { useEffect, useRef, type ReactNode } from 'react' +// Tabbable controls inside the dialog, in DOM order. type="hidden" inputs are +// excluded — they'd match `input:not([disabled])` and, sitting at a boundary, +// break the wrap math. Hoisted out of the handler so it isn't rebuilt per Tab. +const FOCUSABLE_SELECTOR = + 'a[href], button:not([disabled]), textarea:not([disabled]), ' + + 'input:not([disabled]):not([type="hidden"]), select:not([disabled]), [tabindex]:not([tabindex="-1"])' + /** * A centered modal dialog over a dimmed backdrop. Closes on Escape or a backdrop * click, unless `busy` (a mutation is in flight) — then it stays put so the @@ -33,13 +40,8 @@ export function Modal({ const opener = document.activeElement as HTMLElement | null card?.focus() - // The dialog's own focusable controls, in DOM order, skipping disabled ones. const focusable = () => - Array.from( - card?.querySelectorAll( - 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])', - ) ?? [], - ) + Array.from(card?.querySelectorAll(FOCUSABLE_SELECTOR) ?? []) function onKey(e: KeyboardEvent) { if (e.key === 'Escape' && !busyRef.current) { @@ -47,9 +49,6 @@ export function Modal({ return } if (e.key !== 'Tab') return - // Trap Tab inside the dialog: wrap at the ends, and pull a stray focus - // (e.g. starting from the card itself) back to a real control. Without - // this, Tab walks straight out into the page behind the backdrop. const items = focusable() if (items.length === 0) { e.preventDefault() @@ -59,6 +58,15 @@ export function Modal({ const first = items[0] const last = items[items.length - 1] const active = document.activeElement + // If focus is NOT inside the dialog, pull it back in rather than let Tab + // escape. This is the robust case that covers focus having fallen to + // — a control that was removed (ShareGardenModal's remove-share + // button) or disabled while busy — as well as any externally-stolen focus. + if (!card || !card.contains(active)) { + e.preventDefault() + ;(e.shiftKey ? last : first).focus() + return + } if (e.shiftKey && (active === first || active === card)) { e.preventDefault() last.focus() @@ -70,7 +78,12 @@ export function Modal({ document.addEventListener('keydown', onKey) return () => { document.removeEventListener('keydown', onKey) - opener?.focus?.() + // Restore focus to the opener only if it's still in the document — the + // delete/clear flows this trap targets often remove the element that + // opened the dialog (a garden card, a plop row). A disconnected node's + // focus() silently no-ops and leaves focus on , so fall through to + // that case explicitly rather than pretend it worked. + if (opener && opener.isConnected) opener.focus() } }, []) diff --git a/web/src/editor/ObjectShape.tsx b/web/src/editor/ObjectShape.tsx index a51ddb9..2bf7204 100644 --- a/web/src/editor/ObjectShape.tsx +++ b/web/src/editor/ObjectShape.tsx @@ -1,6 +1,6 @@ import { memo, type KeyboardEvent, type PointerEvent } from 'react' import { objectTransform } from './shared' -import { objectDisplayName } from './kinds' +import { kindDef, objectDisplayName } from './kinds' import type { EditorObject } from './types' const DEFAULT_FILL = '#8a8a8a' @@ -73,10 +73,13 @@ export const ObjectShape = memo(function ObjectShape({ const stroke = selected ? '#2f7a3e' : '#00000033' const strokeWidth = selected ? 2 : 1 - // A concise accessible name: the object's label plus its kind, e.g. - // "North Bed, raised bed". The dimensions aren't included — they need the - // garden's unit context this component doesn't hold — so they're a follow-up. - const label = `${objectDisplayName(object)}, ${object.kind.replace(/_/g, ' ')}` + // A concise accessible name: the object's label plus its kind's canonical + // label, e.g. "North Bed, In-ground" — reusing kindDef so it never diverges + // from what the UI shows (an ad-hoc kind.replace() gave "in ground"). The + // dimensions aren't included; they need the garden's unit context this + // component doesn't hold, so they're a follow-up. + const kindLabel = kindDef(object.kind)?.label ?? object.kind + const label = `${objectDisplayName(object)}, ${kindLabel}` // Keyboard focus needs to be VISIBLE — that's the point of making the canvas // keyboard-reachable. The `object-shape` class carries a :focus-visible rule @@ -93,7 +96,10 @@ export const ObjectShape = memo(function ObjectShape({ role="button" tabIndex={0} aria-label={label} - aria-pressed={selected} + // aria-current, not aria-pressed: selecting an object isn't a toggle (a + // toggle is what aria-pressed means). aria-current marks it as the active + // item among the objects. Omitted, not "false", when unselected. + aria-current={selected || undefined} style={{ cursor: 'pointer' }} > {object.shape === 'circle' ? (