mirror of
https://github.com/block/buzz.git
synced 2026-10-30 22:11:36 +00:00
Deep links to top-level messages highlight them in the main timeline
Clicking an inbox row, desktop notification, search hit, or buzz:// link that targets a top-level message (channel or DM) used to force-open the reply panel for that root, showing an empty "No replies in this branch yet" pane instead of the message in its own context. The route-target logic is now an exported pure function, getRouteTargetPanelAction: - top-level target without an explicit threadRootId -> main-timeline scroll + highlight only, no thread panel - top-level target with an explicit threadRootId (inbox "Open full thread", channel-activity rows, thread-draft auto-send) -> panel opens at that root, unchanged - reply target -> panel opens at the thread head scrolled to the reply, unchanged targetThreadRootId is plumbed from the route search params through ChannelRouteScreen/ChannelScreen into the hook. Unit tests cover the new function; the two e2e tests from #1092 are rewritten to the new contract and the deep-link reload test now re-issues the link cold since a consumed messageId no longer pins a thread param. Co-authored-by: morgmart <98432065+morgmart@users.noreply.github.com> Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
This commit is contained in:
@@ -234,6 +234,7 @@ export function ChannelRouteScreen({
|
||||
targetForumReplyId={targetReplyId}
|
||||
targetMessageEvents={targetMessageEvents}
|
||||
targetMessageId={targetMessageId}
|
||||
targetThreadRootId={targetThreadRootId}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -94,6 +94,7 @@ export function ChannelScreen({
|
||||
targetForumReplyId,
|
||||
targetMessageEvents,
|
||||
targetMessageId,
|
||||
targetThreadRootId,
|
||||
}: ChannelScreenProps) {
|
||||
const { goHome } = useAppNavigation();
|
||||
const { activeCommunity } = useCommunities();
|
||||
@@ -651,6 +652,7 @@ export function ChannelScreen({
|
||||
setThreadReplyTargetId,
|
||||
setThreadScrollTargetId,
|
||||
targetMessageId,
|
||||
targetThreadRootId,
|
||||
timelineMessages,
|
||||
});
|
||||
useThreadTargetSync({
|
||||
|
||||
@@ -22,4 +22,11 @@ export type ChannelScreenProps = {
|
||||
targetForumReplyId: string | null;
|
||||
targetMessageEvents: RelayEvent[];
|
||||
targetMessageId: string | null;
|
||||
/**
|
||||
* Thread root requested by the navigation source (`?threadRootId`, or the
|
||||
* `?thread` panel param on deep links). Deciding input for top-level route
|
||||
* targets: present → open the thread panel at that root; absent → the
|
||||
* target is shown in the main timeline only.
|
||||
*/
|
||||
targetThreadRootId: string | null;
|
||||
};
|
||||
|
||||
111
desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs
Normal file
111
desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs
Normal file
@@ -0,0 +1,111 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { getRouteTargetPanelAction } from "./useChannelRouteTarget.ts";
|
||||
|
||||
function makeMessage(overrides = {}) {
|
||||
return {
|
||||
id: "target",
|
||||
author: "alice",
|
||||
body: "hello",
|
||||
createdAt: 1,
|
||||
depth: 0,
|
||||
parentId: null,
|
||||
rootId: null,
|
||||
tags: [],
|
||||
time: "now",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function byId(...messages) {
|
||||
return new Map(messages.map((message) => [message.id, message]));
|
||||
}
|
||||
|
||||
test("top-level target without threadRootId stays in the main timeline", () => {
|
||||
const root = makeMessage();
|
||||
assert.deepEqual(getRouteTargetPanelAction(root, null, byId(root)), {
|
||||
kind: "main-timeline-only",
|
||||
});
|
||||
});
|
||||
|
||||
test("top-level target with an explicit threadRootId opens its thread panel", () => {
|
||||
const root = makeMessage();
|
||||
assert.deepEqual(getRouteTargetPanelAction(root, root.id, byId(root)), {
|
||||
kind: "open-thread",
|
||||
expandedReplyIds: new Set(),
|
||||
replyTargetId: root.id,
|
||||
scrollTargetId: null,
|
||||
threadHeadId: root.id,
|
||||
});
|
||||
});
|
||||
|
||||
test("reply target opens the thread scrolled to the reply", () => {
|
||||
const root = makeMessage({ id: "root" });
|
||||
const reply = makeMessage({
|
||||
id: "reply",
|
||||
parentId: "root",
|
||||
rootId: "root",
|
||||
depth: 1,
|
||||
});
|
||||
assert.deepEqual(getRouteTargetPanelAction(reply, null, byId(root, reply)), {
|
||||
kind: "open-thread",
|
||||
expandedReplyIds: new Set(),
|
||||
replyTargetId: "root",
|
||||
scrollTargetId: "reply",
|
||||
threadHeadId: "root",
|
||||
});
|
||||
});
|
||||
|
||||
test("nested reply target expands its intermediate ancestors", () => {
|
||||
const root = makeMessage({ id: "root" });
|
||||
const mid = makeMessage({
|
||||
id: "mid",
|
||||
parentId: "root",
|
||||
rootId: "root",
|
||||
depth: 1,
|
||||
});
|
||||
const leaf = makeMessage({
|
||||
id: "leaf",
|
||||
parentId: "mid",
|
||||
rootId: "root",
|
||||
depth: 2,
|
||||
});
|
||||
assert.deepEqual(
|
||||
getRouteTargetPanelAction(leaf, null, byId(root, mid, leaf)),
|
||||
{
|
||||
kind: "open-thread",
|
||||
expandedReplyIds: new Set(["mid"]),
|
||||
replyTargetId: "root",
|
||||
scrollTargetId: "leaf",
|
||||
threadHeadId: "root",
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("broadcast reply target is not a panel action", () => {
|
||||
const root = makeMessage({ id: "root" });
|
||||
const broadcast = makeMessage({
|
||||
id: "broadcast",
|
||||
parentId: "root",
|
||||
rootId: "root",
|
||||
depth: 1,
|
||||
tags: [["broadcast", "1"]],
|
||||
});
|
||||
assert.deepEqual(
|
||||
getRouteTargetPanelAction(broadcast, null, byId(root, broadcast)),
|
||||
{ kind: "none" },
|
||||
);
|
||||
});
|
||||
|
||||
test("reply whose thread head is not loaded yet defers", () => {
|
||||
const reply = makeMessage({
|
||||
id: "reply",
|
||||
parentId: "missing-root",
|
||||
rootId: "missing-root",
|
||||
depth: 1,
|
||||
});
|
||||
assert.deepEqual(getRouteTargetPanelAction(reply, null, byId(reply)), {
|
||||
kind: "none",
|
||||
});
|
||||
});
|
||||
@@ -37,6 +37,71 @@ function getThreadRouteTarget(
|
||||
return { expandedReplyIds, threadHeadId };
|
||||
}
|
||||
|
||||
export type RouteTargetPanelAction =
|
||||
| { kind: "none" }
|
||||
| { kind: "main-timeline-only" }
|
||||
| {
|
||||
kind: "open-thread";
|
||||
expandedReplyIds: Set<string>;
|
||||
replyTargetId: string;
|
||||
scrollTargetId: string | null;
|
||||
threadHeadId: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Decides what a message route target does to the thread panel.
|
||||
*
|
||||
* - Top-level target without an explicit `threadRootId` (inbox message rows,
|
||||
* desktop notifications, search hits, `buzz://` root links): the
|
||||
* main-timeline scroll + highlight is the entire navigation. Opening the
|
||||
* reply panel here would show an empty "no replies" pane instead of the
|
||||
* message in its own context — the exact defect this guards against.
|
||||
* - Top-level target with an explicit `threadRootId` (inbox "Open full
|
||||
* thread", thread-draft auto-send, channel-activity rows): the surface
|
||||
* asked for the thread, so the panel opens at that root.
|
||||
* - Reply target: the panel opens at the thread head, scrolled to the reply.
|
||||
* - `none`: not actionable yet (broadcast reply, or the thread head is not
|
||||
* loaded) — the caller retries when more messages arrive.
|
||||
*
|
||||
* Exported as a pure function so the routing contract is unit-testable
|
||||
* without mounting the hook.
|
||||
*/
|
||||
export function getRouteTargetPanelAction(
|
||||
targetMessage: TimelineMessage,
|
||||
targetThreadRootId: string | null,
|
||||
messageById: ReadonlyMap<string, TimelineMessage>,
|
||||
): RouteTargetPanelAction {
|
||||
if (!targetMessage.parentId) {
|
||||
if (!targetThreadRootId) {
|
||||
return { kind: "main-timeline-only" };
|
||||
}
|
||||
return {
|
||||
kind: "open-thread",
|
||||
expandedReplyIds: new Set(),
|
||||
replyTargetId: targetMessage.id,
|
||||
scrollTargetId: null,
|
||||
threadHeadId: targetMessage.id,
|
||||
};
|
||||
}
|
||||
|
||||
if (isBroadcastReply(targetMessage.tags ?? [])) {
|
||||
return { kind: "none" };
|
||||
}
|
||||
|
||||
const routeTarget = getThreadRouteTarget(targetMessage, messageById);
|
||||
if (!routeTarget) {
|
||||
return { kind: "none" };
|
||||
}
|
||||
|
||||
return {
|
||||
kind: "open-thread",
|
||||
expandedReplyIds: routeTarget.expandedReplyIds,
|
||||
replyTargetId: routeTarget.threadHeadId,
|
||||
scrollTargetId: targetMessage.id,
|
||||
threadHeadId: routeTarget.threadHeadId,
|
||||
};
|
||||
}
|
||||
|
||||
function getRouteMainTimelineTargetId(
|
||||
targetMessageId: string | null,
|
||||
targetMessage: TimelineMessage | null,
|
||||
@@ -63,6 +128,7 @@ export function useChannelRouteTarget({
|
||||
setThreadReplyTargetId,
|
||||
setThreadScrollTargetId,
|
||||
targetMessageId,
|
||||
targetThreadRootId,
|
||||
timelineMessages,
|
||||
}: {
|
||||
activeChannel: Channel | null;
|
||||
@@ -75,6 +141,7 @@ export function useChannelRouteTarget({
|
||||
setThreadReplyTargetId: React.Dispatch<React.SetStateAction<string | null>>;
|
||||
setThreadScrollTargetId: React.Dispatch<React.SetStateAction<string | null>>;
|
||||
targetMessageId: string | null;
|
||||
targetThreadRootId: string | null;
|
||||
timelineMessages: TimelineMessage[];
|
||||
}) {
|
||||
const timelineMessageById = React.useMemo(
|
||||
@@ -114,31 +181,21 @@ export function useChannelRouteTarget({
|
||||
return;
|
||||
}
|
||||
|
||||
if (!targetMessage.parentId) {
|
||||
closeAgentSession();
|
||||
// Root message links should open the reply panel for that root. The
|
||||
// timeline scroll/highlight target alone is not enough: root links have
|
||||
// no parent/thread metadata, so the reply-only branch below cannot infer
|
||||
// a thread head.
|
||||
setProfilePanelPubkey(null, { replace: true });
|
||||
setEditTargetId(null);
|
||||
setOpenThreadHeadId(targetMessage.id, { replace: true });
|
||||
setThreadReplyTargetId(targetMessage.id);
|
||||
setThreadScrollTargetId(null);
|
||||
setExpandedThreadReplyIds(new Set());
|
||||
handledThreadRouteTargetRef.current = targetKey;
|
||||
return;
|
||||
}
|
||||
|
||||
if (isBroadcastReply(targetMessage.tags ?? [])) {
|
||||
return;
|
||||
}
|
||||
|
||||
const routeTarget = getThreadRouteTarget(
|
||||
const action = getRouteTargetPanelAction(
|
||||
targetMessage,
|
||||
targetThreadRootId,
|
||||
timelineMessageById,
|
||||
);
|
||||
if (!routeTarget) {
|
||||
if (action.kind === "none") {
|
||||
return;
|
||||
}
|
||||
|
||||
if (action.kind === "main-timeline-only") {
|
||||
// Top-level target with no requested thread: the main-timeline
|
||||
// scroll/highlight (mainTimelineTargetMessageId) is the whole
|
||||
// navigation. Mark handled so a later timeline update cannot
|
||||
// re-process this target.
|
||||
handledThreadRouteTargetRef.current = targetKey;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -147,10 +204,10 @@ export function useChannelRouteTarget({
|
||||
// back should leave the deep link, not strip the panel from it.
|
||||
setProfilePanelPubkey(null, { replace: true });
|
||||
setEditTargetId(null);
|
||||
setOpenThreadHeadId(routeTarget.threadHeadId, { replace: true });
|
||||
setThreadReplyTargetId(routeTarget.threadHeadId);
|
||||
setThreadScrollTargetId(targetMessageId);
|
||||
setExpandedThreadReplyIds(routeTarget.expandedReplyIds);
|
||||
setOpenThreadHeadId(action.threadHeadId, { replace: true });
|
||||
setThreadReplyTargetId(action.replyTargetId);
|
||||
setThreadScrollTargetId(action.scrollTargetId);
|
||||
setExpandedThreadReplyIds(action.expandedReplyIds);
|
||||
handledThreadRouteTargetRef.current = targetKey;
|
||||
}, [
|
||||
activeChannel,
|
||||
@@ -163,6 +220,7 @@ export function useChannelRouteTarget({
|
||||
setThreadReplyTargetId,
|
||||
setThreadScrollTargetId,
|
||||
targetMessageId,
|
||||
targetThreadRootId,
|
||||
timelineMessageById,
|
||||
]);
|
||||
|
||||
|
||||
@@ -445,7 +445,7 @@ test("mixed Buzz permalinks render as chips in the composer", async ({
|
||||
await expect(composerInput).not.toContainText("buzz://");
|
||||
});
|
||||
|
||||
test("message links to visible root messages open the thread panel", async ({
|
||||
test("message links to visible root messages highlight them in the main timeline", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto("/");
|
||||
@@ -639,12 +639,17 @@ test("message links to visible root messages open the thread panel", async ({
|
||||
await rootThreadLink.click({ button: "right" });
|
||||
await linkMenu.getByRole("button", { name: "Open link" }).click();
|
||||
|
||||
// Root-message links resolve in the main timeline: scroll + highlight the
|
||||
// root, never force-open its (possibly empty) reply panel (block/buzz —
|
||||
// "inbox deep links open an empty thread for top-level messages").
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
await expect(threadPanel).toBeVisible();
|
||||
await expect(page).toHaveURL(/thread=mock-general-welcome/);
|
||||
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
||||
"Welcome to general",
|
||||
);
|
||||
await expect(threadPanel).not.toBeVisible();
|
||||
await expect(page).not.toHaveURL(/thread=/);
|
||||
const welcomeRow = page
|
||||
.getByTestId("message-timeline")
|
||||
.locator('[data-message-id="mock-general-welcome"]');
|
||||
await expect(welcomeRow).toBeVisible();
|
||||
await expect(welcomeRow).toHaveClass(/route-target-highlight-fade/);
|
||||
});
|
||||
|
||||
test("direct-message tooltip metadata stays on one physical line", async ({
|
||||
@@ -742,7 +747,7 @@ test("message links explain when preview metadata is unavailable", async ({
|
||||
await expect(unavailableTooltip).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("message links reopen a closed thread when the same messageId is already in the URL", async ({
|
||||
test("root message links re-target the main timeline when the same messageId was already in the URL", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto(
|
||||
@@ -750,14 +755,20 @@ test("message links reopen a closed thread when the same messageId is already in
|
||||
);
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
|
||||
// A top-level deep link never opens the reply panel — the message is shown
|
||||
// highlighted in its own context instead.
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
await expect(threadPanel).toBeVisible();
|
||||
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
||||
"Welcome to general",
|
||||
);
|
||||
|
||||
await threadPanel.getByRole("button", { name: "Close panel" }).click();
|
||||
await expect(threadPanel).not.toBeVisible();
|
||||
const welcomeRow = page
|
||||
.getByTestId("message-timeline")
|
||||
.locator('[data-message-id="mock-general-welcome"]');
|
||||
await expect(welcomeRow).toBeVisible();
|
||||
await expect(welcomeRow).toHaveClass(/route-target-highlight-fade/);
|
||||
|
||||
// Once the target is reached the messageId param clears; re-activating a
|
||||
// link to the same root must route again instead of being swallowed
|
||||
// (regression guard from the original reopen-a-closed-thread test).
|
||||
await expect(page).not.toHaveURL(/messageId=/);
|
||||
|
||||
const link =
|
||||
"buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=mock-general-welcome";
|
||||
@@ -777,24 +788,30 @@ test("message links reopen a closed thread when the same messageId is already in
|
||||
await expect(rootThreadLink).toHaveText("general");
|
||||
await rootThreadLink.click();
|
||||
|
||||
await expect(threadPanel).toBeVisible();
|
||||
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
||||
"Welcome to general",
|
||||
);
|
||||
await expect(threadPanel).not.toBeVisible();
|
||||
await expect(welcomeRow).toHaveClass(/route-target-highlight-fade/);
|
||||
});
|
||||
|
||||
test("message deep links survive reload", async ({ page }) => {
|
||||
await page.goto(
|
||||
`/#/channels/${ENGINEERING_CHANNEL_ID}?messageId=mock-engineering-shipped`,
|
||||
);
|
||||
const deepLinkUrl = `/#/channels/${ENGINEERING_CHANNEL_ID}?messageId=mock-engineering-shipped`;
|
||||
await page.goto(deepLinkUrl);
|
||||
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("engineering");
|
||||
await expect(page.getByTestId("message-timeline")).toContainText(
|
||||
"Engineering shipped the desktop build.",
|
||||
);
|
||||
|
||||
// Once the target is centered the messageId param is consumed (cleared via
|
||||
// onTargetReached so re-activating the same link is never swallowed), and a
|
||||
// top-level target no longer pins a `thread` param either — so a bare
|
||||
// reload lands on the plain channel.
|
||||
await expect(page).not.toHaveURL(/messageId=/);
|
||||
await page.reload();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("engineering");
|
||||
|
||||
// The deep-link URL itself stays valid: loading it again cold re-resolves
|
||||
// and re-splices the target message.
|
||||
await page.goto(deepLinkUrl);
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("engineering");
|
||||
await expect(page.getByTestId("message-timeline")).toContainText(
|
||||
"Engineering shipped the desktop build.",
|
||||
|
||||
Reference in New Issue
Block a user