v4: a chat mention notification opens the chat on the mentioned message, so the newest one clears the chat badge — it linked to ?chat=open, and the chat opened on the "new" divider whenever the unread messages didn't fit. That left the list above the bottom, so the read marker never moved and the badge stayed, while the mention itself was out of sight. It now links to ?chat=<message id>: ChatProvider opens on any ?chat= (open, or a message id passed to ChatModal as focusId), and ChatModal opens centred on that message and flashes it. When the message is within a screen of the bottom, the list stays at the bottom and reads everything, as it always has. An older mention still leaves the messages after it unread. A message outside the first page of history falls back to the divider. ChatModal's flash is now its own callback, shared with jumping to a message. 675 frontend tests pass, and tsc, eslint, deno fmt and doc:lint are clean; reproduced and checked in the app with 36 unread mentions from a second account: the newest clears the badge, an older one opens on it with the badge kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
khannurien
2026-10-09 08:56:59 +00:00
parent 4ea192e0b1
commit 256857f72e
5 changed files with 73 additions and 29 deletions

View File

@@ -103,19 +103,24 @@ type Row =
/** Passed to {@link ChatModal} by `ChatProvider`. */
export interface ChatModalProps {
/** Closes the chat, and drops the `?chat=open` deep link if there is one. */
/** Closes the chat, and drops the `?chat=` deep link if there is one. */
onClose: () => void;
/**
* The message to open on, from a `?chat=<id>` deep link. Ignored when it
* isn't in the first page of history.
*/
focusId?: string;
}
/**
* Renders the chat, opening on the "new" divider when the unread messages
* don't fit, else at the bottom. Reaching the bottom of a visible tab moves the
* read marker. The draft and its reply target survive closing and reloading
* Renders the chat, opening on `focusId`'s message, or on the "new" divider,
* when it is more than a screen above the bottom, else at the bottom. Reaching
* the bottom of a visible tab moves the read marker. The draft and its reply target survive closing and reloading
* (`utils/chat/draft.ts`). While it is open in a visible tab, the server sends
* the user no notification for the mentions they see live.
*/
export function ChatModal(
{ onClose }: ChatModalProps,
{ onClose, focusId }: ChatModalProps,
): React.JSX.Element {
const { wsStatus, connectionEpoch } = useWS();
const {
@@ -174,6 +179,13 @@ export function ChatModal(
const lastTypingSentRef = useRef(0);
const flashTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
/** Highlights message `id` for a moment, after a jump to it. */
const flash = useCallback((id: string) => {
setFlashId(id);
if (flashTimerRef.current) clearTimeout(flashTimerRef.current);
flashTimerRef.current = setTimeout(() => setFlashId(null), 1_200);
}, []);
// Relative times and "Today" go stale while the chat sits open.
const [, setTick] = useState(0);
useEffect(() => {
@@ -373,15 +385,25 @@ export function ChatModal(
return;
}
didInitialScrollRef.current = true;
// The message a notification names, centred, else the divider at the
// top. Either lower than a screen from the bottom leaves the list there,
// which reads everything.
const focused = focusId
? el.querySelector<HTMLElement>(`[data-mid="${CSS.escape(focusId)}"]`)
: null;
if (focused) flash(focused.dataset.mid!);
const divider = el.querySelector<HTMLElement>(".chat-new-sep");
if (divider) {
const target = divider.offsetTop - el.offsetTop - 8;
if (target < el.scrollHeight - el.clientHeight) {
el.scrollTop = target;
stickToBottomRef.current = false;
setAtBottom(false);
return;
}
const target = focused
? focused.offsetTop - el.offsetTop -
(el.clientHeight - focused.offsetHeight) / 2
: divider
? divider.offsetTop - el.offsetTop - 8
: null;
if (target !== null && target < el.scrollHeight - el.clientHeight) {
el.scrollTop = target;
stickToBottomRef.current = false;
setAtBottom(false);
return;
}
// A list too short to scroll never fires the event that would say so.
setAtBottom(true);
@@ -404,6 +426,8 @@ export function ChatModal(
loadState,
dividerId,
openedReadAt,
focusId,
flash,
]);
// Content also grows with no new message: an edited message easing back to
@@ -517,10 +541,8 @@ export function ChatModal(
if (!node) return;
stickToBottomRef.current = false;
node.scrollIntoView({ block: "center", behavior: "smooth" });
setFlashId(id);
if (flashTimerRef.current) clearTimeout(flashTimerRef.current);
flashTimerRef.current = setTimeout(() => setFlashId(null), 1_200);
}, []);
flash(id);
}, [flash]);
const handleSend = useCallback(() => {
if (!sendable(draft)) return;

View File

@@ -32,9 +32,9 @@ export interface ChatProviderProps {
/**
* Provides {@link ChatContext} and renders the one chat modal while it is
* open. `openChat` opens it, and so does a `?chat=open` query parameter on any
* page, which is where a chat mention notification links. It stays closed for
* visitors.
* open. `openChat` opens it, and so does a `?chat=` query parameter on any
* page: `?chat=open`, or `?chat=<message id>` to open on that message, which is
* where a chat mention notification links. It stays closed for visitors.
*/
export function ChatProvider(
{ children }: ChatProviderProps,
@@ -43,14 +43,14 @@ export function ChatProvider(
const [manuallyOpen, setManuallyOpen] = useState(false);
const [searchParams, setSearchParams] = useSearchParams();
const deepLinkOpen = searchParams.get("chat") === "open";
const isChatOpen = !!user && (manuallyOpen || deepLinkOpen);
const deepLink = searchParams.get("chat");
const isChatOpen = !!user && (manuallyOpen || !!deepLink);
const openChat = useCallback(() => setManuallyOpen(true), []);
const closeChat = useCallback(() => {
setManuallyOpen(false);
// Or a reload would open the chat again.
if (searchParams.get("chat") === "open") {
if (searchParams.has("chat")) {
const next = new URLSearchParams(searchParams);
next.delete("chat");
setSearchParams(next, { replace: true });
@@ -64,7 +64,10 @@ export function ChatProvider(
{children}
{isChatOpen && (
<Suspense>
<ChatModal onClose={closeChat} />
<ChatModal
onClose={closeChat}
focusId={deepLink && deepLink !== "open" ? deepLink : undefined}
/>
</Suspense>
)}
</ChatContext.Provider>

View File

@@ -172,8 +172,10 @@ function notificationLink(n: Notification): string | null {
}
if (d.contextType === "dump") return dumpLink(d);
// A query string alone keeps this page, and `ChatProvider` opens the
// chat over it.
if (d.contextType === "chat") return `?chat=open`;
// chat over it, on the message.
if (d.contextType === "chat") {
return `?chat=${encodeURIComponent(d.contextId)}`;
}
return playlistLink(d);
}
}

View File

@@ -12,12 +12,16 @@ const me = makeSession({ id: "me", username: "me" });
async function setup(
history: ChatMessage[] = [],
{ session = me, welcome = {} as Record<string, unknown> } = {},
{
session = me,
welcome = {} as Record<string, unknown>,
focusId = undefined as string | undefined,
} = {},
) {
route("GET /api/chat", () => wire(history));
const onClose = vi.fn();
const user = userEvent.setup();
renderApp(<ChatModal onClose={onClose} />, { session });
renderApp(<ChatModal onClose={onClose} focusId={focusId} />, { session });
const socket = FakeWebSocket.last;
socket.welcome(welcome);
await screen.findByRole("dialog", { name: "Chat" });
@@ -132,6 +136,19 @@ describe("ChatModal: history", () => {
expect(divider.nextElementSibling).toHaveTextContent("unseen");
});
test("a deep link opens on its message, flashed", async () => {
await setup([
makeChatMessage({ id: "m1", body: "hey @me" }),
makeChatMessage({ id: "m2", body: "later" }),
], { focusId: "m1" });
expect(screen.getByText(/hey/).closest(".chat-message")).toHaveClass(
"chat-message--flash",
);
expect(screen.getByText("later").closest(".chat-message")).not.toHaveClass(
"chat-message--flash",
);
});
test("reading at the bottom moves the read marker", async () => {
const { socket } = await setup([makeChatMessage({ body: "hi" })]);
await waitFor(() => expect(sent(socket, "chat_read")).toHaveLength(1));

View File

@@ -167,7 +167,7 @@ describe("Notifications", () => {
);
expect(mentions).toEqual([
"/@bob/dumps/d6#comment-c3",
"/notifications?chat=open",
"/notifications?chat=x",
"/@bob/playlists/faves",
]);
expect(screen.getByText("a comment")).toBeInTheDocument();