From 6e6bf624374a4eb1fd8f9fe43c115fe13d8770d1 Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 31 Aug 2026 18:54:23 +0200 Subject: [PATCH] IssueReportPage: unmount guard on the repos fetch, aria-sort on sortable headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two non-blocking notes from review: - the /api/repos effect now uses the same cancelled guard the repo-filter effect already has, so an unmount mid-flight doesn't call setRepos/setError on a gone component. - Table's TableColumn gains an optional ariaSort field, consumed as the 's aria-sort attribute; the issue-report page's sortable columns now report ascending/descending/none so a screen reader can announce which column and direction is active, not just the sighted ▲/▼ glyph. --- .../swarm-ui/src/pages/IssueReportPage.tsx | 33 ++++++++++++++++--- .../packages/swarm-ui/src/ui/table/Table.tsx | 15 ++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx b/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx index a20b7351..dc7a352c 100644 --- a/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx @@ -109,16 +109,26 @@ export function IssueReportPage() { // Repo dropdown source, fetched once — this page has no refresh // cadence, and a repo gaining/losing its first/last open issue between - // visits is rare enough that a manual reload covers it. + // visits is rare enough that a manual reload covers it. Same + // `cancelled` guard as the repo-filter effect below, so an unmount + // mid-flight (navigating away before this resolves) doesn't call + // `setRepos`/`setError` on a component that's already gone. useEffect(() => { + let cancelled = false; (async () => { const r = await fetch('/api/repos'); if (!r.ok) { - setError(await readApiError(r)); + if (!cancelled) setError(await readApiError(r)); return; } - setRepos((await r.json()) as string[]); - })().catch((e: unknown) => setError({ detail: String(e) })); + const data = (await r.json()) as string[]; + if (!cancelled) setRepos(data); + })().catch((e: unknown) => { + if (!cancelled) setError({ detail: String(e) }); + }); + return () => { + cancelled = true; + }; }, []); // The one fetch this page re-runs on state change — everything else @@ -188,6 +198,15 @@ export function IssueReportPage() { }); } + // `aria-sort` for a sortable column's `` — 'none' while sortable + // but not the active column, the real direction while it is. Screen + // readers announce this; the ▲/▼ glyph in `SortHeader` is `aria-hidden` + // and carries no information without it. + function ariaSortFor(key: SortKey): 'ascending' | 'descending' | 'none' { + if (sortKey !== key) return 'none'; + return sortDir === 'asc' ? 'ascending' : 'descending'; + } + const columns: TableColumn[] = [ // Shown even with one repo selected (every row shares the value) — // a fixed column set means the table's shape doesn't shift under @@ -195,11 +214,13 @@ export function IssueReportPage() { { key: 'repo', header: , + ariaSort: ariaSortFor('repo'), render: (r) => r.repo, }, { key: 'number', header: , + ariaSort: ariaSortFor('number'), render: (r) => r.html_url ? ( @@ -212,6 +233,7 @@ export function IssueReportPage() { { key: 'title', header: , + ariaSort: ariaSortFor('title'), render: (r) => r.title, }, { @@ -224,11 +246,13 @@ export function IssueReportPage() { header: ( ), + ariaSort: ariaSortFor('assignees'), render: (r) => (r.assignees.length ? r.assignees.join(', ') : '—'), }, { key: 'blocked', header: , + ariaSort: ariaSortFor('blocked'), render: (r) => (r.blocked ? : '—'), }, { @@ -242,6 +266,7 @@ export function IssueReportPage() { onSort={onSort} /> ), + ariaSort: ariaSortFor('depended_on_by_count'), render: (r) => r.depended_on_by_count, }, ]; diff --git a/frontend/packages/swarm-ui/src/ui/table/Table.tsx b/frontend/packages/swarm-ui/src/ui/table/Table.tsx index 5814edbd..8c14b423 100644 --- a/frontend/packages/swarm-ui/src/ui/table/Table.tsx +++ b/frontend/packages/swarm-ui/src/ui/table/Table.tsx @@ -20,6 +20,17 @@ export interface TableColumn { // can pass real markup instead of forking a second table primitive. header: ComponentChildren; render: (row: T) => ComponentChildren; + /** + * ARIA sort state for this column's `` — `'ascending'` / + * `'descending'` while this is the active sort column, `'none'` while + * sortable but not active, omitted entirely for a non-sortable column + * (no `aria-sort` attribute at all, the correct value for a column + * that can never be the active sort). A screen reader announces which + * column/direction is active from this attribute; the ▲/▼ glyph a + * sortable header renders is `aria-hidden` and carries no information + * on its own. + */ + ariaSort?: 'ascending' | 'descending' | 'none'; } export function Table({ @@ -44,7 +55,9 @@ export function Table({ {columns.map((c) => ( - {c.header} + + {c.header} + ))}