From d6e72d7576c1bb9e3b351f95a7f7e43fca327bea Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 29 Oct 2026 00:47:52 +0100 Subject: [PATCH] fix(notifications): Back to a row that folded away since opens Earlier on it --- docs/DESIGN.md | 3 +- .../NotificationsPanel.order.dom.test.tsx | 30 +++++- .../notifications/NotificationsPanel.tsx | 101 ++++++++++++------ 3 files changed, 97 insertions(+), 37 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 7273b150..01c946fa 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -381,7 +381,8 @@ Which actions are visible and which live in a menu. "…" holds find / undo for an activity notice. The notification center lists what asks for the reader first ("Needs you · n": an unread row, a notice waiting on an action — one read during the visit keeps its place there until Mark all read), then one stream of both sources, newest first, - and folds read rows older than a week under "Earlier (n)", a heading-style disclosure, closed. Its band keeps Mark all read + and folds read rows older than a week under "Earlier (n)", a heading-style disclosure, closed — Back to a row + that folded away since opens it on that row. Its band keeps Mark all read and puts Clear everything, now behind a confirm, in its "…"; the Scheduler's strip keeps Run all and Pause and puts Purge discovery in its "…". A list whose row objects are rebuilt each render or replaced by a poll passes `useContextMenu({ keyOf })` (its id), so the "…" still closes its own menu. A menu's diff --git a/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx b/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx index 0868fd5e..15e307b2 100644 --- a/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx +++ b/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx @@ -49,7 +49,7 @@ vi.mock("../../components/ui/PageShell", async (importOriginal) => ({ })); import NotificationsPanel from "./NotificationsPanel"; -import { focusNavItem } from "../../lib/listNav"; +import { focusNavItem, restoreNavPosition } from "../../lib/listNav"; // The server list goes through the real live query, so an optimistic patch of its cache is seen. beforeEach(() => { @@ -94,7 +94,10 @@ function mount() { const qc = new QueryClient(); const tree = () => ( - + {/* The page's scroller, as in the app: Back finds a row in another of the page's lists. */} +
+ +
); const { rerender } = render(tree()); @@ -147,6 +150,29 @@ describe("NotificationsPanel · order", () => { expect(rows()).toEqual(["Ready 1", "Activity 3", "Ready 2"]); }); + it("Back to a row that folded away while the page was gone opens Earlier on it", async () => { + // Read during the visit it was opened from, the row kept its place in Needs you; on the next + // visit it is read and over a week old, and folded under Earlier the restore found no list + // holding it — the focus fell on the neighbour at its old place. + h.server = [server(1, 8 * D, false), server(2, 9 * D, true)]; + h.client = []; + const cancel = restoreNavPosition({ + list: 0, + lists: 2, + index: 0, + name: "inbox.downloadDone.title: Ready 2", + }); + mount(); + await loaded(); + await waitFor(() => + expect(document.activeElement?.closest("[data-nav-index]")?.textContent).toMatch(/Ready 2/), + ); + cancel(); + expect( + screen.getByRole("button", { name: "inbox.earlier:1" }).getAttribute("aria-expanded"), + ).toBe("true"); + }); + it("numbers each list's rows from 0 for the keyboard walk", async () => { h.server = [server(1, 1 * H, false), server(2, 2 * H, true), server(3, 10 * D, true)]; h.client = [client(4, 3 * H)]; diff --git a/frontend/src/modules/notifications/NotificationsPanel.tsx b/frontend/src/modules/notifications/NotificationsPanel.tsx index 19b69de3..f5e6ec42 100644 --- a/frontend/src/modules/notifications/NotificationsPanel.tsx +++ b/frontend/src/modules/notifications/NotificationsPanel.tsx @@ -59,7 +59,7 @@ import { useFeedFilters, useFeedFiltersActions } from "../feed"; import { SURFACE } from "../../components/ui/surfaces"; import { SectionLabel } from "../../components/ui/Headings"; import NavGrid from "../../components/ui/NavGrid"; -import { focusNavItem, navItemAttrs } from "../../lib/listNav"; +import { focusNavItem, navItemAttrs, pendingNavName } from "../../lib/listNav"; import { useContextMenu, type ContextMenuController } from "../../components/ui/useContextMenu"; import { ContextMenu } from "../../components/ui/ContextMenu"; import { MoreButton } from "../../components/ui/MoreButton"; @@ -79,7 +79,7 @@ const POLL_MS = 8000; const byId = (n: { id: number }) => n.id; export default function NotificationsPanel() { - const { t } = useTranslation(); + const { t, i18n } = useTranslation(); const { setPage } = useNavigationActions(); const { openFile } = useFilesActions(); const filters = useFeedFilters(); @@ -108,6 +108,10 @@ export default function NotificationsPanel() { ); const [openedAt] = useState(() => Date.now()); const [earlierOpen, setEarlierOpen] = useState(false); + // Back to a row that folded away since it was opened (read on that visit, and over a week old): + // Earlier opens on it, or the restore finds no list holding it and the focus falls on the row now + // at its old place. Read once, as the center opens; settled once the server's list is in. + const [seeking, setSeeking] = useState(pendingNavName); // Opening the center marks the (ephemeral) client items read, matching the old bell. Server // items stay unread until acted on individually. @@ -157,6 +161,10 @@ export default function NotificationsPanel() { const order = orderInbox(shownServer, clientItems, { now: openedAt, held }); const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k)); if (asking.length > 0) setHeld(new Set([...held, ...asking])); + if (seeking && q.data) { + setSeeking(null); + if (order.earlier.some((e) => entryName(e, t, i18n.language) === seeking)) setEarlierOpen(true); + } const serverUnread = serverItems.some((n) => !n.read); const hasAny = serverItems.length > 0 || clientItems.length > 0; @@ -614,36 +622,12 @@ function RowEnd({ ); } -function NotificationRow({ - n, - t, - navIndex, - rowProps, - more, - onRead, - onDismiss, - onOpenScheduler, - onOpenTakeout, - onOpenFile, - onOpenTokens, -}: { - n: AppNotification; - t: TFunction; - navIndex: number; - rowProps: Record; - more: ReactNode; - onRead: () => void; - onDismiss: () => void; - onOpenScheduler: () => void; - onOpenTakeout: () => void; - onOpenFile: () => void; - onOpenTokens: () => void; -}) { - const lang = useTranslation().i18n.language; - // The maintenance producer ships the affected videos in `data.videos`; list a few titles. - const videos: { id: string; title: string | null }[] = Array.isArray(n.data?.videos) - ? n.data!.videos - : []; +/** What a server row says: its title and body — in the reader's language for a known type. */ +function serverRowText( + n: AppNotification, + t: TFunction, + lang: string, +): { title: string; body: string | null } { // Known notification types are rendered from i18n (so they're trilingual) using the typed // payload; the server-stored title/body are an English fallback for any unknown type. const isMaintenance = n.type === "maintenance" && typeof n.data?.count === "number"; @@ -698,9 +682,58 @@ function NotificationRow({ title = t(DONE_TITLE[d.kind ?? ""] ?? "inbox.downloadDone.title"); body = d.title || n.body; } + return { title, body }; +} + +/** A row's name, for a screen reader and for Back to find it by: what it is about, not its kind. */ +const rowName = (title: string, body: string | null | undefined): string => + body ? `${title}: ${body}` : title; +const clientRowName = (n: ClientNotif): string => + n.title ? rowName(n.title, n.message) : n.message; +const entryName = (e: InboxEntry, t: TFunction, lang: string): string => { + if (e.src === "client") return clientRowName(e.n); + const { title, body } = serverRowText(e.n, t, lang); + return rowName(title, body); +}; + +function NotificationRow({ + n, + t, + navIndex, + rowProps, + more, + onRead, + onDismiss, + onOpenScheduler, + onOpenTakeout, + onOpenFile, + onOpenTokens, +}: { + n: AppNotification; + t: TFunction; + navIndex: number; + rowProps: Record; + more: ReactNode; + onRead: () => void; + onDismiss: () => void; + onOpenScheduler: () => void; + onOpenTakeout: () => void; + onOpenFile: () => void; + onOpenTokens: () => void; +}) { + const lang = useTranslation().i18n.language; + // The maintenance producer ships the affected videos in `data.videos`; list a few titles. + const videos: { id: string; title: string | null }[] = Array.isArray(n.data?.videos) + ? n.data!.videos + : []; + const { title, body } = serverRowText(n, t, lang); + const isTakeout = serverKind(n) === "takeout"; + const isScheduler = serverKind(n) === "scheduler" || isTakeout; + const isDownloadDone = serverKind(n) === "download"; + const isTokenExpiring = serverKind(n) === "token"; // Named by what it is about, not its kind alone: every "Download ready" read the same to a screen // reader, and Back could not tell the row it was opened from by its name. - const name = body ? `${title}: ${body}` : title; + const name = rowName(title, body); return (