fix(notifications): Mark all read waits for the fresh list, is not offered where it cannot act, and a row says it is unread
The mask over a stale read lifted when the write settled, before the refetch landed, and the band took the rows back. A band holding only what asks by its kind (an access-requests nudge) left the button enabled with nothing to do. The unread dot was only drawn: the spoken label now says it, apart from the name Back finds the row by, and follows the row while it holds the focus.
This commit is contained in:
@@ -6,6 +6,7 @@
|
|||||||
"clearConfirmTitle": "Clear every notification?",
|
"clearConfirmTitle": "Clear every notification?",
|
||||||
"clearConfirmBody": "Every notification here leaves the list, read or not. They don't come back.",
|
"clearConfirmBody": "Every notification here leaves the list, read or not. They don't come back.",
|
||||||
"markRead": "Mark read",
|
"markRead": "Mark read",
|
||||||
|
"unreadName": "{{name}}, unread",
|
||||||
"dismiss": "Dismiss",
|
"dismiss": "Dismiss",
|
||||||
"rowMenu": "Notification actions",
|
"rowMenu": "Notification actions",
|
||||||
"menu": "Notifications actions",
|
"menu": "Notifications actions",
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
"clearConfirmTitle": "Törlöd az összes értesítést?",
|
"clearConfirmTitle": "Törlöd az összes értesítést?",
|
||||||
"clearConfirmBody": "Minden itteni értesítés eltűnik a listából, olvasott és olvasatlan is. Nem hozhatók vissza.",
|
"clearConfirmBody": "Minden itteni értesítés eltűnik a listából, olvasott és olvasatlan is. Nem hozhatók vissza.",
|
||||||
"markRead": "Olvasottnak jelöl",
|
"markRead": "Olvasottnak jelöl",
|
||||||
|
"unreadName": "{{name}}, olvasatlan",
|
||||||
"dismiss": "Elrejtés",
|
"dismiss": "Elrejtés",
|
||||||
"rowMenu": "Értesítés műveletei",
|
"rowMenu": "Értesítés műveletei",
|
||||||
"menu": "Értesítések műveletei",
|
"menu": "Értesítések műveletei",
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ export const entryKey = (e: InboxEntry): string => keyOf(e.src, e.n.id);
|
|||||||
/** Whether a client notice asks for the reader by what it is: an access-requests nudge for as long
|
/** Whether a client notice asks for the reader by what it is: an access-requests nudge for as long
|
||||||
* as it exists (its producer removes it when nothing is pending), a notice that waits on an
|
* as it exists (its producer removes it when nothing is pending), a notice that waits on an
|
||||||
* interaction until it is dealt with. */
|
* interaction until it is dealt with. */
|
||||||
const clientAsks = (n: ClientNotif): boolean =>
|
export const clientAsks = (n: ClientNotif): boolean =>
|
||||||
n.meta?.kind === "access-requests" || (n.requiresInteraction && !n.dismissed);
|
n.meta?.kind === "access-requests" || (n.requiresInteraction && !n.dismissed);
|
||||||
|
|
||||||
export function orderInbox(
|
export function orderInbox(
|
||||||
|
|||||||
@@ -42,6 +42,15 @@ const lists = new Map<HTMLElement, NavList>();
|
|||||||
// The item we gave a tabindex to, so the next move can take it back — and whether we named it too.
|
// The item we gave a tabindex to, so the next move can take it back — and whether we named it too.
|
||||||
let roved: HTMLElement | null = null;
|
let roved: HTMLElement | null = null;
|
||||||
let named = false;
|
let named = false;
|
||||||
|
// A named item's label follows its spoken state while it holds the focus: Mark read hands the
|
||||||
|
// focus to the row BEFORE the row reads read, and the label given then said "unread" until the
|
||||||
|
// keyboard moved on.
|
||||||
|
const relabel =
|
||||||
|
typeof MutationObserver === "undefined"
|
||||||
|
? null
|
||||||
|
: new MutationObserver(() => {
|
||||||
|
if (named && roved) roved.setAttribute("aria-label", spokenName(roved));
|
||||||
|
});
|
||||||
|
|
||||||
const INDEX = "data-nav-index";
|
const INDEX = "data-nav-index";
|
||||||
const LIST = "data-nav-list";
|
const LIST = "data-nav-list";
|
||||||
@@ -56,14 +65,24 @@ const REVEAL_FRAMES = 10;
|
|||||||
* `inset` draws the keyboard ring INSIDE the item. The ring sits 2 px outside by default, which a
|
* `inset` draws the keyboard ring INSIDE the item. The ring sits 2 px outside by default, which a
|
||||||
* card with a gap round it has room for; an item flush against its neighbours (a log line) or against
|
* card with a gap round it has room for; an item flush against its neighbours (a log line) or against
|
||||||
* the edge of a clipping card (a Plex playlist show header) does not: the ring ran past the list, or
|
* the edge of a clipping card (a Plex playlist show header) does not: the ring ran past the list, or
|
||||||
* was clipped away entirely. A table row needs no flag: index.css insets every `tr` by its tag. */
|
* was clipped away entirely. A table row needs no flag: index.css insets every `tr` by its tag.
|
||||||
|
*
|
||||||
|
* `label` is what is SPOKEN instead of `name`, when it carries a state the item can lose while the
|
||||||
|
* page is away (a notification's "unread"): `name` is also how Back finds the item again, and a
|
||||||
|
* name that changed with the state would miss it. */
|
||||||
export const navItemAttrs = (
|
export const navItemAttrs = (
|
||||||
index: number,
|
index: number,
|
||||||
name?: string | null,
|
name?: string | null,
|
||||||
{ inset = false }: { inset?: boolean } = {},
|
{ inset = false, label }: { inset?: boolean; label?: string } = {},
|
||||||
): { "data-nav-index": number; "data-nav-name"?: string; "data-nav-inset"?: "" } => ({
|
): {
|
||||||
|
"data-nav-index": number;
|
||||||
|
"data-nav-name"?: string;
|
||||||
|
"data-nav-label"?: string;
|
||||||
|
"data-nav-inset"?: "";
|
||||||
|
} => ({
|
||||||
[INDEX]: index,
|
[INDEX]: index,
|
||||||
...(name ? { "data-nav-name": name } : null),
|
...(name ? { "data-nav-name": name } : null),
|
||||||
|
...(label ? { "data-nav-label": label } : null),
|
||||||
...(inset ? { "data-nav-inset": "" } : null),
|
...(inset ? { "data-nav-inset": "" } : null),
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -204,11 +223,15 @@ function itemName(el: HTMLElement): string {
|
|||||||
).trim();
|
).trim();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** What a screen reader hears while the item holds the focus: its `label` when it gives one. */
|
||||||
|
const spokenName = (el: HTMLElement): string => el.getAttribute("data-nav-label") || itemName(el);
|
||||||
|
|
||||||
function focusItem(el: HTMLElement, reveal = true): void {
|
function focusItem(el: HTMLElement, reveal = true): void {
|
||||||
if (roved !== el) {
|
if (roved !== el) {
|
||||||
if (named) roved?.removeAttribute("role");
|
if (named) roved?.removeAttribute("role");
|
||||||
if (named) roved?.removeAttribute("aria-label");
|
if (named) roved?.removeAttribute("aria-label");
|
||||||
roved?.removeAttribute("tabindex");
|
roved?.removeAttribute("tabindex");
|
||||||
|
relabel?.disconnect();
|
||||||
roved = null;
|
roved = null;
|
||||||
named = false;
|
named = false;
|
||||||
// An item that is a Tab stop in its own right (a native <button> tile, a role=button card with
|
// An item that is a Tab stop in its own right (a native <button> tile, a role=button card with
|
||||||
@@ -219,7 +242,7 @@ function focusItem(el: HTMLElement, reveal = true): void {
|
|||||||
// A card root is a plain <div>: focused, it has no role and no name, and a screen reader read it
|
// A card root is a plain <div>: focused, it has no role and no name, and a screen reader read it
|
||||||
// as an unlabelled run of text. While it holds the focus it is a group named after what Enter
|
// as an unlabelled run of text. While it holds the focus it is a group named after what Enter
|
||||||
// opens. A row (<tr>) keeps its own role.
|
// opens. A row (<tr>) keeps its own role.
|
||||||
const name = itemName(el);
|
const name = spokenName(el);
|
||||||
if (
|
if (
|
||||||
el.tagName === "DIV" &&
|
el.tagName === "DIV" &&
|
||||||
name &&
|
name &&
|
||||||
@@ -229,6 +252,7 @@ function focusItem(el: HTMLElement, reveal = true): void {
|
|||||||
el.setAttribute("role", "group");
|
el.setAttribute("role", "group");
|
||||||
el.setAttribute("aria-label", name);
|
el.setAttribute("aria-label", name);
|
||||||
named = true;
|
named = true;
|
||||||
|
relabel?.observe(el, { attributeFilter: ["data-nav-label", "data-nav-name"] });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ vi.mock("../../components/ui/PageShell", async (importOriginal) => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
import NotificationsPanel from "./NotificationsPanel";
|
import NotificationsPanel from "./NotificationsPanel";
|
||||||
|
import { focusNavItem } from "../../lib/listNav";
|
||||||
|
|
||||||
// The server list goes through the real live query, so an optimistic patch of its cache is seen.
|
// The server list goes through the real live query, so an optimistic patch of its cache is seen.
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -256,6 +257,61 @@ describe("NotificationsPanel · order", () => {
|
|||||||
expect(listUnder("inbox.sectionRecent")).toEqual(["Ready 1"]);
|
expect(listUnder("inbox.sectionRecent")).toEqual(["Ready 1"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("the rows stay out of the band until the read after the write has landed", async () => {
|
||||||
|
// The write settling is not the list being fresh: a refetch over the network takes a moment,
|
||||||
|
// and until it lands the cache still holds what the read before the commit said.
|
||||||
|
let commit!: () => void;
|
||||||
|
vi.spyOn(api, "markAllNotificationsRead").mockImplementation(
|
||||||
|
() => new Promise((resolve) => (commit = () => resolve({ ok: true } as never))),
|
||||||
|
);
|
||||||
|
h.server = [server(1, 1 * H, false)];
|
||||||
|
h.client = [];
|
||||||
|
const { qc } = mount();
|
||||||
|
await loaded();
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: "inbox.markAllRead" }));
|
||||||
|
await waitFor(() => expect(api.markAllNotificationsRead).toHaveBeenCalled());
|
||||||
|
void qc.invalidateQueries({ queryKey: ["notifications"] });
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(qc.getQueryData<{ items: AppNotification[] }>(["notifications"])?.items[0]?.read).toBe(
|
||||||
|
false,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
let land!: () => void;
|
||||||
|
vi.mocked(api.notifications).mockImplementation(
|
||||||
|
() =>
|
||||||
|
new Promise((resolve) => {
|
||||||
|
land = () => resolve({ items: [server(1, 1 * H, true)], total: 1 });
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await act(async () => commit());
|
||||||
|
await act(() => new Promise((r) => setTimeout(r, 30)));
|
||||||
|
expect(screen.queryByText(/inbox\.sectionNeedsYou/)).toBeNull();
|
||||||
|
await act(async () => land());
|
||||||
|
await waitFor(() => expect(listUnder("inbox.sectionRecent")).toEqual(["Ready 1"]));
|
||||||
|
expect(screen.queryByText(/inbox\.sectionNeedsYou/)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not offer Mark all read for a band that only holds what asks by its kind", async () => {
|
||||||
|
// An access-requests nudge stays up for as long as requests wait: the gesture cannot empty
|
||||||
|
// that band, and a button that does nothing when pressed is not offered.
|
||||||
|
h.server = [server(1, 1 * H, true)];
|
||||||
|
h.client = [
|
||||||
|
client(9, 5 * 60 * 1000, {
|
||||||
|
level: "warning",
|
||||||
|
title: "common.accessRequestsTitle",
|
||||||
|
message: "1 pending",
|
||||||
|
meta: { kind: "access-requests" },
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
mount();
|
||||||
|
await loaded();
|
||||||
|
expect(screen.getByText("inbox.sectionNeedsYou:1")).toBeTruthy();
|
||||||
|
const button = screen.getByRole("button", { name: "inbox.markAllRead" });
|
||||||
|
expect(button.hasAttribute("disabled") || button.getAttribute("aria-disabled") === "true").toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("a refused Mark all read brings the rows back at once, not at the next poll", async () => {
|
it("a refused Mark all read brings the rows back at once, not at the next poll", async () => {
|
||||||
vi.spyOn(api, "markAllNotificationsRead").mockRejectedValue(new Error("offline"));
|
vi.spyOn(api, "markAllNotificationsRead").mockRejectedValue(new Error("offline"));
|
||||||
h.server = [server(1, 1 * H, false)];
|
h.server = [server(1, 1 * H, false)];
|
||||||
@@ -270,6 +326,42 @@ describe("NotificationsPanel · order", () => {
|
|||||||
await waitFor(() => expect(listUnder("inbox.sectionNeedsYou:1")).toEqual(["Ready 1"]));
|
await waitFor(() => expect(listUnder("inbox.sectionNeedsYou:1")).toEqual(["Ready 1"]));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("says a row is unread while the keyboard is on it; its name for Back stays the same", async () => {
|
||||||
|
// The unread dot is only drawn: a read row held in Needs you sounded the same as an unread one.
|
||||||
|
vi.spyOn(api, "markNotificationRead").mockResolvedValue({ ok: true } as never);
|
||||||
|
h.server = [server(1, 1 * H, false), server(2, 2 * H, true)];
|
||||||
|
h.client = [];
|
||||||
|
mount();
|
||||||
|
await loaded();
|
||||||
|
const spoken = (text: string) => {
|
||||||
|
const row = screen.getByText(text).closest<HTMLElement>("[data-nav-index]")!;
|
||||||
|
act(() => focusNavItem(row.closest("section")!, Number(row.getAttribute("data-nav-index"))));
|
||||||
|
return [
|
||||||
|
document.activeElement?.getAttribute("aria-label"),
|
||||||
|
row.getAttribute("data-nav-name"),
|
||||||
|
];
|
||||||
|
};
|
||||||
|
expect(spoken("Ready 1")).toEqual(["inbox.unreadName", "inbox.downloadDone.title: Ready 1"]);
|
||||||
|
expect(spoken("Ready 2")).toEqual([
|
||||||
|
"inbox.downloadDone.title: Ready 2",
|
||||||
|
"inbox.downloadDone.title: Ready 2",
|
||||||
|
]);
|
||||||
|
// Marked read from its button, the row takes the focus before it reads read: what it is called
|
||||||
|
// follows once it does.
|
||||||
|
const row = screen.getByText("Ready 1").closest<HTMLElement>("[data-nav-index]")!;
|
||||||
|
const button = within(row).getByRole("button", { name: "inbox.markRead" });
|
||||||
|
button.focus();
|
||||||
|
h.server = [server(1, 1 * H, true), server(2, 2 * H, true)];
|
||||||
|
fireEvent.click(button);
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(within(row).queryByRole("button", { name: "inbox.markRead" })).toBeNull(),
|
||||||
|
);
|
||||||
|
expect(document.activeElement).toBe(row);
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(row.getAttribute("aria-label")).toBe("inbox.downloadDone.title: Ready 1"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
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.
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ import {
|
|||||||
type VideoWatchedMeta,
|
type VideoWatchedMeta,
|
||||||
} from "../../lib/notifications";
|
} from "../../lib/notifications";
|
||||||
import { channelYouTubeUrl, formatDate } from "../../lib/format";
|
import { channelYouTubeUrl, formatDate } from "../../lib/format";
|
||||||
import { entryKey, keyOf, orderInbox, type InboxEntry } from "../../lib/inboxOrder";
|
import { clientAsks, entryKey, keyOf, orderInbox, type InboxEntry } from "../../lib/inboxOrder";
|
||||||
import { Button } from "../../components/ui/Button";
|
import { Button } from "../../components/ui/Button";
|
||||||
import { IconButton } from "../../components/ui/IconButton";
|
import { IconButton } from "../../components/ui/IconButton";
|
||||||
import { Pressable } from "../../components/ui/Pressable";
|
import { Pressable } from "../../components/ui/Pressable";
|
||||||
@@ -128,7 +128,13 @@ export default function NotificationsPanel() {
|
|||||||
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
|
||||||
// leaving them read until the next poll moves them under the reader.
|
// leaving them read until the next poll moves them under the reader.
|
||||||
onSettled: refresh,
|
// And it settles once the read after it has landed: until then the cache holds what a read
|
||||||
|
// before the commit said, and the band would take those rows back the moment the mask below lifts.
|
||||||
|
onSettled: () =>
|
||||||
|
Promise.all([
|
||||||
|
qc.invalidateQueries({ queryKey: qk.notifications() }),
|
||||||
|
qc.invalidateQueries({ queryKey: qk.notifUnread() }),
|
||||||
|
]),
|
||||||
meta: { failTitle: "errors.writeFailed.notifReadAll" },
|
meta: { failTitle: "errors.writeFailed.notifReadAll" },
|
||||||
});
|
});
|
||||||
const dismissMut = useMutation({
|
const dismissMut = useMutation({
|
||||||
@@ -154,8 +160,12 @@ export default function NotificationsPanel() {
|
|||||||
|
|
||||||
const serverUnread = serverItems.some((n) => !n.read);
|
const serverUnread = serverItems.some((n) => !n.read);
|
||||||
const hasAny = serverItems.length > 0 || clientItems.length > 0;
|
const hasAny = serverItems.length > 0 || clientItems.length > 0;
|
||||||
// A band of rows already read still offers the gesture that empties it.
|
// A band of rows already read still offers the gesture that empties it — but not one that only
|
||||||
const canMarkAll = serverUnread || clientUnread > 0 || order.needsYou.length > 0;
|
// holds what asks by its kind (an access-requests nudge): pressing it would change nothing.
|
||||||
|
const canMarkAll =
|
||||||
|
serverUnread ||
|
||||||
|
clientUnread > 0 ||
|
||||||
|
order.needsYou.some((e) => e.src === "server" || !clientAsks(e.n));
|
||||||
|
|
||||||
function markAll() {
|
function markAll() {
|
||||||
markAllClientRead();
|
markAllClientRead();
|
||||||
@@ -688,11 +698,15 @@ function NotificationRow({
|
|||||||
title = t(DONE_TITLE[d.kind ?? ""] ?? "inbox.downloadDone.title");
|
title = t(DONE_TITLE[d.kind ?? ""] ?? "inbox.downloadDone.title");
|
||||||
body = d.title || n.body;
|
body = d.title || n.body;
|
||||||
}
|
}
|
||||||
|
// 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;
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
// Named by what it is about, not its kind alone: every "Download ready" read the same to a
|
// The unread state is spoken too (the dot is only drawn), apart from the name Back finds it by.
|
||||||
// screen reader, and Back could not tell the row it was opened from by its name.
|
{...navItemAttrs(navIndex, name, {
|
||||||
{...navItemAttrs(navIndex, body ? `${title}: ${body}` : title)}
|
label: n.read ? undefined : t("inbox.unreadName", { name }),
|
||||||
|
})}
|
||||||
{...rowProps}
|
{...rowProps}
|
||||||
className={`${SURFACE.card} group p-3.5 flex items-start gap-3 transition ${
|
className={`${SURFACE.card} group p-3.5 flex items-start gap-3 transition ${
|
||||||
n.read ? "opacity-70" : ""
|
n.read ? "opacity-70" : ""
|
||||||
|
|||||||
Reference in New Issue
Block a user