From 8e906bef4764e69b09beecd785dc150fca174398 Mon Sep 17 00:00:00 2001 From: Riley Crane Date: Mon, 24 Aug 2026 16:52:07 -0700 Subject: [PATCH] fix(desktop): manage viewer panel focus and give the view picker its own row Review P2 (#6731): in the narrow single-panel layout, opening a document unmounted the focused attachment card and closing unmounted the focused panel, dropping keyboard/screen-reader users onto (then the remounted channel's composer autofocus). The panel now moves focus to its close control on mount and returns it to the opening card on unmount, matched by data-doc-url since the original element was remounted meanwhile; the restore yields to any control that already claimed focus, overriding only and the composer autofocus. A narrow-layout e2e covers open-focus and Escape-restore. Also moves the Preview/Code picker out of the title row onto its own pinned row below the header (review feedback: the shared row squeezed the filename out), inside the chrome-padded body since the header band overlays anything placed after it in the header slot. Signed-off-by: Riley Crane Co-authored-by: Fizz Co-Authored-By: Claude Fable 5 --- .../features/channels/ui/MarkdownDocPanel.tsx | 115 +++++++++++------- .../features/channels/ui/markdownDocFocus.ts | 78 ++++++++++++ desktop/src/shared/ui/markdown/FileCard.tsx | 4 + desktop/tests/e2e/markdown-doc-viewer.spec.ts | 26 ++++ 4 files changed, 179 insertions(+), 44 deletions(-) create mode 100644 desktop/src/features/channels/ui/markdownDocFocus.ts diff --git a/desktop/src/features/channels/ui/MarkdownDocPanel.tsx b/desktop/src/features/channels/ui/MarkdownDocPanel.tsx index 411d30bf87..072fa6d4d9 100644 --- a/desktop/src/features/channels/ui/MarkdownDocPanel.tsx +++ b/desktop/src/features/channels/ui/MarkdownDocPanel.tsx @@ -8,6 +8,10 @@ import { fetchMarkdownDocBytes, isMediaTooLargeError, } from "@/shared/api/tauriMedia"; +import { + focusMarkdownDocPanelClose, + restoreFocusToMarkdownDocOpener, +} from "@/features/channels/ui/markdownDocFocus"; import { useEscapeKey } from "@/shared/hooks/useEscapeKey"; import { useIsThreadPanelOverlay } from "@/shared/hooks/use-mobile"; import { @@ -74,6 +78,18 @@ export function MarkdownDocPanel({ useEscapeKey(onClose, isOverlay || isSinglePanelView); const [view, setView] = React.useState("preview"); + // Opening can unmount the section holding the focused attachment card + // (narrow layout swaps the whole channel out), and closing unmounts this + // panel — move focus in on mount and hand it back to the opener card on + // unmount so keyboard users never fall to . + React.useEffect(() => { + const cancel = focusMarkdownDocPanelClose(); + return () => { + cancel(); + restoreFocusToMarkdownDocOpener(url); + }; + }, [url]); + // Blob URLs are content-addressed (`/media/{sha256}.{ext}`), so a fetched // document never changes under its URL — cache it for the session. const docQuery = useQuery({ @@ -126,17 +142,6 @@ export function MarkdownDocPanel({ - {decoded?.kind === "ok" ? ( - - ) : null} - - ) : decoded?.kind === "ok" ? ( - view === "preview" ? ( - + {/* The view picker gets its own pinned row below the title: sharing + the title row squeezed the filename out, and the header chrome + band overlays anything placed directly after it in the header + slot — so the row lives inside the chrome-padded body instead. */} + {decoded?.kind === "ok" ? ( +
+ - ) : ( - 
-              {/* Shiki's synchronous-tokenization guard caps highlighting at
-                  150 lines; longer documents render as plain text here. */}
-              
-
- ) +
) : null} +
+ {docQuery.isPending ? ( +
+ +
+ ) : errorMessage !== null ? ( +
+

{errorMessage}

+ +
+ ) : decoded?.kind === "ok" ? ( + view === "preview" ? ( + + ) : ( + 
+                {/* Shiki's synchronous-tokenization guard caps highlighting at
+                  150 lines; longer documents render as plain text here. */}
+                
+
+ ) + ) : null} +
); diff --git a/desktop/src/features/channels/ui/markdownDocFocus.ts b/desktop/src/features/channels/ui/markdownDocFocus.ts new file mode 100644 index 0000000000..8615493763 --- /dev/null +++ b/desktop/src/features/channels/ui/markdownDocFocus.ts @@ -0,0 +1,78 @@ +/** + * Focus choreography for the markdown document panel (PR #6731 P2). + * + * In the narrow single-panel layout, opening a document unmounts the channel + * section containing the focused attachment card, and closing unmounts the + * panel that held focus — in both directions focus falls to `` and + * keyboard/screen-reader users lose their place. On open, focus moves to the + * panel's close control; on close, it returns to the attachment card that + * opened the document (matched by URL, since the original element was + * unmounted meanwhile). + */ + +const PANEL_CLOSE_SELECTOR = + '[data-testid="markdown-doc-panel"] [data-testid="auxiliary-panel-close"]'; + +/** Frames to wait for the target to (re)mount before giving up. */ +const FOCUS_SEARCH_FRAMES = 12; + +function scheduleFocusSearch( + find: () => HTMLElement | null, + shouldAbort: () => boolean, +): () => void { + let frame = 0; + let attempts = 0; + const tick = () => { + if (shouldAbort()) return; + const target = find(); + if (target) { + target.focus(); + return; + } + attempts += 1; + if (attempts < FOCUS_SEARCH_FRAMES) frame = requestAnimationFrame(tick); + }; + frame = requestAnimationFrame(tick); + return () => cancelAnimationFrame(frame); +} + +/** + * True when moving focus is a restoration, not a steal. ``/null means + * focus fell off an unmounted subtree. The composer counts as free too: the + * remounting channel autofocuses it, which is exactly the "lands on the + * composer rather than the invoking attachment" behavior being fixed — + * anything else (another panel's control, a clicked button) keeps focus. + */ +function focusIsFree(): boolean { + const active = document.activeElement; + if (active === null || active === document.body) return true; + return active.closest('[data-testid="message-composer"]') !== null; +} + +/** + * Move focus onto the open panel's close control. Returns a canceler for + * effect cleanup so an unmounting panel stops hunting for its own button. + */ +export function focusMarkdownDocPanelClose(): () => void { + return scheduleFocusSearch( + () => document.querySelector(PANEL_CLOSE_SELECTOR), + // Never abort: the open was user-initiated, the panel is the destination. + () => false, + ); +} + +/** + * After the panel closes, return focus to the attachment card that opened + * `url` once the channel section has remounted. Aborts if focus has already + * landed on a real control (e.g. a different panel claimed it), and gives up + * quietly when the card no longer exists. + */ +export function restoreFocusToMarkdownDocOpener(url: string): void { + scheduleFocusSearch( + () => + document.querySelector( + `[data-testid="file-card"][data-doc-url="${CSS.escape(url)}"]`, + ), + () => !focusIsFree(), + ); +} diff --git a/desktop/src/shared/ui/markdown/FileCard.tsx b/desktop/src/shared/ui/markdown/FileCard.tsx index 48318ada17..ba0555d25b 100644 --- a/desktop/src/shared/ui/markdown/FileCard.tsx +++ b/desktop/src/shared/ui/markdown/FileCard.tsx @@ -82,6 +82,10 @@ export function FileCard({ ); }} aria-label={opensInViewer ? `Open ${filename}` : `Download ${filename}`} + // Focus-return anchor: when the viewer panel closes, focus comes back + // to the card that opened this URL (the element itself may have been + // unmounted and remounted in the narrow layout meanwhile). + data-doc-url={opensInViewer ? href : undefined} data-testid="file-card" className="my-1 inline-flex max-w-sm items-center gap-3 rounded-2xl border border-border/70 bg-muted/40 px-3 py-2 text-left no-underline transition-colors hover:bg-muted/70" style={{ borderRadius: "1rem" }} diff --git a/desktop/tests/e2e/markdown-doc-viewer.spec.ts b/desktop/tests/e2e/markdown-doc-viewer.spec.ts index c362a617bd..e3f35039ee 100644 --- a/desktop/tests/e2e/markdown-doc-viewer.spec.ts +++ b/desktop/tests/e2e/markdown-doc-viewer.spec.ts @@ -126,6 +126,32 @@ test("markdown attachment opens the in-app viewer with Preview/Code toggle", asy await expect(page.getByTestId("markdown-doc-panel")).toHaveCount(0); }); +test("narrow layout moves focus into the panel and returns it to the card", async ({ + page, +}) => { + await sendMarkdownAttachment(page); + + // Below the split-pane threshold the panel replaces the channel section, + // unmounting the focused attachment card — the focus contract under test. + await page.setViewportSize({ width: 560, height: 720 }); + + const card = page.getByTestId("file-card").last(); + await card.focus(); + await expect(card).toBeFocused(); + await page.keyboard.press("Enter"); + + // Focus lands on the panel's close control, not . + const panel = page.getByTestId("markdown-doc-panel"); + await expect(panel).toBeVisible(); + await expect(page.getByTestId("auxiliary-panel-close")).toBeFocused(); + + // Escape closes the panel and hands focus back to the invoking card — + // overriding the remounted channel's composer autofocus. + await page.keyboard.press("Escape"); + await expect(page.getByTestId("markdown-doc-panel")).toHaveCount(0); + await expect(page.getByTestId("file-card").last()).toBeFocused(); +}); + test("open document survives reload and back/forward navigation", async ({ page, }) => {