diff --git a/CHANGELOG.d/dialog-focus-evidence-readability.md b/CHANGELOG.d/dialog-focus-evidence-readability.md new file mode 100644 index 000000000..be53e85fb --- /dev/null +++ b/CHANGELOG.d/dialog-focus-evidence-readability.md @@ -0,0 +1,8 @@ +# Unreleased — Dialog focus and evidence text remain readable + +## Fixed + +- Post and evidence dialogs now exclude collapsed, hidden, inert, transparent, + and CSS-invisible controls from keyboard focus; related-post navigation keeps + focus inside the active dialog, and evidence fields retain visible + separators. OIDC login also preserves its validated deep-link return context. diff --git a/docs/adr/0153-ask-evidence-layer-popup.md b/docs/adr/0153-ask-evidence-layer-popup.md index 1c035d697..25ae2a583 100644 --- a/docs/adr/0153-ask-evidence-layer-popup.md +++ b/docs/adr/0153-ask-evidence-layer-popup.md @@ -23,12 +23,15 @@ displaying any other citation's evidence. It reuses the app's existing `.popup-backdrop`/`.popup-panel` visual language (`PostDetailPopup`'s own classes) rather than introducing a new modal style. -Its dialog semantics are stricter than `PostDetailPopup`'s: `role="dialog"`, -`aria-modal="true"`, `aria-labelledby` naming the cited post's title, -Escape-to-close, backdrop-click-to-close, and initial focus moved onto the -panel on mount. `PostDetailPopup` has none of these today; this decision -does not retrofit them there -- a focused follow-up, not silently expanded -scope of this change. +Both evidence and post-detail layers use `role="dialog"`, `aria-modal="true"`, +an accessible title, Escape-to-close, backdrop-click-to-close, focus +containment, and opener focus restoration. Post-detail navigation moves focus +back to the same dialog for the newly selected post. Native DOM visibility +and disclosure state exclude collapsed, hidden, inert, and CSS-invisible +controls from both focus orders. + +Each evidence row separates its type, value, OCR text, and image tags with +visible punctuation. Adjacent spans must not collapse into ambiguous text. `chatEvidenceKindLabel` (previously a private `App.tsx` helper) moved to `frontend/src/evidenceKindLabels.ts` so both `App.tsx` and the new @@ -58,5 +61,7 @@ could drift. piece of evidence) is now precedent for future evidence surfaces that don't warrant a full post detail popup. - The follow-up now applies the same dialog semantics, Escape-to-close, - initial focus, and focus restoration to `PostDetailPopup`; both popup + initial focus, focus restoration, focus containment, and selected-post + navigation refocus to `PostDetailPopup`. Collapsed, hidden, inert, and + CSS-invisible descendants are excluded from its focus order; both popup variants retain their separate content and data-fetching boundaries. diff --git a/frontend/src/App.test.tsx b/frontend/src/App.test.tsx index e0579a65a..9a8168017 100644 --- a/frontend/src/App.test.tsx +++ b/frontend/src/App.test.tsx @@ -2222,6 +2222,7 @@ describe("App, authenticated", () => { await waitFor(() => expect(screen.getByText("The evidence panel should show exactly this text.")).toBeInTheDocument(), ); + expect(screen.getByRole("dialog", { name: "Linked post" })).toHaveFocus(); }); it("shows an embedded invoice image instead of the raw base64 string", async () => { @@ -2291,9 +2292,17 @@ describe("App, authenticated", () => { const dialog = await screen.findByRole("dialog", { name: "Public post" }); expect(dialog).toHaveFocus(); + const collapsed = document.createElement("details"); + const collapsedButton = document.createElement("button"); + collapsedButton.textContent = "Collapsed action"; + collapsed.append(collapsedButton); + dialog.append(collapsed); await userEvent.tab({ shift: true }); - const focusable = within(dialog).getAllByRole("button").filter((button) => !button.hasAttribute("disabled")); + const focusable = within(dialog) + .getAllByRole("button") + .filter((button) => !button.hasAttribute("disabled") && !button.closest("details:not([open])")); expect(focusable.at(-1)).toHaveFocus(); + expect(collapsedButton).not.toHaveFocus(); await userEvent.tab(); const closeButton = within(dialog).getByRole("button", { name: "Close" }); expect(closeButton).toHaveFocus(); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 640262baf..ea8d8b3af 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -98,6 +98,7 @@ import { LineageDag } from "./LineageDag"; import { PostBody } from "./PostBody"; import { decodeHtmlEntities } from "./postBodyDisplay"; import { FiveW1H } from "./components/FiveW1H"; +import { isFocusableVisible } from "./focusVisibility"; import { subgraphForPost } from "./lineageLayout"; import { rememberOidcReturnUrl, returnUrlFromLocation, stripOidcCallbackParams } from "./oidcReturnUrl"; import { @@ -1809,12 +1810,15 @@ function PostDetailPopup({ useEffect(() => { const previouslyFocused = document.activeElement instanceof HTMLElement ? document.activeElement : null; - dialogRef.current?.focus(); return () => { if (previouslyFocused?.isConnected) previouslyFocused.focus(); }; }, []); + useEffect(() => { + dialogRef.current?.focus(); + }, [postId]); + useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { if (event.key === "Escape") { @@ -1828,9 +1832,9 @@ function PostDetailPopup({ if (!dialog) return; const focusable = Array.from( dialog.querySelectorAll( - 'a[href], button:not([disabled]), input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])', + 'a[href], button:not([disabled]), summary, input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])', ), - ).filter((element) => !element.hasAttribute("hidden") && element.getAttribute("aria-hidden") !== "true"); + ).filter(isFocusableVisible); if (focusable.length === 0) { event.preventDefault(); dialog.focus(); @@ -4968,7 +4972,8 @@ export default function App({ showLabPanels = false }: { showLabPanels?: boolean