diff --git a/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx b/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx index 15e307b2..3099056f 100644 --- a/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx +++ b/frontend/src/modules/notifications/NotificationsPanel.order.dom.test.tsx @@ -173,6 +173,48 @@ describe("NotificationsPanel · order", () => { ).toBe("true"); }); + it("does not open Earlier for a namesake there while the row Back left is still in sight", async () => { + // A video downloaded twice, a scheduler run that failed the same way again: names repeat. The + // extra list took the place match away, the name stood twice on the page, and the focus fell to + // with Earlier open for nothing. + const twin = { ...server(3, 9 * D, true), data: { title: "Ready 1" } } as AppNotification; + h.server = [server(1, 1 * H, true), twin]; + h.client = []; + const cancel = restoreNavPosition({ + list: 0, + lists: 1, + index: 0, + name: "inbox.downloadDone.title: Ready 1", + }); + mount(); + await waitFor(() => + expect(document.activeElement?.closest("[data-nav-index]")?.textContent).toMatch(/Ready 1/), + ); + cancel(); + expect( + screen.getByRole("button", { name: "inbox.earlier:1" }).getAttribute("aria-expanded"), + ).toBe("false"); + }); + + it("opens Earlier on a row whose title ends in a space, as Back saved it trimmed", async () => { + // A caption cut at the title's length often ends on a space. + const spaced = { ...server(2, 9 * D, true), data: { title: "Ready 2 " } } as AppNotification; + h.server = [server(1, 8 * D, false), spaced]; + 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(); + }); + 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)]; @@ -388,6 +430,37 @@ describe("NotificationsPanel · order", () => { ); }); + it("a row marked read reads read as it takes the focus, not when the write lands", async () => { + // The button hands the focus to its row first: a screen reader announced "…, unread" right + // after the reader marked it read. + vi.spyOn(api, "markNotificationRead").mockImplementation(() => new Promise(() => {})); + h.server = [server(1, 1 * H, false)]; + h.client = []; + mount(); + await loaded(); + const row = screen.getByText("Ready 1").closest("[data-nav-index]")!; + const button = within(row).getByRole("button", { name: "inbox.markRead" }); + button.focus(); + // Within the press's own task (the label is read out when the task ends): no render later. + fireEvent.click(button); + await Promise.resolve(); + expect(document.activeElement).toBe(row); + expect(row.getAttribute("aria-label")).toBe("inbox.downloadDone.title: Ready 1"); + }); + + it("a refused Mark read brings the row back unread", async () => { + vi.spyOn(api, "markNotificationRead").mockRejectedValue(new Error("offline")); + h.server = [server(1, 1 * H, false)]; + h.client = []; + mount(); + await loaded(); + const row = screen.getByText("Ready 1").closest("[data-nav-index]")!; + fireEvent.click(within(row).getByRole("button", { name: "inbox.markRead" })); + await waitFor(() => + expect(within(row).queryByRole("button", { name: "inbox.markRead" })).not.toBeNull(), + ); + }); + it("names each row by what it is about, so two of one kind tell apart", async () => { // A screen reader hears the name while the keyboard is on the row, and Back finds the row it // left by it: "Download ready" alone was every download's name. diff --git a/frontend/src/modules/notifications/NotificationsPanel.tsx b/frontend/src/modules/notifications/NotificationsPanel.tsx index f5e6ec42..79c747de 100644 --- a/frontend/src/modules/notifications/NotificationsPanel.tsx +++ b/frontend/src/modules/notifications/NotificationsPanel.tsx @@ -123,11 +123,25 @@ export default function NotificationsPanel() { qc.invalidateQueries({ queryKey: qk.notifications() }); qc.invalidateQueries({ queryKey: qk.notifUnread() }); }; + // A row marked read reads read at once, in the press's own render: its Mark read button hands the + // focus to the row, and a row still unread then was announced "…, unread" until the write landed. + const [readNow, setReadNow] = useState>(() => new Set()); const readMut = useMutation({ mutationFn: api.markNotificationRead, onSuccess: refresh, + // Refused: the row reads unread again. + onError: (_e, id) => + setReadNow((s) => { + const next = new Set(s); + next.delete(id); + return next; + }), meta: { failTitle: "errors.writeFailed.notifRead" }, }); + function markRead(id: number) { + setReadNow((s) => new Set(s).add(id)); + readMut.mutate(id); + } const readAllMut = useMutation({ mutationFn: api.markAllNotificationsRead, // Settled, not only succeeded: a refused write brings the rows back unread at once, rather than @@ -155,15 +169,19 @@ export default function NotificationsPanel() { // While Mark all read is on its way, a read that lands before the server commits it (the poll, or // another row's write refreshing the list) still says "unread": those rows are read already, or // the band would take them back and hold them for the rest of the visit. - const shownServer = readAllMut.isPending - ? serverItems.map((n) => (n.read ? n : { ...n, read: true })) - : serverItems; + const shownServer = serverItems.map((n) => + n.read || !(readAllMut.isPending || readNow.has(n.id)) ? n : { ...n, read: true }, + ); 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); + // Only a row that can be nowhere else: a namesake in Earlier while the row itself is still in + // sight would open it for nothing, and the name standing twice on the page loses the row. + const sought = (e: InboxEntry) => entryName(e, t, i18n.language) === seeking; + if (!order.needsYou.some(sought) && !order.recent.some(sought) && order.earlier.some(sought)) + setEarlierOpen(true); } const serverUnread = serverItems.some((n) => !n.read); @@ -315,7 +333,7 @@ export default function NotificationsPanel() { id: "read", label: t("inbox.markRead"), icon: icon(Check), - onSelect: () => readMut.mutate(n.id), + onSelect: () => markRead(n.id), }, ] : []), @@ -404,7 +422,7 @@ export default function NotificationsPanel() { navIndex={i} rowProps={serverMenu.triggerProps(e.n)} more={more(serverMenu, e.n)} - onRead={() => readMut.mutate(e.n.id)} + onRead={() => markRead(e.n.id)} onDismiss={() => dismissMut.mutate(e.n.id)} onOpenScheduler={openScheduler} onOpenTakeout={openTakeout} @@ -685,11 +703,12 @@ function serverRowText( 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. */ +/** A row's name, for a screen reader and for Back to find it by: what it is about, not its kind. + * Trimmed as Back saves it (`lib/listNav`), or a title ending in a space never matches its row. */ const rowName = (title: string, body: string | null | undefined): string => - body ? `${title}: ${body}` : title; + (body ? `${title}: ${body}` : title).trim(); const clientRowName = (n: ClientNotif): string => - n.title ? rowName(n.title, n.message) : n.message; + n.title ? rowName(n.title, n.message) : rowName(n.message, null); 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);