feat(notifications): what asks for you comes first, then one stream; old read rows fold
The center showed two fixed sections, the server's above this browser's, so an admin's "access requests waiting" sat under every server row, week-old read "Download ready" notices among them. One order now, for both sources (lib/inboxOrder): "Needs you" (unread, an access-requests nudge, a notice waiting on an interaction), then everything else newest first, and read rows older than a week fold under "Earlier (n)", closed by default. A row that asked in this visit keeps its place once read: marking it read no longer moves it to another list under the reader. Mark all read empties the band, the client notices of the visit included (the server rows read at once, not when the write lands). Marking a row read from its button hands the focus to the row instead of dropping it on <body> as the button goes.
This commit is contained in:
@@ -0,0 +1,212 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// The notification center's order: what asks for the reader comes first, whichever source it is
|
||||
// from, then one time-ordered stream; read rows older than a week fold under "Earlier (n)". It was
|
||||
// two fixed sections, the server's above this browser's, so an admin's "access requests waiting"
|
||||
// sat under every server row — week-old read "Download ready" notices among them.
|
||||
// i18n is key-passthrough, so labels read as keys.
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { api, type AppNotification } from "../../lib/api";
|
||||
import type { Notification as ClientNotif } from "../../lib/notifications";
|
||||
|
||||
const h = vi.hoisted(() => ({
|
||||
server: [] as unknown[],
|
||||
client: [] as unknown[],
|
||||
}));
|
||||
|
||||
vi.mock("react-i18next", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("react-i18next")>()),
|
||||
useTranslation: () => ({
|
||||
i18n: { language: "en" },
|
||||
t: (key: string, o?: { count?: number }) =>
|
||||
o?.count === undefined ? key : `${key}:${o.count}`,
|
||||
}),
|
||||
}));
|
||||
vi.mock("../../components/ui/confirmContext", () => ({ useConfirm: () => vi.fn() }));
|
||||
vi.mock("../../lib/notifications", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../lib/notifications")>()),
|
||||
subscribe: () => () => {},
|
||||
getNotifications: () => h.client,
|
||||
getUnreadCount: () => 0,
|
||||
markAllRead: () => {},
|
||||
}));
|
||||
vi.mock("../../features/navigation/navigationContext", () => ({
|
||||
useNavigationActions: () => ({ setPage: vi.fn() }),
|
||||
}));
|
||||
vi.mock("../files/filesContext", () => ({ useFilesActions: () => ({ openFile: vi.fn() }) }));
|
||||
vi.mock("../channels/channelsContext", () => ({
|
||||
useChannelsActions: () => ({ focusChannel: vi.fn() }),
|
||||
}));
|
||||
vi.mock("../feed/feedFiltersContext", () => ({
|
||||
useFeedFilters: () => ({}),
|
||||
useFeedFiltersActions: () => ({ setFilters: vi.fn() }),
|
||||
}));
|
||||
vi.mock("../../components/ui/PageShell", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../../components/ui/PageShell")>()),
|
||||
PageToolbar: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
||||
}));
|
||||
|
||||
import NotificationsPanel from "./NotificationsPanel";
|
||||
|
||||
// The server list goes through the real live query, so an optimistic patch of its cache is seen.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(api, "notifications").mockImplementation(async () => ({
|
||||
items: h.server as AppNotification[],
|
||||
total: h.server.length,
|
||||
}));
|
||||
});
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
const H = 3600 * 1000;
|
||||
const D = 24 * H;
|
||||
const server = (id: number, ageMs: number, read: boolean) =>
|
||||
({
|
||||
id,
|
||||
type: "download_done",
|
||||
title: `Server ${id}`,
|
||||
body: null,
|
||||
data: { title: `Ready ${id}` },
|
||||
read,
|
||||
dismissed: false,
|
||||
created_at: new Date(Date.now() - ageMs).toISOString(),
|
||||
}) as unknown as AppNotification;
|
||||
const client = (id: number, ageMs: number, extra: Partial<ClientNotif> = {}) =>
|
||||
({
|
||||
id,
|
||||
level: "success",
|
||||
message: `Activity ${id}.`,
|
||||
requiresInteraction: false,
|
||||
ts: Date.now() - ageMs,
|
||||
read: true,
|
||||
dismissed: true,
|
||||
transient: false,
|
||||
repeat: 1,
|
||||
...extra,
|
||||
}) as unknown as ClientNotif;
|
||||
|
||||
function mount() {
|
||||
const qc = new QueryClient();
|
||||
const tree = () => (
|
||||
<QueryClientProvider client={qc}>
|
||||
<NotificationsPanel />
|
||||
</QueryClientProvider>
|
||||
);
|
||||
const { rerender } = render(tree());
|
||||
return { rerender: () => rerender(tree()) };
|
||||
}
|
||||
// Every test has a "Ready 1" on screen once the server list has landed.
|
||||
const loaded = () => screen.findByText("Ready 1");
|
||||
// The rows of the list under a section's heading.
|
||||
const listUnder = (heading: string) =>
|
||||
[...screen.getByText(heading).closest("section")!.querySelectorAll("[data-nav-index]")].map(
|
||||
(r) => (r.textContent ?? "").match(/Ready \d+|Activity \d+/)?.[0],
|
||||
);
|
||||
// Every row on screen, in document order, by its text.
|
||||
const rows = () =>
|
||||
[...document.querySelectorAll<HTMLElement>("[data-nav-index]")].map(
|
||||
(r) => (r.textContent ?? "").match(/Ready \d+|Activity \d+|common\.accessRequestsTitle/)?.[0],
|
||||
);
|
||||
|
||||
describe("NotificationsPanel · order", () => {
|
||||
it("puts a waiting access-requests nudge first, above every server row", async () => {
|
||||
h.server = [server(1, 1 * H, false), server(2, 1 * D, true)];
|
||||
h.client = [
|
||||
client(9, 5 * 60 * 1000, {
|
||||
level: "warning",
|
||||
title: "common.accessRequestsTitle",
|
||||
message: "1 pending",
|
||||
meta: { kind: "access-requests" },
|
||||
}),
|
||||
client(8, 2 * D),
|
||||
];
|
||||
mount();
|
||||
await loaded();
|
||||
expect(rows()).toEqual(["common.accessRequestsTitle", "Ready 1", "Ready 2", "Activity 8"]);
|
||||
expect(screen.getByText("inbox.sectionNeedsYou:2")).toBeTruthy();
|
||||
expect(screen.getByText("inbox.sectionRecent")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("folds read rows older than a week under Earlier (n), closed until opened", async () => {
|
||||
h.server = [server(1, 1 * H, true), server(2, 10 * D, true)];
|
||||
h.client = [client(3, 9 * D)];
|
||||
mount();
|
||||
await loaded();
|
||||
expect(rows()).toEqual(["Ready 1"]);
|
||||
const fold = screen.getByRole("button", { name: "inbox.earlier:2" });
|
||||
expect(fold.getAttribute("aria-expanded")).toBe("false");
|
||||
fireEvent.click(fold);
|
||||
expect(fold.getAttribute("aria-expanded")).toBe("true");
|
||||
expect(rows()).toEqual(["Ready 1", "Activity 3", "Ready 2"]);
|
||||
});
|
||||
|
||||
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)];
|
||||
mount();
|
||||
await loaded();
|
||||
fireEvent.click(screen.getByRole("button", { name: "inbox.earlier:1" }));
|
||||
const lists = [...document.querySelectorAll("[data-nav-list]")].map((l) =>
|
||||
[...l.querySelectorAll("[data-nav-index]")].map((r) => r.getAttribute("data-nav-index")),
|
||||
);
|
||||
expect(lists).toEqual([["0"], ["0", "1"], ["0"]]);
|
||||
});
|
||||
|
||||
it("a row marked read keeps its place in Needs you for the visit, in the same list", async () => {
|
||||
vi.spyOn(api, "markNotificationRead").mockResolvedValue({ ok: true } as never);
|
||||
h.server = [server(1, 1 * H, false), server(2, 2 * H, true)];
|
||||
h.client = [];
|
||||
const { rerender } = mount();
|
||||
await loaded();
|
||||
const before = screen.getByText("Ready 1").closest("[data-nav-index]");
|
||||
fireEvent.click(within(before as HTMLElement).getByRole("button", { name: "inbox.markRead" }));
|
||||
h.server = [server(1, 1 * H, true), server(2, 2 * H, true)]; // what the refetch after the write reads
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByRole("button", { name: "inbox.markRead" })).toBeNull(),
|
||||
);
|
||||
await act(async () => rerender());
|
||||
expect(listUnder("inbox.sectionNeedsYou:1")).toEqual(["Ready 1"]);
|
||||
// The same element, not a remount in another list: the keyboard focus stays on it.
|
||||
expect(screen.getByText("Ready 1").closest("[data-nav-index]")).toBe(before);
|
||||
});
|
||||
|
||||
it("Mark all read empties Needs you, the client notices of the visit included", async () => {
|
||||
vi.spyOn(api, "markAllNotificationsRead").mockImplementation(async () => {
|
||||
h.server = [server(1, 1 * H, true)];
|
||||
return { ok: true } as never;
|
||||
});
|
||||
h.server = [server(1, 1 * H, false)];
|
||||
h.client = [client(2, 2 * H, { read: false })];
|
||||
const { rerender } = mount();
|
||||
await loaded();
|
||||
expect(listUnder("inbox.sectionNeedsYou:2")).toEqual(["Ready 1", "Activity 2"]);
|
||||
h.client = [client(2, 2 * H, { read: true })]; // what the store's markAllRead leaves
|
||||
fireEvent.click(screen.getByRole("button", { name: "inbox.markAllRead" }));
|
||||
await act(async () => rerender());
|
||||
expect(screen.queryByText(/inbox\.sectionNeedsYou/)).toBeNull();
|
||||
await waitFor(() => expect(api.markAllNotificationsRead).toHaveBeenCalled());
|
||||
expect(listUnder("inbox.sectionRecent")).toEqual(["Ready 1", "Activity 2"]);
|
||||
});
|
||||
|
||||
it("marking a row read from its button hands the focus to the row, not <body>", async () => {
|
||||
vi.spyOn(api, "markNotificationRead").mockResolvedValue({ ok: true } as never);
|
||||
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();
|
||||
h.server = [server(1, 1 * H, true)]; // what the refetch after the write reads
|
||||
fireEvent.click(button);
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByRole("button", { name: "inbox.markRead" })).toBeNull(),
|
||||
);
|
||||
expect(row.isConnected).toBe(true);
|
||||
expect(document.activeElement).toBe(row);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useSyncExternalStore, type ReactNode } from "react";
|
||||
import { useEffect, useState, useSyncExternalStore, type ReactNode } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { TFunction } from "i18next";
|
||||
import { useMutation, useQueryClient } from "@tanstack/react-query";
|
||||
@@ -8,6 +8,8 @@ import {
|
||||
Bell,
|
||||
Check,
|
||||
CheckCheck,
|
||||
ChevronDown,
|
||||
ChevronRight,
|
||||
ExternalLink,
|
||||
Eye,
|
||||
FolderOpen,
|
||||
@@ -41,6 +43,7 @@ import {
|
||||
type VideoWatchedMeta,
|
||||
} from "../../lib/notifications";
|
||||
import { channelYouTubeUrl, formatDate } from "../../lib/format";
|
||||
import { entryKey, keyOf, orderInbox, type InboxEntry } from "../../lib/inboxOrder";
|
||||
import { Button } from "../../components/ui/Button";
|
||||
import { IconButton } from "../../components/ui/IconButton";
|
||||
import { LEVEL_STYLE } from "../../components/ui/toastLevels";
|
||||
@@ -55,7 +58,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 { navItemAttrs } from "../../lib/listNav";
|
||||
import { focusNavItem, navItemAttrs } from "../../lib/listNav";
|
||||
import { useContextMenu, type ContextMenuController } from "../../components/ui/useContextMenu";
|
||||
import { ContextMenu } from "../../components/ui/ContextMenu";
|
||||
import { MoreButton } from "../../components/ui/MoreButton";
|
||||
@@ -65,10 +68,11 @@ import { HOVER_REVEAL } from "../../components/ui/reveal";
|
||||
import { useConfirm } from "../../components/ui/confirmContext";
|
||||
import { RelativeTime } from "../../components/ui/RelativeTime";
|
||||
|
||||
// The single notification center. "System" = durable, server-backed notifications (inbox
|
||||
// phase 1). "Activity" = the client-side transient events (action confirmations, errors, with
|
||||
// their inline undo/find actions) that used to live behind a separate bell — folded in here so
|
||||
// there's one indicator and one place to look. Transient toasts still pop via the Toaster.
|
||||
// The single notification center, over two sources: the durable, server-backed notifications, and
|
||||
// this browser's client-side events (action confirmations, errors, with their inline undo/find
|
||||
// actions) that used to live behind a separate bell — folded in here so there's one indicator and
|
||||
// one place to look. Transient toasts still pop via the Toaster. Both sources share one order
|
||||
// (`lib/inboxOrder`): what asks for the reader first, then one stream, old read rows folded.
|
||||
const POLL_MS = 8000;
|
||||
|
||||
const byId = (n: { id: number }) => n.id;
|
||||
@@ -93,6 +97,20 @@ export default function NotificationsPanel() {
|
||||
const clientItems = useSyncExternalStore(subscribe, getNotifications, getNotifications);
|
||||
const clientUnread = useSyncExternalStore(subscribe, getUnreadCount, getUnreadCount);
|
||||
|
||||
// The rows that asked for the reader in this visit keep their place in "Needs you" once read
|
||||
// (`lib/inboxOrder`): seeded with the client notices unread at open, which the effect below marks
|
||||
// read, and grown by every row that asks while the center is open. Mark all read empties it — the
|
||||
// one gesture that means "I have seen the lot". The fold is measured from the time the center
|
||||
// opened, fixed for the visit, so a row does not fold away under the reader.
|
||||
const [held, setHeld] = useState<ReadonlySet<string>>(
|
||||
() => new Set(getNotifications().flatMap((n) => (n.read ? [] : [keyOf("client", n.id)]))),
|
||||
);
|
||||
const [openedAt] = useState(() => Date.now());
|
||||
const [earlierOpen, setEarlierOpen] = useState(false);
|
||||
const order = orderInbox(serverItems, clientItems, { now: openedAt, held });
|
||||
const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k));
|
||||
if (asking.length > 0) setHeld(new Set([...held, ...asking]));
|
||||
|
||||
// Opening the center marks the (ephemeral) client items read, matching the old bell. Server
|
||||
// items stay unread until acted on individually.
|
||||
useEffect(() => {
|
||||
@@ -130,6 +148,13 @@ export default function NotificationsPanel() {
|
||||
|
||||
function markAll() {
|
||||
markAllClientRead();
|
||||
// Read at once, not when the write lands: until then the rows still read unread, and the band
|
||||
// would take them straight back. A failed write refetches the truth (refresh, then the poll).
|
||||
void qc.cancelQueries({ queryKey: qk.notifications() });
|
||||
qc.setQueryData<{ items: AppNotification[]; total: number }>(qk.notifications(), (d) =>
|
||||
d ? { ...d, items: d.items.map((n) => (n.read ? n : { ...n, read: true })) } : d,
|
||||
);
|
||||
setHeld(new Set());
|
||||
if (serverUnread) readAllMut.mutate();
|
||||
else refresh();
|
||||
}
|
||||
@@ -340,6 +365,37 @@ export default function NotificationsPanel() {
|
||||
);
|
||||
};
|
||||
|
||||
// One row of either source; `i` is its place in the list it is rendered into.
|
||||
const entry = (e: InboxEntry, i: number) =>
|
||||
e.src === "server" ? (
|
||||
<NotificationRow
|
||||
key={entryKey(e)}
|
||||
n={e.n}
|
||||
t={t}
|
||||
navIndex={i}
|
||||
rowProps={serverMenu.triggerProps(e.n)}
|
||||
more={more(serverMenu, e.n)}
|
||||
onRead={() => readMut.mutate(e.n.id)}
|
||||
onDismiss={() => dismissMut.mutate(e.n.id)}
|
||||
onOpenScheduler={openScheduler}
|
||||
onOpenTakeout={openTakeout}
|
||||
onOpenFile={() => openDone(e.n)}
|
||||
onOpenTokens={openTokens}
|
||||
/>
|
||||
) : (
|
||||
<ClientActivityRow
|
||||
key={entryKey(e)}
|
||||
n={e.n}
|
||||
t={t}
|
||||
navIndex={i}
|
||||
rowProps={clientMenu.triggerProps(e.n)}
|
||||
more={more(clientMenu, e.n)}
|
||||
onFocusChannel={onFocusChannel}
|
||||
onReviewAccess={reviewAccess}
|
||||
onRemove={() => removeClient(e.n.id)}
|
||||
/>
|
||||
);
|
||||
|
||||
// --- The page's own menu: the header's "…" ------------------------------------------------------
|
||||
const headerMenu = useContextMenu<"inbox">();
|
||||
const headerItems: ActionMenuItem[] = [
|
||||
@@ -409,51 +465,35 @@ export default function NotificationsPanel() {
|
||||
</div>
|
||||
) : (
|
||||
<div className="space-y-4">
|
||||
{serverItems.length > 0 && (
|
||||
{order.needsYou.length > 0 && (
|
||||
<section>
|
||||
<SectionLabel className="mb-2">{t("inbox.sectionSystem")}</SectionLabel>
|
||||
<NavGrid className="flex flex-col gap-2">
|
||||
{serverItems.map((n, i) => {
|
||||
return (
|
||||
<NotificationRow
|
||||
key={n.id}
|
||||
n={n}
|
||||
t={t}
|
||||
navIndex={i}
|
||||
rowProps={serverMenu.triggerProps(n)}
|
||||
more={more(serverMenu, n)}
|
||||
onRead={() => readMut.mutate(n.id)}
|
||||
onDismiss={() => dismissMut.mutate(n.id)}
|
||||
onOpenScheduler={openScheduler}
|
||||
onOpenTakeout={openTakeout}
|
||||
onOpenFile={() => openDone(n)}
|
||||
onOpenTokens={openTokens}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</NavGrid>
|
||||
<SectionLabel className="mb-2">
|
||||
{t("inbox.sectionNeedsYou", { count: order.needsYou.length })}
|
||||
</SectionLabel>
|
||||
<NavGrid className="flex flex-col gap-2">{order.needsYou.map(entry)}</NavGrid>
|
||||
</section>
|
||||
)}
|
||||
{clientItems.length > 0 && (
|
||||
{order.recent.length > 0 && (
|
||||
<section>
|
||||
<SectionLabel className="mb-2">{t("inbox.sectionActivity")}</SectionLabel>
|
||||
<NavGrid className="flex flex-col gap-2">
|
||||
{clientItems.map((n, i) => {
|
||||
return (
|
||||
<ClientActivityRow
|
||||
key={n.id}
|
||||
n={n}
|
||||
t={t}
|
||||
navIndex={i}
|
||||
rowProps={clientMenu.triggerProps(n)}
|
||||
more={more(clientMenu, n)}
|
||||
onFocusChannel={onFocusChannel}
|
||||
onReviewAccess={reviewAccess}
|
||||
onRemove={() => removeClient(n.id)}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</NavGrid>
|
||||
<SectionLabel className="mb-2">{t("inbox.sectionRecent")}</SectionLabel>
|
||||
<NavGrid className="flex flex-col gap-2">{order.recent.map(entry)}</NavGrid>
|
||||
</section>
|
||||
)}
|
||||
{order.earlier.length > 0 && (
|
||||
<section>
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
aria-expanded={earlierOpen}
|
||||
onClick={() => setEarlierOpen((o) => !o)}
|
||||
icon={earlierOpen ? <ChevronDown /> : <ChevronRight />}
|
||||
className={earlierOpen ? "mb-2" : undefined}
|
||||
>
|
||||
{t("inbox.earlier", { count: order.earlier.length })}
|
||||
</Button>
|
||||
{earlierOpen && (
|
||||
<NavGrid className="flex flex-col gap-2">{order.earlier.map(entry)}</NavGrid>
|
||||
)}
|
||||
</section>
|
||||
)}
|
||||
</div>
|
||||
@@ -517,7 +557,19 @@ function RowEnd({
|
||||
return (
|
||||
<div className="flex items-center gap-1 shrink-0">
|
||||
{onRead && (
|
||||
<IconButton label={t("inbox.markRead")} size="sm" variant="ghost" onClick={onRead}>
|
||||
<IconButton
|
||||
label={t("inbox.markRead")}
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
onClick={(e) => {
|
||||
// The button goes once the row reads read, and the focus on it would fall to <body>:
|
||||
// hand it to the row first, where j / k carry on.
|
||||
const row = e.currentTarget.closest<HTMLElement>("[data-nav-index]");
|
||||
if (row?.parentElement && document.activeElement === e.currentTarget)
|
||||
focusNavItem(row.parentElement, Number(row.getAttribute("data-nav-index")));
|
||||
onRead();
|
||||
}}
|
||||
>
|
||||
<Check className="w-4 h-4" />
|
||||
</IconButton>
|
||||
)}
|
||||
|
||||
Reference in New Issue
Block a user