Initial

app/components/NotificationDot.tsx

@@ -1,9 +1,15 @@
 import clsx from "clsx";
 import styles from "./NotificationDot.module.css";
 
-export function NotificationDot({ className }: { className?: string }) {
+export function NotificationDot({
+	className,
+	testId,
+}: {
+	className?: string;
+	testId?: string;
+}) {
 	return (
-		<span className={clsx(styles.dotWrapper, className)}>
+		<span className={clsx(styles.dotWrapper, className)} data-testid={testId}>
 			<span className={styles.pulse} />
 			<span className={styles.dot} />
 		</span>

app/components/layout/NotificationPopover.tsx

@@ -11,7 +11,10 @@ import {
 import { NOTIFICATIONS } from "~/features/notifications/notifications-contants";
 import type { RootLoaderData } from "~/root";
 import { NOTIFICATIONS_URL } from "~/utils/urls";
-import { useMarkNotificationsAsSeen } from "../../features/notifications/notifications-hooks";
+import {
+	useMarkNotificationsAsSeen,
+	useStickyUnseenIds,
+} from "../../features/notifications/notifications-hooks";
 import { SendouButton } from "../elements/Button";
 
 import styles from "./NotificationPopover.module.css";
@@ -45,6 +48,7 @@ export function NotificationContent({
 }) {
 	const { t } = useTranslation(["common"]);
 	const { refresh, isRefreshing } = useLayoutData();
+	const stickyUnseenIds = useStickyUnseenIds(notifications);
 
 	useMarkNotificationsAsSeen(unseenIds);
 
@@ -73,7 +77,10 @@ export function NotificationContent({
 						<React.Fragment key={notification.id}>
 							<NotificationItem
 								key={notification.id}
-								notification={notification}
+								notification={{
+									...notification,
+									seen: Number(!stickyUnseenIds.has(notification.id)),
+								}}
 								onClose={onClose}
 							/>
 							{i !== notifications.length - 1 && <NotificationItemDivider />}

app/components/layout/index.tsx

@@ -691,6 +691,7 @@ function SideNavUserPanel() {
 							{unseenIds.length > 0 ? (
 								<NotificationDot
 									className={sideNavStyles.sideNavFooterUnseenDot}
+									testId="notifications-bell-dot"
 								/>
 							) : null}
 							<SendouPopover

app/features/layout/LayoutDataProvider.tsx

@@ -69,6 +69,9 @@ export function LayoutDataProvider({
 		};
 	}, [load]);
 
+	// stable so effects that refresh after a mutation don't re-run every render
+	const refresh = React.useCallback(() => load(LAYOUT_DATA_ROUTE), [load]);
+
 	const newest = useNewestOf(data, fetcher.data);
 
 	useReloadOnNewDeploy(newest.buildCommit ?? "");
@@ -79,7 +82,7 @@ export function LayoutDataProvider({
 
 	const value: LayoutDataContextValue = {
 		...newest,
-		refresh: () => load(LAYOUT_DATA_ROUTE),
+		refresh,
 		isRefreshing: state !== "idle",
 	};
 

app/features/notifications/components/NotificationList.tsx

@@ -33,7 +33,12 @@ export function NotificationItem({
 			onClick={onClose}
 		>
 			<NotificationImage notification={notification}>
-				{!notification.seen ? <div className={styles.unseenDot} /> : null}
+				{!notification.seen ? (
+					<div
+						className={styles.unseenDot}
+						data-testid="notification-unseen-dot"
+					/>
+				) : null}
 			</NotificationImage>
 			<div className={styles.itemHeader}>
 				{t(

app/features/notifications/notifications-hooks.ts

@@ -1,10 +1,13 @@
 import * as React from "react";
 import { useFetcher } from "react-router";
+import { useLayoutData } from "~/features/layout/LayoutDataProvider";
 import { NOTIFICATIONS_MARK_AS_SEEN_ROUTE } from "~/utils/urls";
 
 export function useMarkNotificationsAsSeen(unseenIds: number[]) {
 	const fetcher = useFetcher();
+	const { refresh } = useLayoutData();
 	const submittedIdsRef = React.useRef(new Set<number>());
+	const refreshPendingRef = React.useRef(false);
 
 	const { submit } = fetcher;
 	React.useEffect(() => {
@@ -12,6 +15,13 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) {
 		// get submitted when the fetcher returns to idle
 		if (fetcher.state !== "idle") return;
 
+		// the bell dot reads from layout data, which the root loader does not
+		// revalidate for this action, so it has to be refetched by hand
+		if (refreshPendingRef.current) {
+			refreshPendingRef.current = false;
+			refresh();
+		}
+
 		const idsToSubmit = unseenIds.filter(
 			(id) => !submittedIdsRef.current.has(id),
 		);
@@ -20,6 +30,7 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) {
 		for (const id of idsToSubmit) {
 			submittedIdsRef.current.add(id);
 		}
+		refreshPendingRef.current = true;
 
 		submit(
 			{ notificationIds: idsToSubmit },
@@ -29,5 +40,45 @@ export function useMarkNotificationsAsSeen(unseenIds: number[]) {
 				action: NOTIFICATIONS_MARK_AS_SEEN_ROUTE,
 			},
 		);
-	}, [submit, unseenIds, fetcher.state]);
+	}, [submit, unseenIds, fetcher.state, refresh]);
+}
+
+/**
+ * Ids of the notifications to show an unseen dot for, keeping the dot for as
+ * long as the list stays open. Opening the list marks its notifications as
+ * seen right away so the bell stops claiming there is something new, and this
+ * keeps the reader from losing track of which ones those were.
+ */
+export function useStickyUnseenIds(
+	notifications: Array<{ id: number; seen: number }>,
+) {
+	const [unseenIds, setUnseenIds] = React.useState(
+		() => new Set(unseenIdsOf(notifications)),
+	);
+	const [prevNotifications, setPrevNotifications] =
+		React.useState(notifications);
+
+	if (prevNotifications !== notifications) {
+		setPrevNotifications(notifications);
+		setUnseenIds((prevUnseenIds) => {
+			const newUnseenIds = new Set(prevUnseenIds);
+
+			for (const id of unseenIdsOf(notifications)) {
+				newUnseenIds.add(id);
+			}
+
+			// optimize render by not updating state if nothing changed
+			if (newUnseenIds.size === prevUnseenIds.size) return prevUnseenIds;
+
+			return newUnseenIds;
+		});
+	}
+
+	return unseenIds;
+}
+
+function unseenIdsOf(notifications: Array<{ id: number; seen: number }>) {
+	return notifications
+		.filter((notification) => !notification.seen)
+		.map((notification) => notification.id);
 }

app/features/notifications/routes/notifications.tsx

@@ -11,7 +11,10 @@ import {
 	NotificationsList,
 } from "../components/NotificationList";
 import { loader } from "../loaders/notifications.server";
-import { useMarkNotificationsAsSeen } from "../notifications-hooks";
+import {
+	useMarkNotificationsAsSeen,
+	useStickyUnseenIds,
+} from "../notifications-hooks";
 
 export { loader };
 
@@ -27,36 +30,7 @@ export const meta: MetaFunction = (args) => {
 export default function NotificationsPage() {
 	const { t } = useTranslation(["common"]);
 	const data = useLoaderData<typeof loader>();
-	const [unseenIds, setUnseenIds] = React.useState(
-		() =>
-			new Set(
-				data.notifications
-					.filter((notification) => !notification.seen)
-					.map((notification) => notification.id),
-			),
-	);
-	const [prevNotifications, setPrevNotifications] = React.useState(
-		data.notifications,
-	);
-
-	// persist unseen dots for the duration of the page being viewed
-	if (prevNotifications !== data.notifications) {
-		setPrevNotifications(data.notifications);
-		setUnseenIds((prevUnseenIds) => {
-			const newUnseenIds = new Set(prevUnseenIds);
-
-			for (const notification of data.notifications) {
-				if (!notification.seen) {
-					newUnseenIds.add(notification.id);
-				}
-			}
-
-			// optimize render by not updating state if nothing changed
-			if (newUnseenIds.size === prevUnseenIds.size) return prevUnseenIds;
-
-			return newUnseenIds;
-		});
-	}
+	const unseenIds = useStickyUnseenIds(data.notifications);
 
 	const unSeenIdsArr = React.useMemo(() => Array.from(unseenIds), [unseenIds]);
 

e2e/notifications.spec.ts

@@ -0,0 +1,38 @@
+import { ADMIN_ID } from "~/features/admin/admin-constants";
+import { expect, impersonate, navigate, test } from "./helpers/playwright";
+import { NotificationPopover } from "./pages/layout/notification-popover";
+
+const UNSEEN_COUNT = 2;
+
+test.describe("Notifications", () => {
+	test("opening the popover clears the bell dot but keeps the unseen dots listed", async ({
+		page,
+		factories,
+	}) => {
+		for (let seasonNth = 1; seasonNth <= UNSEEN_COUNT; seasonNth++) {
+			await factories.NotificationFactory.create({
+				notification: { type: "SEASON_STARTED", meta: { seasonNth } },
+				users: [{ userId: ADMIN_ID, seen: 0 }],
+			});
+		}
+
+		await impersonate(page);
+		await navigate({ page, url: "/" });
+
+		const notifications = new NotificationPopover(page);
+		await expect(notifications.locators.bellDot).toBeVisible();
+
+		await notifications.open();
+
+		await expect(notifications.locators.bellDot).toBeHidden();
+		await expect(notifications.locators.unseenDots).toHaveCount(UNSEEN_COUNT);
+
+		await notifications.close();
+		await expect(notifications.locators.items).toHaveCount(0);
+
+		await notifications.open();
+
+		await expect(notifications.locators.items).toHaveCount(UNSEEN_COUNT);
+		await expect(notifications.locators.unseenDots).toHaveCount(0);
+	});
+});

e2e/pages/layout/notification-popover.ts

@@ -12,6 +12,10 @@ export class NotificationPopover {
 			openButton: this.page.getByTestId("notifications-button"),
 			items: this.page.getByTestId("notification-item"),
 			seeAllLink: this.page.getByTestId("notifications-see-all-button"),
+			/** Shown on the bell while unseen notifications exist. */
+			bellDot: this.page.getByTestId("notifications-bell-dot"),
+			/** Per notification, marking it as one the user has not read yet. */
+			unseenDots: this.page.getByTestId("notification-unseen-dot"),
 		};
 	}
 
@@ -19,6 +23,10 @@ export class NotificationPopover {
 		await this.locators.openButton.click();
 	}
 
+	async close() {
+		await this.page.keyboard.press("Escape");
+	}
+
 	notification(text: string) {
 		return this.locators.items.filter({ hasText: text });
 	}