fix(notifications): Earlier opens on Back only for a row that can be nowhere else, and Mark read reads read at once

A namesake in Earlier while the row itself was still in Needs you or Recent opened the fold, which took the place match away and left the name twice on the page: the focus fell to <body>. Row names are trimmed as Back saves them, so a title ending in a space still matches. Mark read handed the focus to a row that still said it was unread until the write landed; the row now reads read in the press's own render, and unread again if the write is refused.
This commit is contained in:
2026-10-29 01:31:40 +01:00
parent d6e72d7576
commit b22e903174
2 changed files with 101 additions and 9 deletions
@@ -173,6 +173,48 @@ describe("NotificationsPanel · order", () => {
).toBe("true"); ).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
// <body> 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 () => { 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.server = [server(1, 1 * H, false), server(2, 2 * H, true), server(3, 10 * D, true)];
h.client = [client(4, 3 * H)]; 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<HTMLElement>("[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<HTMLElement>("[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 () => { 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 // 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. // left by it: "Download ready" alone was every download's name.
@@ -123,11 +123,25 @@ export default function NotificationsPanel() {
qc.invalidateQueries({ queryKey: qk.notifications() }); qc.invalidateQueries({ queryKey: qk.notifications() });
qc.invalidateQueries({ queryKey: qk.notifUnread() }); 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<ReadonlySet<number>>(() => new Set());
const readMut = useMutation({ const readMut = useMutation({
mutationFn: api.markNotificationRead, mutationFn: api.markNotificationRead,
onSuccess: refresh, 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" }, meta: { failTitle: "errors.writeFailed.notifRead" },
}); });
function markRead(id: number) {
setReadNow((s) => new Set(s).add(id));
readMut.mutate(id);
}
const readAllMut = useMutation({ const readAllMut = useMutation({
mutationFn: api.markAllNotificationsRead, mutationFn: api.markAllNotificationsRead,
// Settled, not only succeeded: a refused write brings the rows back unread at once, rather than // 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 // 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 // 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. // the band would take them back and hold them for the rest of the visit.
const shownServer = readAllMut.isPending const shownServer = serverItems.map((n) =>
? serverItems.map((n) => (n.read ? n : { ...n, read: true })) n.read || !(readAllMut.isPending || readNow.has(n.id)) ? n : { ...n, read: true },
: serverItems; );
const order = orderInbox(shownServer, clientItems, { now: openedAt, held }); const order = orderInbox(shownServer, clientItems, { now: openedAt, held });
const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k)); const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k));
if (asking.length > 0) setHeld(new Set([...held, ...asking])); if (asking.length > 0) setHeld(new Set([...held, ...asking]));
if (seeking && q.data) { if (seeking && q.data) {
setSeeking(null); 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); const serverUnread = serverItems.some((n) => !n.read);
@@ -315,7 +333,7 @@ export default function NotificationsPanel() {
id: "read", id: "read",
label: t("inbox.markRead"), label: t("inbox.markRead"),
icon: icon(Check), icon: icon(Check),
onSelect: () => readMut.mutate(n.id), onSelect: () => markRead(n.id),
}, },
] ]
: []), : []),
@@ -404,7 +422,7 @@ export default function NotificationsPanel() {
navIndex={i} navIndex={i}
rowProps={serverMenu.triggerProps(e.n)} rowProps={serverMenu.triggerProps(e.n)}
more={more(serverMenu, e.n)} more={more(serverMenu, e.n)}
onRead={() => readMut.mutate(e.n.id)} onRead={() => markRead(e.n.id)}
onDismiss={() => dismissMut.mutate(e.n.id)} onDismiss={() => dismissMut.mutate(e.n.id)}
onOpenScheduler={openScheduler} onOpenScheduler={openScheduler}
onOpenTakeout={openTakeout} onOpenTakeout={openTakeout}
@@ -685,11 +703,12 @@ function serverRowText(
return { title, 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. */ /** 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 => const rowName = (title: string, body: string | null | undefined): string =>
body ? `${title}: ${body}` : title; (body ? `${title}: ${body}` : title).trim();
const clientRowName = (n: ClientNotif): string => 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 => { const entryName = (e: InboxEntry, t: TFunction, lang: string): string => {
if (e.src === "client") return clientRowName(e.n); if (e.src === "client") return clientRowName(e.n);
const { title, body } = serverRowText(e.n, t, lang); const { title, body } = serverRowText(e.n, t, lang);