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 <body>
(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 <body> 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 <rileycrane@squareup.com>
Co-authored-by: Fizz <d4ec90109b43092b0ec94cf136850915a3732a29ab3089a1dec43d2ed43dd6eb@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Riley Crane
2026-08-24 16:52:07 -07:00
parent c3b1721810
commit 8e906bef47
4 changed files with 179 additions and 44 deletions

View File

@@ -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<MarkdownDocView>("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 <body>.
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<MarkdownDocDecodeResult>({
@@ -126,17 +142,6 @@ export function MarkdownDocPanel({
<AuxiliaryPanelHeaderTitleBlock title={filename} />
</AuxiliaryPanelHeaderGroup>
<AuxiliaryPanelHeaderActions includeCloseAction>
{decoded?.kind === "ok" ? (
<SegmentedControl
legend="Document view"
onValueChange={setView}
optionTestIdPrefix="markdown-doc-view"
options={VIEW_OPTIONS}
size="compact"
testId="markdown-doc-view-toggle"
value={view}
/>
) : null}
<Button
aria-label={`Download ${filename}`}
data-testid="markdown-doc-download"
@@ -150,41 +155,63 @@ export function MarkdownDocPanel({
</AuxiliaryPanelHeader>
}
>
<AuxiliaryPanelBody className="overflow-y-auto px-4 pb-6" panelPadding>
{docQuery.isPending ? (
<div
className="flex items-center justify-center py-12"
data-testid="markdown-doc-loading"
>
<Loader2 className="h-5 w-5 animate-spin text-muted-foreground/70" />
</div>
) : errorMessage !== null ? (
<div className="flex flex-col items-center gap-3 py-12 text-center">
<p className="text-sm text-muted-foreground">{errorMessage}</p>
<Button onClick={handleDownload} size="sm" variant="secondary">
<Download className="mr-1.5 h-4 w-4" />
Download file
</Button>
</div>
) : decoded?.kind === "ok" ? (
view === "preview" ? (
<Markdown
blockCode
className="pt-3 text-sm"
content={decoded.text}
hardLineBreaks={false}
<AuxiliaryPanelBody className="flex min-h-0 flex-col" panelPadding>
{/* 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" ? (
<div className="flex shrink-0 items-center px-4 pb-2">
<SegmentedControl
legend="Document view"
onValueChange={setView}
optionTestIdPrefix="markdown-doc-view"
options={VIEW_OPTIONS}
size="compact"
testId="markdown-doc-view-toggle"
value={view}
/>
) : (
<pre
className="overflow-x-auto pt-3 text-xs leading-relaxed"
data-testid="markdown-doc-code"
>
{/* Shiki's synchronous-tokenization guard caps highlighting at
150 lines; longer documents render as plain text here. */}
<SyntaxHighlightedCode code={decoded.text} language="markdown" />
</pre>
)
</div>
) : null}
<div className="min-h-0 flex-1 overflow-y-auto px-4 pb-6">
{docQuery.isPending ? (
<div
className="flex items-center justify-center py-12"
data-testid="markdown-doc-loading"
>
<Loader2 className="h-5 w-5 animate-spin text-muted-foreground/70" />
</div>
) : errorMessage !== null ? (
<div className="flex flex-col items-center gap-3 py-12 text-center">
<p className="text-sm text-muted-foreground">{errorMessage}</p>
<Button onClick={handleDownload} size="sm" variant="secondary">
<Download className="mr-1.5 h-4 w-4" />
Download file
</Button>
</div>
) : decoded?.kind === "ok" ? (
view === "preview" ? (
<Markdown
blockCode
className="pt-3 text-sm"
content={decoded.text}
hardLineBreaks={false}
/>
) : (
<pre
className="overflow-x-auto pt-3 text-xs leading-relaxed"
data-testid="markdown-doc-code"
>
{/* Shiki's synchronous-tokenization guard caps highlighting at
150 lines; longer documents render as plain text here. */}
<SyntaxHighlightedCode
code={decoded.text}
language="markdown"
/>
</pre>
)
) : null}
</div>
</AuxiliaryPanelBody>
</AuxiliaryPanel>
);

View File

@@ -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 `<body>` 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. `<body>`/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<HTMLElement>(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<HTMLElement>(
`[data-testid="file-card"][data-doc-url="${CSS.escape(url)}"]`,
),
() => !focusIsFree(),
);
}

View File

@@ -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" }}

View File

@@ -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 <body>.
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,
}) => {