Author SHA1 Message Date
steveandClaude Opus 4.8 4b348dcbc0 Address Gadfly findings on #84
Build image / build-and-push (push) Successful in 10s
- 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
  <body> 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) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
2026-07-21 23:33:38 -04:00
steveandClaude Opus 4.8 bfc5d9a871 Make the canvas keyboard-reachable + trap focus in dialogs (#84)
Build image / build-and-push (push) Successful in 9s
Gadfly review (reusable) / review (pull_request) Successful in 7m40s
Adversarial Review (Gadfly) / review (pull_request) Successful in 7m41s
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 <svg> gets role="application" + an aria-label describing the controls,
  and a <title> naming the garden — a screen reader now announces an
  interactive canvas rather than an empty graphic.
- Each object <g> 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 <g> 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 <body>.

Verified live against the built binary with real keyboard input: Tab focuses
an object (SVG <g tabindex> 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) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
2026-07-21 23:20:47 -04:00
4 changed files with 116 additions and 5 deletions
+55 -3
View File
@@ -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
@@ -27,12 +34,57 @@ 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 <body> and a keyboard user loses their place.
const opener = document.activeElement as HTMLElement | null
card?.focus()
const focusable = () =>
Array.from(card?.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR) ?? [])
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
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 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
// <body> — 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()
} else if (!e.shiftKey && active === last) {
e.preventDefault()
first.focus()
}
}
document.addEventListener('keydown', onKey)
return () => document.removeEventListener('keydown', onKey)
return () => {
document.removeEventListener('keydown', onKey)
// 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 <body>, so fall through to
// that case explicitly rather than pretend it worked.
if (opener && opener.isConnected) opener.focus()
}
}, [])
return (
+6
View File
@@ -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 <title> 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</title>
<g transform={`translate(${viewport.tx} ${viewport.ty}) scale(${viewport.scale})`}>
{drawnGridCm != null && (
<>
+42 -2
View File
@@ -1,5 +1,6 @@
import { memo, type PointerEvent } from 'react'
import { memo, type KeyboardEvent, type PointerEvent } from 'react'
import { objectTransform } from './shared'
import { kindDef, objectDisplayName } from './kinds'
import type { EditorObject } from './types'
const DEFAULT_FILL = '#8a8a8a'
@@ -57,11 +58,50 @@ 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'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
// (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 <g> is unreliable and a
// presentation attribute is overridden by any CSS rule.
return (
<g transform={objectTransform(object)} onPointerDown={handleDown} style={{ cursor: 'pointer' }}>
<g
className="object-shape"
transform={objectTransform(object)}
onPointerDown={handleDown}
onKeyDown={handleKey}
role="button"
tabIndex={0}
aria-label={label}
// 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' ? (
<ellipse
cx={0}
+13
View File
@@ -28,6 +28,19 @@
font-family: var(--font-sans);
-webkit-font-smoothing: antialiased;
}
/* Keyboard focus on a canvas object (#84). :focus-visible shows the ring for
keyboard focus but not a mouse click; the dashed accent ring distinguishes
"focused" from the solid ring that marks "selected". A CSS rule overrides
the shape's inline stroke presentation attributes. */
.object-shape {
outline: none;
}
.object-shape:focus-visible :is(rect, ellipse) {
stroke: var(--color-accent-strong);
stroke-width: 2;
stroke-dasharray: 5 4;
}
}
/* Dark theme: override the same tokens so utilities recolor automatically. */