fix(ui): preserve dialog focus and evidence readability

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.

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.

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();

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<HTMLElement>(
-          '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
             </div>
             <div className="login-controls">
               <button className="btn-primary" onClick={() => {
-                const returnUrl = window.location.pathname + window.location.search;
+                const returnUrl = returnUrlFromLocation();
+                rememberOidcReturnUrl(returnUrl);
                 void auth.signinRedirect({ state: { returnUrl } });
               }}>
                 {t("Log in")}

frontend/src/components/AskEvidenceLayerPopup.test.tsx

@@ -30,7 +30,13 @@ describe("AskEvidenceLayerPopup", () => {
     expect(screen.getByRole("dialog", { name: "Checkout error follow-up" })).toBeInTheDocument();
     expect(screen.getByText(/project: Checkout revamp/)).toBeInTheDocument();
     expect(screen.getByText("Screenshot of the checkout error")).toBeInTheDocument();
-    expect(screen.getByText("Error code 500 on checkout")).toBeInTheDocument();
+    expect(screen.getByText(/Error code 500 on checkout/)).toBeInTheDocument();
+    expect(screen.getByText(/Semantic project:/).closest("li")).toHaveTextContent(
+      "Semantic project: project: Checkout revamp | evidence: Body evidence",
+    );
+    expect(screen.getByText("Screenshot of the checkout error").closest("li")).toHaveTextContent(
+      "Screenshot of the checkout error · Error code 500 on checkout · Image tags: screenshot, error",
+    );
     expect(
       screen.getByRole("list", { name: "Checkout error follow-up Evidence facts" }),
     ).toBeInTheDocument();
@@ -47,7 +53,7 @@ describe("AskEvidenceLayerPopup", () => {
       />,
     );
 
-    expect(screen.getByText("Time axis")).toBeInTheDocument();
+    expect(screen.getByText(/^Time axis:/)).toBeInTheDocument();
     expect(screen.getByText("time axis: event occurred at")).toBeInTheDocument();
     expect(
       screen.getByRole("button", { name: "Open post: Checkout error follow-up" }),
@@ -137,6 +143,23 @@ describe("AskEvidenceLayerPopup", () => {
     expect(openPostButton).toHaveFocus();
   });
 
+  it("excludes collapsed controls from the modal focus order", async () => {
+    render(
+      <AskEvidenceLayerPopup {...baseProps} facts={[]} images={[]} onClose={vi.fn()} onOpenPost={vi.fn()} />,
+    );
+    const panel = screen.getByRole("dialog");
+    const collapsed = document.createElement("details");
+    const collapsedButton = document.createElement("button");
+    collapsedButton.textContent = "Collapsed action";
+    collapsed.append(collapsedButton);
+    panel.append(collapsed);
+
+    panel.focus();
+    await userEvent.tab({ shift: true });
+    expect(screen.getByRole("button", { name: "Open post: Checkout error follow-up" })).toHaveFocus();
+    expect(collapsedButton).not.toHaveFocus();
+  });
+
   it("returns focus to the element that invoked the modal when the layer unmounts", () => {
     const opener = document.createElement("button");
     opener.textContent = "View evidence";

frontend/src/components/AskEvidenceLayerPopup.tsx

@@ -1,11 +1,13 @@
 import { useEffect, useId, useRef } from "react";
 import { chatEvidenceKindLabel } from "../evidenceKindLabels";
+import { isFocusableVisible } from "../focusVisibility";
 import { t, tf } from "../i18n";
 import { PopupCloseButton } from "./PopupCloseButton";
 
 const FOCUSABLE_SELECTOR = [
   "a[href]",
   "button:not([disabled])",
+  "summary",
   "input:not([disabled])",
   "select:not([disabled])",
   "textarea:not([disabled])",
@@ -75,9 +77,9 @@ export function AskEvidenceLayerPopup({
 
       const panel = panelRef.current;
       if (!panel) return;
-      const focusable = Array.from(panel.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR)).filter(
-        (element) => !element.hasAttribute("hidden") && element.getAttribute("aria-hidden") !== "true",
-      );
+      const focusable = Array.from(
+        panel.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR),
+      ).filter(isFocusableVisible);
       if (focusable.length === 0) {
         event.preventDefault();
         panel.focus();
@@ -135,7 +137,7 @@ export function AskEvidenceLayerPopup({
             <ul className="post-evidence-list" aria-labelledby={`${headingId} ${factsHeadingId}`}>
               {facts.map((fact, index) => (
                 <li key={`${fact.kind}:${fact.text}:${index}`}>
-                  <span>{chatEvidenceKindLabel(fact.kind)}</span>
+                  <span>{chatEvidenceKindLabel(fact.kind)}: </span>
                   <span>{fact.text}</span>
                 </li>
               ))}
@@ -149,8 +151,8 @@ export function AskEvidenceLayerPopup({
               {images.map((image) => (
                 <li key={image.unit_index}>
                   <span>{image.caption?.trim() ? image.caption : t("Untitled image")}</span>
-                  {image.extracted_text ? <span>{image.extracted_text}</span> : null}
-                  {image.tags.length ? <span>{t("Image tags")}: {image.tags.join(", ")}</span> : null}
+                  {image.extracted_text ? <span> · {image.extracted_text}</span> : null}
+                  {image.tags.length ? <span> · {t("Image tags")}: {image.tags.join(", ")}</span> : null}
                 </li>
               ))}
             </ul>

frontend/src/focusVisibility.test.ts

@@ -0,0 +1,27 @@
+import { describe, expect, it, vi } from "vitest";
+import { isFocusableVisible } from "./focusVisibility";
+
+describe("isFocusableVisible", () => {
+  it("requests the current opacity and visibility property checks", () => {
+    const button = document.createElement("button");
+    const checkVisibility = vi.fn(() => true);
+    button.checkVisibility = checkVisibility;
+
+    expect(isFocusableVisible(button)).toBe(true);
+    expect(checkVisibility).toHaveBeenCalledWith({
+      opacityProperty: true,
+      visibilityProperty: true,
+      checkOpacity: true,
+      checkVisibilityCSS: true,
+    });
+  });
+
+  it("excludes controls inside collapsed disclosure content", () => {
+    const details = document.createElement("details");
+    const summary = document.createElement("summary");
+    const button = document.createElement("button");
+    details.append(summary, button);
+    expect(isFocusableVisible(summary)).toBe(true);
+    expect(isFocusableVisible(button)).toBe(false);
+  });
+});

frontend/src/focusVisibility.ts

@@ -0,0 +1,15 @@
+/** Return whether an interactive descendant belongs in a modal focus order. */
+export function isFocusableVisible(element: HTMLElement): boolean {
+  const collapsedDetails = element.closest("details:not([open])");
+  if (collapsedDetails && collapsedDetails.querySelector(":scope > summary") !== element) return false;
+  if (element.closest('[hidden], [aria-hidden="true"], [inert]')) return false;
+  return (
+    typeof element.checkVisibility !== "function" ||
+    element.checkVisibility({
+      opacityProperty: true,
+      visibilityProperty: true,
+      checkOpacity: true,
+      checkVisibilityCSS: true,
+    })
+  );
+}