diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 606120b0..c4c48b5e 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -536,8 +536,10 @@ of a full-width panel and nothing on the right. ## Dialogs — _Decided_ (C4, built in S3f) - **Initial focus by purpose** (`Modal`'s required `initialFocus`, so no dialog inherits "the first - focusable", which is always the ✕): `field` — a form opens on the body's first control, and one that - loads after opening moves the focus there when it arrives, as long as the focus still rests on the ✕; + focusable", which is always the ✕): `field` — a form opens on its first field: the body's first + control, or the one marked `data-initial-focus` when a control above the form is not one of its + fields (the issue composer's Edit / Preview switch); one that loads after opening moves the focus + there when it arrives, as long as the focus still rests on the ✕; `action` — a confirm, where the least destructive button takes `autoFocus`; `close` — anything to read, browse or manage. [WAI-ARIA APG] - **The header and the footer are fixed; only the body scrolls,** with the B9 fade at each edge, so the @@ -702,3 +704,4 @@ of a full-width panel and nothing on the right. | 2026-10-01 | E31 Nav rail rows are links | Each page row is a real link: a middle-click or Ctrl-click opens the page in a new tab | A button | The owner: open a page in a tab of its own, as on any site | | 2026-10-02 | E31 Plex detail second column (refined) | The facts over the key crew beside the text; the keywords, chips that filter like the genres, under the text; the show page gets all three too | All three in the column, as the mockup drew it | At 20 keywords and 12 crew the column ran to twice the text's height (849 vs ~400 px at 133%): a hole under the text instead of beside it | | 2026-10-02 | E31 Plex detail poster and crew | The movie and show poster keeps its 2:3 and grows a rung in a wide hero (w-56 at a 64rem panel); the crew shows its first 6 names and "Show N more" (a list of 8 or fewer shows whole) | Poster stretched to the row; a "More" on the keywords | The row is as tall as the crew column, so the poster drew ~175×590 and cropped; the crew, not the keywords, set the hero's height | +| 2026-10-04 | E32 `field` focus skips a mode switch | A form opens on its first field; a control above it that is not a field (the composer's Edit / Preview) is passed over by marking the field `data-initial-focus` | The first text field everywhere | That moved the focus in the download dialog off its profile choice and in the upload dialog off its drop zone; the mark changes only the form that needs it | diff --git a/frontend/src/components/IssueComposer.dom.test.tsx b/frontend/src/components/IssueComposer.dom.test.tsx index be3aba63..0af536b4 100644 --- a/frontend/src/components/IssueComposer.dom.test.tsx +++ b/frontend/src/components/IssueComposer.dom.test.tsx @@ -153,6 +153,7 @@ beforeEach(() => { afterEach(() => { cleanup(); vi.useRealTimers(); + vi.restoreAllMocks(); }); let tracker: { repo: string } | null = null; @@ -167,6 +168,9 @@ function mount() { ); } +/** jsdom lays nothing out, so every element reads as hidden to the dialog's focus rules. */ +const layOut = () => + vi.spyOn(HTMLElement.prototype, "offsetParent", "get").mockReturnValue(document.body); const lastSaved = () => api.saved[api.saved.length - 1]!; const title = () => screen.getByLabelText("recorder.composer.fieldTitle") as HTMLInputElement; /** A button once it can be pressed: `loading` holds it while the draft's first save settles. */ @@ -489,6 +493,24 @@ describe("the issue composer", () => { expect(screen.queryByRole("button", { name: /recorder.composer.addStep/ })).toBeNull(); }); + it("opens on the title, not on the Edit / Preview switch above it", async () => { + // The form arrives after the dialog opens; its first control is the switch, and the focus + // landed there — typing went nowhere (E32 review round 3). + layOut(); + mount(); + await waitFor(() => expect(document.activeElement).toBe(title())); + }); + + it("moves to the title once a retried load brings the form", async () => { + layOut(); + api.failSession = true; + mount(); + const message = await screen.findByText("recorder.composer.loadFailed"); + api.failSession = false; + fireEvent.click(within(message.closest('[role="alert"]')!).getByRole("button")); + await waitFor(() => expect(document.activeElement).toBe(title())); + }); + it("says so when the session cannot be loaded, with a retry", async () => { api.failSession = true; mount(); diff --git a/frontend/src/components/IssueComposer.tsx b/frontend/src/components/IssueComposer.tsx index 3a36f963..e87214d7 100644 --- a/frontend/src/components/IssueComposer.tsx +++ b/frontend/src/components/IssueComposer.tsx @@ -483,6 +483,8 @@ function DraftFields({ - (initialFocus === "field" && bodyRef.current ? focusablesIn(bodyRef.current)[0] : null) ?? + (initialFocus === "field" && bodyRef.current ? firstFieldIn(bodyRef.current) : null) ?? (closeRef.current?.offsetParent != null ? closeRef.current : null), }); // A form that loads after the dialog opens (the issue composer shows a spinner first) has no field - // to take the focus at open, so it lands on the ✕. When the first control arrives the focus moves - // there — but only while it still rests on the ✕: a user who has moved it keeps it where it is. + // to take the focus at open, so it lands on the ✕. When the form arrives the focus moves to its + // first field — but only while it still rests on the ✕: a user who has moved it keeps it there. // The form takes it ONCE: arrived while the focus was elsewhere, it disarms all the same, or a later // mutation would pull the focus off a ✕ the user went back to (E32 review round 2). The watch // re-arms only when the focus is taken away, not moved: the body has no control again, or the // focused one went (a failed load's Retry giving way to the spinner, or straight to the form) and - // the focus fell to — it then waits on the ✕ for the form (E32 review). Only : focus - // in a dialog stacked on top is not this one's to take. + // the focus fell to . It then goes to the ✕, and on to the first field as soon as the body + // has one — at once, when other controls stayed beside the one that went (E32 review). Only + // : focus in a dialog stacked on top is not this one's to take. useEffect(() => { const body = bodyRef.current; if (initialFocus !== "field" || !body || focusablesIn(body).length > 0) return; @@ -121,7 +123,7 @@ export default function Modal({ closeRef.current.focus(); armed = true; } - const first = focusablesIn(body)[0]; + const first = firstFieldIn(body); if (!first) { armed = true; return; diff --git a/frontend/src/components/ui/dialogFocus.ts b/frontend/src/components/ui/dialogFocus.ts index 2fbe6e9c..33a439fb 100644 --- a/frontend/src/components/ui/dialogFocus.ts +++ b/frontend/src/components/ui/dialogFocus.ts @@ -16,6 +16,15 @@ const FOCUSABLE = export const focusablesIn = (root: HTMLElement): HTMLElement[] => [...root.querySelectorAll(FOCUSABLE)].filter((el) => el.offsetParent !== null); +/** A form's first field — where a dialog opened to fill something in takes the focus: its first + * control, unless one is marked `data-initial-focus`. The mark is for a form with a control above + * its fields that is not one of them — the issue composer's Edit / Preview switch took the focus, + * and typing went nowhere (E32 review). */ +export const firstFieldIn = (root: HTMLElement): HTMLElement | undefined => { + const all = focusablesIn(root); + return all.find((el) => el.hasAttribute("data-initial-focus")) ?? all[0]; +}; + /** Keep Tab / Shift+Tab cycling inside `scope` instead of reaching the inert page behind the * backdrop. Call from a keydown handler that has already decided this dialog is the topmost layer — * a nested dialog must defer to its child. `container` is the scope's own tabIndex=-1 panel, if it