fix(ui): the issue composer opens on its title, not on the Edit switch
A form dialog opens on the body's first control, and the composer's first control is the Edit / Preview switch above the form: the focus landed there, at open and when a retried load brought the form, and typing went nowhere. A form can now mark the field to open on (data-initial-focus); every other dialog keeps its first control. The Modal's comment on the re-armed watch now says where the focus goes when the focused control is removed but others stay.
This commit is contained in:
+5
-2
@@ -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 |
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -483,6 +483,8 @@ function DraftFields({
|
||||
<Input
|
||||
className="w-full"
|
||||
id="issue-title"
|
||||
// the dialog opens here, not on the Edit / Preview switch above the form
|
||||
data-initial-focus
|
||||
value={form.title}
|
||||
readOnly={readOnly}
|
||||
maxLength={200}
|
||||
|
||||
@@ -14,16 +14,17 @@ import { X } from "lucide-react";
|
||||
import { useBackToClose } from "../../lib/history";
|
||||
import { anyBlockingLayerAbove, pushLayer, removeLayer, takeEscape } from "../../lib/layerStack";
|
||||
import { useFullscreenPortalHost } from "../../lib/fullscreen";
|
||||
import { focusablesIn, trapTab, useDialogChrome } from "./dialogFocus";
|
||||
import { firstFieldIn, focusablesIn, trapTab, useDialogChrome } from "./dialogFocus";
|
||||
import { IconButton } from "./IconButton";
|
||||
import Toaster from "./Toaster";
|
||||
import { useOverlayOrder } from "./overlayOrder";
|
||||
import { SURFACE } from "./surfaces";
|
||||
|
||||
/** Where a dialog opens focused (C4, by purpose — WAI-ARIA APG): `field` = the body's first control
|
||||
* (a form), `close` = the header's ✕ (anything to read or browse), `action` = the child that takes
|
||||
* `autoFocus` itself (a confirm's least destructive button; the ✕ if none does). Required, so every
|
||||
* dialog states its purpose instead of inheriting "the first focusable", which is always the ✕. */
|
||||
/** Where a dialog opens focused (C4, by purpose — WAI-ARIA APG): `field` = the body's first control,
|
||||
* or the one marked `data-initial-focus` (a form), `close` = the header's ✕ (anything to read or
|
||||
* browse), `action` = the child that takes `autoFocus` itself (a confirm's least destructive button;
|
||||
* the ✕ if none does). Required, so every dialog states its purpose instead of inheriting "the first
|
||||
* focusable", which is always the ✕. */
|
||||
export type ModalFocus = "field" | "close" | "action";
|
||||
|
||||
// The footer slot of the open dialog. `ModalFooter` portals into it from any depth, so a body that
|
||||
@@ -100,18 +101,19 @@ export default function Modal({
|
||||
// Only a rendered target (the trap's own `offsetParent` test); none falls back to the hook's
|
||||
// first-focusable rule. `action` lands on the ✕ here and its child's autoFocus then takes over.
|
||||
initial: () =>
|
||||
(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 <body> — it then waits on the ✕ for the form (E32 review). Only <body>: focus
|
||||
// in a dialog stacked on top is not this one's to take.
|
||||
// the focus fell to <body>. 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
|
||||
// <body>: 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;
|
||||
|
||||
@@ -16,6 +16,15 @@ const FOCUSABLE =
|
||||
export const focusablesIn = (root: HTMLElement): HTMLElement[] =>
|
||||
[...root.querySelectorAll<HTMLElement>(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
|
||||
|
||||
Reference in New Issue
Block a user