diff --git a/src/components/chat/ChatModal.tsx b/src/components/chat/ChatModal.tsx index 4e87bc7e..0fa67c0e 100644 --- a/src/components/chat/ChatModal.tsx +++ b/src/components/chat/ChatModal.tsx @@ -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=` 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 | 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(`[data-mid="${CSS.escape(focusId)}"]`) + : null; + if (focused) flash(focused.dataset.mid!); const divider = el.querySelector(".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; diff --git a/src/contexts/chat/ChatProvider.tsx b/src/contexts/chat/ChatProvider.tsx index 70e4a2b1..d395dfba 100644 --- a/src/contexts/chat/ChatProvider.tsx +++ b/src/contexts/chat/ChatProvider.tsx @@ -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=` 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 && ( - + )} diff --git a/src/pages/Notifications.tsx b/src/pages/Notifications.tsx index a37f404b..e2285ec2 100644 --- a/src/pages/Notifications.tsx +++ b/src/pages/Notifications.tsx @@ -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); } } diff --git a/src/test/components/chat/ChatModal.test.tsx b/src/test/components/chat/ChatModal.test.tsx index 9301376a..6c1eaa2a 100644 --- a/src/test/components/chat/ChatModal.test.tsx +++ b/src/test/components/chat/ChatModal.test.tsx @@ -12,12 +12,16 @@ const me = makeSession({ id: "me", username: "me" }); async function setup( history: ChatMessage[] = [], - { session = me, welcome = {} as Record } = {}, + { + session = me, + welcome = {} as Record, + focusId = undefined as string | undefined, + } = {}, ) { route("GET /api/chat", () => wire(history)); const onClose = vi.fn(); const user = userEvent.setup(); - renderApp(, { session }); + renderApp(, { 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)); diff --git a/src/test/pages/Notifications.test.tsx b/src/test/pages/Notifications.test.tsx index 16e5a227..2332a812 100644 --- a/src/test/pages/Notifications.test.tsx +++ b/src/test/pages/Notifications.test.tsx @@ -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();