From efef6b7090b18bb069399b577f6d3645fa9dc6aa Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 11:03:00 -0700 Subject: [PATCH 01/13] fix(messages): route edits to the owning composer Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 6 +- .../features/channels/ui/ChannelPane.types.ts | 11 +-- .../messages/lib/draftMentionRefs.test.mjs | 20 +++++ .../features/messages/lib/draftMentionRefs.ts | 2 + .../features/messages/ui/MessageActionBar.tsx | 2 +- .../messages/ui/MessageComposer.types.ts | 1 + desktop/tests/e2e/messaging.spec.ts | 76 +++++++++++++++++++ 7 files changed, 103 insertions(+), 15 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index bccc163ed40..931818e4e34 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -220,11 +220,7 @@ export const ChannelPane = React.memo(function ChannelPane({ isActiveWelcomeChannel, currentPubkey ?? null, ); - const isEditInThread = - editTarget != null && - threadHeadMessage != null && - (editTarget.id === threadHeadMessage.id || - threadMessages.some((entry) => entry.message.id === editTarget.id)); + const isEditInThread = editTarget?.isThreadReply ?? false; const mainEditTarget = editTarget && !isEditInThread ? editTarget : null; const threadEditTarget = editTarget && isEditInThread ? editTarget : null; const findLastOwnEditable = React.useCallback( diff --git a/desktop/src/features/channels/ui/ChannelPane.types.ts b/desktop/src/features/channels/ui/ChannelPane.types.ts index 760ef58073b..7ac3930b84e 100644 --- a/desktop/src/features/channels/ui/ChannelPane.types.ts +++ b/desktop/src/features/channels/ui/ChannelPane.types.ts @@ -1,13 +1,12 @@ import type * as React from "react"; import type { BotActivityAgent } from "@/features/channels/ui/BotActivityBar"; import type { ChannelAgentSessionAgent } from "@/features/channels/ui/useChannelAgentSessions"; -import type { ImetaMedia } from "@/features/messages/lib/imetaMediaMarkdown"; +import type { MessageComposerEditTarget } from "@/features/messages/ui/MessageComposer.types"; import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; import type { ChannelWindowThreadSummary } from "@/features/messages/lib/channelWindowStore"; import type { TimelineMessage } from "@/features/messages/types"; import type { TypingIndicatorEntry } from "@/features/messages/useChannelTyping"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; -import type { DraftMentionRef } from "@/features/messages/lib/useDrafts"; import type { ProfilePanelTab, ProfilePanelView, @@ -37,13 +36,7 @@ export type ChannelPaneProps = { botTypingEntries: TypingIndicatorEntry[]; channelManagementOpen?: boolean; currentPubkey?: string; - editTarget?: { - author: string; - body: string; - id: string; - imetaMedia?: ImetaMedia[]; - mentionRefs?: DraftMentionRef[]; - } | null; + editTarget?: MessageComposerEditTarget | null; fetchOlder?: () => Promise; header?: React.ReactNode; hasOlderMessages?: boolean; diff --git a/desktop/src/features/messages/lib/draftMentionRefs.test.mjs b/desktop/src/features/messages/lib/draftMentionRefs.test.mjs index 87ec604464b..e0a7dedc3e6 100644 --- a/desktop/src/features/messages/lib/draftMentionRefs.test.mjs +++ b/desktop/src/features/messages/lib/draftMentionRefs.test.mjs @@ -69,6 +69,26 @@ test("edit target preserves tagged identities while profiles are unavailable", ( assert.deepEqual(target.unresolvedMentionPubkeys, [ALICE, BOB]); }); +test("edit target records semantic thread ownership", () => { + const root = buildMessageComposerEditTarget( + message("Root", [["h", "channel-id"]]), + undefined, + () => false, + ); + const reply = buildMessageComposerEditTarget( + message("Reply", [ + ["h", "channel-id"], + ["e", "root-id", "", "root"], + ["e", "root-id", "", "reply"], + ]), + undefined, + () => false, + ); + + assert.equal(root.isThreadReply, false); + assert.equal(reply.isThreadReply, true); +}); + test("edit target separates resolved refs from identities missing profiles", () => { const target = buildMessageComposerEditTarget( message("Please review this, @Alice and @Bob.", [ diff --git a/desktop/src/features/messages/lib/draftMentionRefs.ts b/desktop/src/features/messages/lib/draftMentionRefs.ts index 65c7a68fec9..7aebf86b2f8 100644 --- a/desktop/src/features/messages/lib/draftMentionRefs.ts +++ b/desktop/src/features/messages/lib/draftMentionRefs.ts @@ -1,5 +1,6 @@ import { hasMention } from "@/features/messages/lib/hasMention"; import { imetaMediaFromTags } from "@/features/messages/lib/imetaMediaMarkdown"; +import { isThreadReply } from "@/features/messages/lib/threading"; import type { DraftMentionRef } from "@/features/messages/lib/useDrafts"; import type { TimelineMessage } from "@/features/messages/types"; import type { MessageComposerEditTarget } from "@/features/messages/ui/MessageComposer.types"; @@ -95,6 +96,7 @@ export function buildMessageComposerEditTarget( author: message.author, body: message.body, id: message.id, + isThreadReply: isThreadReply(message.tags ?? []), imetaMedia: imetaMediaFromTags(message.tags), ...mentionState, }; diff --git a/desktop/src/features/messages/ui/MessageActionBar.tsx b/desktop/src/features/messages/ui/MessageActionBar.tsx index 11163aa8c7a..4fcf0f067ab 100644 --- a/desktop/src/features/messages/ui/MessageActionBar.tsx +++ b/desktop/src/features/messages/ui/MessageActionBar.tsx @@ -143,7 +143,7 @@ function MoreActionsMenu({ {onEdit ? ( { + onSelect={() => { editJustSelectedRef.current = true; onEdit(message); }} diff --git a/desktop/src/features/messages/ui/MessageComposer.types.ts b/desktop/src/features/messages/ui/MessageComposer.types.ts index 517b9afb00e..f95c1f6e2e0 100644 --- a/desktop/src/features/messages/ui/MessageComposer.types.ts +++ b/desktop/src/features/messages/ui/MessageComposer.types.ts @@ -10,6 +10,7 @@ export type MessageComposerEditTarget = { author: string; body: string; id: string; + isThreadReply?: boolean; /** * NIP-92 imeta attachments on the original event, in tag order. Loaded * into the composer's pending-imeta state on edit-open so the user sees diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 9a93086cf87..3abbddab405 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -2970,6 +2970,82 @@ test("thread composer keeps focus after sending a thread reply", async ({ await expect(threadInput).toBeFocused(); }); +test("editing the thread root uses and focuses the main composer", async ({ + page, +}) => { + const root = `Root edit routing ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + + const timeline = page.getByTestId("message-timeline"); + const timelineRoot = timeline.getByTestId("message-row").last(); + await expect(timelineRoot).toContainText(root); + await timelineRoot.hover(); + await timelineRoot.getByRole("button", { name: "Reply" }).click(); + + const threadPanel = page.getByTestId("message-thread-panel"); + await expect(threadPanel).toBeVisible(); + const threadRoot = threadPanel.getByTestId("message-row").first(); + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(page.getByTestId("edit-target")).toHaveCount(1); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await expect(mainInput).toHaveText(root); + await expect(mainInput).toBeFocused(); +}); + +test("editing a thread reply uses and focuses the thread composer", async ({ + page, +}) => { + const root = `Reply edit routing root ${Date.now()}`; + const reply = `Reply edit routing ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.scrollIntoViewIfNeeded(); + await timelineRoot.hover(); + await timelineRoot.getByRole("button", { name: "Reply" }).click({ + force: true, + }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + await expect(threadPanel).toContainText(reply); + + const threadReply = threadPanel.getByTestId("message-row").last(); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(reply); + await expect(threadInput).toBeFocused(); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 3562cbe520b5e22ba318f2d824ed7ca0bb850a7a Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 11:33:09 -0700 Subject: [PATCH 02/13] fix(messages): resolve persisted reply edits Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../features/channels/ui/ChannelScreen.tsx | 6 ++- desktop/tests/e2e/messaging.spec.ts | 46 +++++++++++-------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 68df9bc05c6..c20a18968e7 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -462,8 +462,10 @@ export function ChannelScreen({ }); const editTargetMessage = React.useMemo( () => - timelineMessages.find((message) => message.id === editTargetId) ?? null, - [editTargetId, timelineMessages], + timelineMessages.find((message) => message.id === editTargetId) ?? + threadPanelData.messages.find((message) => message.id === editTargetId) ?? + null, + [editTargetId, threadPanelData.messages, timelineMessages], ); const [emptyDeleteId, setEmptyDeleteId] = React.useState(null); const { diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 3abbddab405..4bd88b00c09 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3004,39 +3004,47 @@ test("editing the thread root uses and focuses the main composer", async ({ await expect(mainInput).toBeFocused(); }); -test("editing a thread reply uses and focuses the thread composer", async ({ +test("editing a pre-seeded thread reply uses and focuses the thread composer", async ({ page, }) => { const root = `Reply edit routing root ${Date.now()}`; const reply = `Reply edit routing ${Date.now()}`; await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { replyId, rootId } = await page.evaluate( + ({ replyContent, rootContent }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const rootEvent = emit({ + channelName: "general", + content: rootContent, + }); + const replyEvent = emit({ + channelName: "general", + content: replyContent, + parentEventId: rootEvent.id, + }); + return { replyId: replyEvent.id, rootId: rootEvent.id }; + }, + { replyContent: reply, rootContent: root }, + ); + await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); - - const mainInput = page - .getByTestId("channel-composer-overlay") - .getByTestId("message-input"); - await mainInput.fill(root); - await mainInput.press("Enter"); - const timelineRoot = page .getByTestId("message-timeline") - .getByTestId("message-row") - .last(); - await timelineRoot.scrollIntoViewIfNeeded(); + .locator(`[data-message-id="${rootId}"]`); + await expect(timelineRoot).toContainText(root); await timelineRoot.hover(); - await timelineRoot.getByRole("button", { name: "Reply" }).click({ - force: true, - }); + await timelineRoot.getByRole("button", { name: "Reply" }).click(); const threadPanel = page.getByTestId("message-thread-panel"); const threadInput = threadPanel.getByTestId("message-input"); - await threadInput.fill(reply); - await threadInput.press("Enter"); - await expect(threadPanel).toContainText(reply); - - const threadReply = threadPanel.getByTestId("message-row").last(); + const threadReply = threadPanel.locator(`[data-message-id="${replyId}"]`); + await expect(threadReply).toContainText(reply); await threadReply.hover(); await threadReply.getByRole("button", { name: "More actions" }).click(); await page.getByRole("menuitem", { name: "Edit message" }).click(); From c531e9ea31df6a931fa613327fca8d12c6d53630 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 11:53:42 -0700 Subject: [PATCH 03/13] fix(messages): preserve edits across thread layouts Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 28 ++- .../channels/useChannelPaneHandlers.ts | 5 + .../src/features/home/ui/InboxDetailPane.tsx | 1 + .../messages/lib/draftMentionRefs.test.mjs | 11 + .../messages/ui/MessageComposer.types.ts | 2 +- desktop/tests/e2e/messaging.spec.ts | 196 ++++++++++++++++++ 6 files changed, 239 insertions(+), 4 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 931818e4e34..8b59a1302e8 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -23,6 +23,7 @@ import { hasOtherDmParticipant, } from "@/features/channels/lib/dmHuddleMembers"; import { buildVideoReviewPresentationByMessageId } from "@/features/messages/lib/videoReviewContext"; +import { isThreadReply } from "@/features/messages/lib/threading"; import { useComposerHeightPadding } from "@/features/messages/ui/useComposerHeightPadding"; import { UserProfilePanel } from "@/features/profile/ui/UserProfilePanel"; import { AgentSessionThreadPanel } from "@/features/channels/ui/AgentSessionThreadPanel"; @@ -220,7 +221,7 @@ export const ChannelPane = React.memo(function ChannelPane({ isActiveWelcomeChannel, currentPubkey ?? null, ); - const isEditInThread = editTarget?.isThreadReply ?? false; + const isEditInThread = editTarget?.isThreadReply === true; const mainEditTarget = editTarget && !isEditInThread ? editTarget : null; const threadEditTarget = editTarget && isEditInThread ? editTarget : null; const findLastOwnEditable = React.useCallback( @@ -457,6 +458,27 @@ export const ChannelPane = React.memo(function ChannelPane({ useFocusThreadDrawer, onCloseThread, ); + const pendingMainEditRef = React.useRef(null); + const handleRoutedEdit = React.useCallback( + (message: TimelineMessage) => { + if ( + !isThreadReply(message.tags ?? []) && + (isSinglePanelView || useFocusThreadDrawer) + ) { + pendingMainEditRef.current = message; + onCloseThread(); + return; + } + onEdit?.(message); + }, + [isSinglePanelView, onCloseThread, onEdit, useFocusThreadDrawer], + ); + React.useEffect(() => { + const pendingMainEdit = pendingMainEditRef.current; + if (!pendingMainEdit || isSinglePanelView || channelIsCovered) return; + pendingMainEditRef.current = null; + onEdit?.(pendingMainEdit); + }, [channelIsCovered, isSinglePanelView, onEdit]); const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } = useThreadViewModeSwitch({ activeThreadHeadId: threadHeadMessage?.id ?? null, @@ -612,7 +634,7 @@ export const ChannelPane = React.memo(function ChannelPane({ firstUnreadMessageId={firstUnreadMessageId} unreadCount={unreadCount} onDelete={onDelete} - onEdit={onEdit} + onEdit={handleRoutedEdit} onMarkUnread={onMarkUnread} onMarkRead={onMarkRead} onReply={timelineReplyHandler} @@ -804,7 +826,7 @@ export const ChannelPane = React.memo(function ChannelPane({ onCancelReply={onCancelThreadReply} onClose={onCloseThread} onDelete={onDelete} - onEdit={onEdit} + onEdit={handleRoutedEdit} onEditLastOwnMessage={handleEditLastOwnThreadMessage} onEditSave={onEditSave} onFollowThread={onFollowThread} diff --git a/desktop/src/features/channels/useChannelPaneHandlers.ts b/desktop/src/features/channels/useChannelPaneHandlers.ts index f9c57f6656a..154abebe49e 100644 --- a/desktop/src/features/channels/useChannelPaneHandlers.ts +++ b/desktop/src/features/channels/useChannelPaneHandlers.ts @@ -1,4 +1,5 @@ import * as React from "react"; +import { toast } from "sonner"; import type { useDeleteMessageMutation, @@ -118,6 +119,10 @@ export function useChannelPaneHandlers({ }, [setThreadReplyTargetId]); const handleCloseThread = React.useCallback(() => { + if (editTargetIdRef.current) { + toast.info("Finish or cancel your edit before closing the thread."); + return; + } deferPanelState(() => { onOptimisticOpenThreadHeadIdChange(null); setOpenThreadHeadId(null); diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 9192c649521..373ef80f452 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -461,6 +461,7 @@ function InboxMessageDetailPane({ author: editTarget.authorLabel, body: editTarget.content, id: editTarget.id, + isThreadReply: false, imetaMedia: imetaMediaFromTags(editTarget.tags), ...editMentionState, } diff --git a/desktop/src/features/messages/lib/draftMentionRefs.test.mjs b/desktop/src/features/messages/lib/draftMentionRefs.test.mjs index e0a7dedc3e6..ac1ed2d8c3a 100644 --- a/desktop/src/features/messages/lib/draftMentionRefs.test.mjs +++ b/desktop/src/features/messages/lib/draftMentionRefs.test.mjs @@ -85,8 +85,19 @@ test("edit target records semantic thread ownership", () => { () => false, ); + const broadcastReply = buildMessageComposerEditTarget( + message("Broadcast reply", [ + ["h", "channel-id"], + ["e", "root-id", "", "reply"], + ["broadcast", "1"], + ]), + undefined, + () => false, + ); + assert.equal(root.isThreadReply, false); assert.equal(reply.isThreadReply, true); + assert.equal(broadcastReply.isThreadReply, false); }); test("edit target separates resolved refs from identities missing profiles", () => { diff --git a/desktop/src/features/messages/ui/MessageComposer.types.ts b/desktop/src/features/messages/ui/MessageComposer.types.ts index f95c1f6e2e0..e1bc4098020 100644 --- a/desktop/src/features/messages/ui/MessageComposer.types.ts +++ b/desktop/src/features/messages/ui/MessageComposer.types.ts @@ -10,7 +10,7 @@ export type MessageComposerEditTarget = { author: string; body: string; id: string; - isThreadReply?: boolean; + isThreadReply: boolean; /** * NIP-92 imeta attachments on the original event, in tag order. Loaded * into the composer's pending-imeta state on edit-open so the user sees diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 4bd88b00c09..2252437e60a 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3054,6 +3054,202 @@ test("editing a pre-seeded thread reply uses and focuses the thread composer", a await expect(threadInput).toBeFocused(); }); +test("editing a broadcast reply from a thread returns to the main composer", async ({ + page, +}) => { + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { broadcastId, rootId } = await page.evaluate(() => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const rootEvent = emit({ + channelName: "general", + content: "Broadcast edit root", + }); + const broadcastEvent = emit({ + channelName: "general", + content: "Broadcast reply to edit", + parentEventId: rootEvent.id, + extraTags: [["broadcast", "1"]], + }); + return { broadcastId: broadcastEvent.id, rootId: rootEvent.id }; + }); + + await page.getByTestId("channel-general").click(); + const timelineRoot = page.locator(`[data-message-id="${rootId}"]`); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + const broadcastReply = threadPanel.locator( + `[data-message-id="${broadcastId}"]`, + ); + await expect(broadcastReply).toContainText("Broadcast reply to edit"); + await broadcastReply.hover(); + await broadcastReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await expect(mainInput).toHaveText("Broadcast reply to edit"); + await expect(mainInput).toBeFocused(); +}); + +test("editing a live thread reply uses and focuses the thread composer", async ({ + page, +}) => { + const root = `Live reply edit root ${Date.now()}`; + const reply = `Live reply edit ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + const threadReply = threadPanel.getByTestId("message-row").last(); + await expect(threadReply).toContainText(reply); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(reply); + await expect(threadInput).toBeFocused(); +}); + +test("editing a thread root in single-panel view returns to the main composer", async ({ + page, +}) => { + await page.setViewportSize({ width: 860, height: 720 }); + const root = `Narrow root edit ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const input = page.getByTestId("message-input"); + await input.fill(root); + await input.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadRoot = threadPanel.getByTestId("message-row").first(); + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(threadPanel).toBeHidden(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await expect(mainInput).toHaveText(root); + await expect(mainInput).toBeFocused(); +}); + +test("editing a thread root in focus mode dismisses the drawer before focusing the main composer", async ({ + page, +}) => { + await page.addInitScript(() => { + localStorage.setItem("buzz.channels.threadViewMode", "focus"); + }); + const root = `Focus root edit ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const drawer = page.getByTestId("focus-thread-drawer"); + const threadRoot = drawer.getByTestId("message-row").first(); + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(drawer).toBeHidden(); + await expect(mainInput).toHaveText(root); + await expect(mainInput).toBeFocused(); +}); + +test("closing a thread while editing a reply preserves the typed edit", async ({ + page, +}) => { + const root = `Close guard root ${Date.now()}`; + const reply = `Close guard reply ${Date.now()}`; + const edited = `${reply} with unsaved text`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + const threadReply = threadPanel.getByTestId("message-row").last(); + await expect(threadReply).toContainText(reply); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(edited); + + await threadPanel.getByTestId("auxiliary-panel-close").click(); + + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(edited); + await expect( + page.getByText("Finish or cancel your edit before closing the thread."), + ).toBeVisible(); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 212a3118eb9f47ed677a98d3ad0ba9a4979c0cc5 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 12:41:46 -0700 Subject: [PATCH 04/13] fix(messages): guard active thread edits Refuse cross-message edit transitions while preserving unsaved text, and let Escape cancel focus-drawer edits before drawer dismissal. Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 59 ++++++-- .../features/channels/ui/ChannelScreen.tsx | 12 +- .../channels/ui/FocusThreadDrawer.tsx | 8 ++ .../channels/ui/useChannelAgentSessions.ts | 4 + .../channels/ui/useChannelProfilePanel.ts | 4 + .../channels/useChannelPaneHandlers.ts | 17 ++- desktop/tests/e2e/messaging.spec.ts | 135 +++++++++++++++++- 7 files changed, 223 insertions(+), 16 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 8b59a1302e8..7a27ef3fc93 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -1,4 +1,5 @@ import * as React from "react"; +import { toast } from "sonner"; import { Hash, LogIn } from "lucide-react"; import { AnimatePresence } from "motion/react"; import { useAppNavigation } from "@/app/navigation/useAppNavigation"; @@ -251,16 +252,6 @@ export const ChannelPane = React.memo(function ChannelPane({ return true; }, [findLastOwnEditable, messages, onEdit]); - const handleEditLastOwnThreadMessage = React.useCallback((): boolean => { - if (!onEdit) return false; - const scope: TimelineMessage[] = []; - if (threadHeadMessage) scope.push(threadHeadMessage); - for (const entry of threadMessages) scope.push(entry.message); - const target = findLastOwnEditable(scope); - if (!target) return false; - onEdit(target); - return true; - }, [findLastOwnEditable, onEdit, threadHeadMessage, threadMessages]); const timeoutState = useTimeoutState(); // A moderation DM (1:1 with the relay identity) is read-only for the member; // only DMs pay for the NIP-11 `self` lookup. Fails open: no `relaySelf` → @@ -459,20 +450,64 @@ export const ChannelPane = React.memo(function ChannelPane({ onCloseThread, ); const pendingMainEditRef = React.useRef(null); + const editTargetRef = React.useRef(editTarget); + editTargetRef.current = editTarget; + const pendingMainEditContextRef = React.useRef({ + channelId: activeChannel?.id ?? null, + threadId: threadHeadMessage?.id ?? null, + }); + const pendingMainEditContext = { + channelId: activeChannel?.id ?? null, + threadId: threadHeadMessage?.id ?? null, + }; + const previousPendingContext = pendingMainEditContextRef.current; + if ( + previousPendingContext.channelId !== pendingMainEditContext.channelId || + (previousPendingContext.threadId !== null && + pendingMainEditContext.threadId !== null && + previousPendingContext.threadId !== pendingMainEditContext.threadId) + ) { + pendingMainEditRef.current = null; + } + pendingMainEditContextRef.current = pendingMainEditContext; const handleRoutedEdit = React.useCallback( - (message: TimelineMessage) => { + (message: TimelineMessage): boolean => { + const currentEditTarget = editTargetRef.current; + if (currentEditTarget && currentEditTarget.id !== message.id) { + pendingMainEditRef.current = null; + toast.info("Finish or cancel your edit first."); + return false; + } + if (currentEditTarget?.id === message.id) { + pendingMainEditRef.current = null; + onEdit?.(message); + return true; + } if ( !isThreadReply(message.tags ?? []) && (isSinglePanelView || useFocusThreadDrawer) ) { pendingMainEditRef.current = message; onCloseThread(); - return; + return true; } onEdit?.(message); + return Boolean(onEdit); }, [isSinglePanelView, onCloseThread, onEdit, useFocusThreadDrawer], ); + const handleEditLastOwnThreadMessage = React.useCallback((): boolean => { + const scope: TimelineMessage[] = []; + if (threadHeadMessage) scope.push(threadHeadMessage); + for (const entry of threadMessages) scope.push(entry.message); + const target = findLastOwnEditable(scope); + return target ? handleRoutedEdit(target) : false; + }, [ + findLastOwnEditable, + handleRoutedEdit, + threadHeadMessage, + threadMessages, + ]); React.useEffect(() => { const pendingMainEdit = pendingMainEditRef.current; if (!pendingMainEdit || isSinglePanelView || channelIsCovered) return; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index c20a18968e7..0b838a2e347 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -45,7 +45,10 @@ import { import { buildMessageComposerEditTarget } from "@/features/messages/lib/draftMentionRefs"; import { formatTimelineMessages } from "@/features/messages/lib/formatTimelineMessages"; import { DeleteMessageConfirmDialog } from "@/features/messages/ui/DeleteMessageConfirmDialog"; -import { getThreadReference } from "@/features/messages/lib/threading"; +import { + getThreadReference, + isThreadReply, +} from "@/features/messages/lib/threading"; import { hasPersistedHydratedChannel } from "@/features/messages/lib/channelHeadCache"; import { resolveTimelineLoadingLatch, @@ -477,6 +480,7 @@ export function ChannelScreen({ handleEditSave, handleExpandThreadReplies, handleOpenThread, + requireThreadEditResolution, handleSendMessage, handleSendToChannel, handleSendThreadReply, @@ -486,6 +490,8 @@ export function ChannelScreen({ deleteMessageMutation, editMessageMutation, editTargetId, + editTargetIsThreadReply: + editTargetMessage !== null && isThreadReply(editTargetMessage.tags ?? []), expandedThreadReplyIds, getFirstReplyIdForMessage, getReplyDescendantIdsForMessage, @@ -579,6 +585,7 @@ export function ChannelScreen({ openAgentSessionPubkey, openThreadHeadId: effectiveOpenThreadHeadId, profilePanelPubkey, + requireThreadEditResolution, setChannelManagementOpen, setExpandedThreadReplyIds, setOpenAgentSessionChannelId, @@ -592,6 +599,7 @@ export function ChannelScreen({ useChannelProfilePanel({ closeAgentSession: handleCloseAgentSession, openProfilePanel, + requireThreadEditResolution, setChannelManagementOpen, setExpandedThreadReplyIds, setOpenThreadHeadId, @@ -712,6 +720,7 @@ export function ChannelScreen({ enabled: !isSinglePanelView, }); const handleManageChannel = React.useCallback(() => { + if (!requireThreadEditResolution()) return; if (activeChannel?.channelType === "forum") { openGlobalChannelManagement(); return; @@ -731,6 +740,7 @@ export function ChannelScreen({ activeChannel?.channelType, channelManagementOpen, openGlobalChannelManagement, + requireThreadEditResolution, setChannelManagementOpen, setOpenThreadHeadId, handleCloseAgentSession, diff --git a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx index 1aaad0e6093..3a53983b552 100644 --- a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx +++ b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx @@ -149,6 +149,14 @@ export function FocusThreadDrawer({ React.useEffect(() => { function handleEscape(event: KeyboardEvent) { if (event.key !== "Escape") return; + const target = event.target; + if ( + target instanceof Node && + drawerRef.current?.contains(target) && + drawerRef.current.querySelector('[data-testid="edit-target"]') + ) { + return; + } event.preventDefault(); event.stopImmediatePropagation(); onClose(); diff --git a/desktop/src/features/channels/ui/useChannelAgentSessions.ts b/desktop/src/features/channels/ui/useChannelAgentSessions.ts index 20c561e7981..8dd22bb94a9 100644 --- a/desktop/src/features/channels/ui/useChannelAgentSessions.ts +++ b/desktop/src/features/channels/ui/useChannelAgentSessions.ts @@ -39,6 +39,7 @@ type UseChannelAgentSessionsOptions = { openAgentSessionPubkey: string | null; openThreadHeadId: string | null; profilePanelPubkey?: string | null; + requireThreadEditResolution: () => boolean; setChannelManagementOpen: (open: boolean) => void; setExpandedThreadReplyIds: (value: Set) => void; setOpenAgentSessionChannelId: PanelValueSetter; @@ -173,6 +174,7 @@ export function useChannelAgentSessions({ openAgentSessionPubkey, openThreadHeadId, profilePanelPubkey = null, + requireThreadEditResolution, setChannelManagementOpen, setExpandedThreadReplyIds, setOpenAgentSessionChannelId, @@ -209,6 +211,7 @@ export function useChannelAgentSessions({ const openAgentSession = React.useCallback( (pubkey: string, channelId?: string | null) => { + if (!requireThreadEditResolution()) return; if (!isAgentSessionOpen) { returnTarget.capture( resolveAgentSessionReturnTarget({ @@ -234,6 +237,7 @@ export function useChannelAgentSessions({ isAgentSessionOpen, openThreadHeadId, profilePanelPubkey, + requireThreadEditResolution, returnTarget, setChannelManagementOpen, setExpandedThreadReplyIds, diff --git a/desktop/src/features/channels/ui/useChannelProfilePanel.ts b/desktop/src/features/channels/ui/useChannelProfilePanel.ts index 61e9211480b..1a35666478a 100644 --- a/desktop/src/features/channels/ui/useChannelProfilePanel.ts +++ b/desktop/src/features/channels/ui/useChannelProfilePanel.ts @@ -7,6 +7,7 @@ import type { ProfilePanelOpenOptions } from "@/shared/context/ProfilePanelConte type UseChannelProfilePanelOptions = { closeAgentSession: () => void; openProfilePanel: (pubkey: string, options?: ProfilePanelOpenOptions) => void; + requireThreadEditResolution: () => boolean; setChannelManagementOpen: (open: boolean) => void; setExpandedThreadReplyIds: (value: Set) => void; setOpenThreadHeadId: (value: string | null) => void; @@ -18,6 +19,7 @@ type UseChannelProfilePanelOptions = { export function useChannelProfilePanel({ closeAgentSession, openProfilePanel, + requireThreadEditResolution, setChannelManagementOpen, setExpandedThreadReplyIds, setOpenThreadHeadId, @@ -30,6 +32,7 @@ export function useChannelProfilePanel({ const handleOpenProfilePanel = React.useCallback( (pubkey: string, options?: ProfilePanelOpenOptions) => { + if (!requireThreadEditResolution()) return; setOpenThreadHeadId(null); setExpandedThreadReplyIds(new Set()); setThreadScrollTargetId(null); @@ -41,6 +44,7 @@ export function useChannelProfilePanel({ [ closeAgentSession, openProfilePanel, + requireThreadEditResolution, setChannelManagementOpen, setExpandedThreadReplyIds, setOpenThreadHeadId, diff --git a/desktop/src/features/channels/useChannelPaneHandlers.ts b/desktop/src/features/channels/useChannelPaneHandlers.ts index 154abebe49e..c7c97951214 100644 --- a/desktop/src/features/channels/useChannelPaneHandlers.ts +++ b/desktop/src/features/channels/useChannelPaneHandlers.ts @@ -25,6 +25,7 @@ export function useChannelPaneHandlers({ deleteMessageMutation, editMessageMutation, editTargetId, + editTargetIsThreadReply, expandedThreadReplyIds, getFirstReplyIdForMessage, getReplyDescendantIdsForMessage, @@ -46,6 +47,7 @@ export function useChannelPaneHandlers({ deleteMessageMutation: ReturnType; editMessageMutation: ReturnType; editTargetId: string | null; + editTargetIsThreadReply: boolean; expandedThreadReplyIds: ReadonlySet; getFirstReplyIdForMessage: (messageId: string) => string | null; getReplyDescendantIdsForMessage: (messageId: string) => string[]; @@ -75,6 +77,8 @@ export function useChannelPaneHandlers({ const editTargetIdRef = React.useRef(editTargetId); editTargetIdRef.current = editTargetId; + const editTargetIsThreadReplyRef = React.useRef(editTargetIsThreadReply); + editTargetIsThreadReplyRef.current = editTargetIsThreadReply; const expandedThreadReplyIdsRef = React.useRef(expandedThreadReplyIds); expandedThreadReplyIdsRef.current = expandedThreadReplyIds; @@ -118,9 +122,14 @@ export function useChannelPaneHandlers({ setThreadReplyTargetId(openThreadHeadIdRef.current); }, [setThreadReplyTargetId]); + const requireThreadEditResolution = React.useCallback(() => { + if (!editTargetIsThreadReplyRef.current) return true; + toast.info("Finish or cancel your edit before leaving the thread."); + return false; + }, []); + const handleCloseThread = React.useCallback(() => { - if (editTargetIdRef.current) { - toast.info("Finish or cancel your edit before closing the thread."); + if (!requireThreadEditResolution()) { return; } deferPanelState(() => { @@ -133,6 +142,7 @@ export function useChannelPaneHandlers({ }, [ deferPanelState, onOptimisticOpenThreadHeadIdChange, + requireThreadEditResolution, setExpandedThreadReplyIds, setOpenThreadHeadId, setThreadReplyTargetId, @@ -203,6 +213,7 @@ export function useChannelPaneHandlers({ const handleOpenThread = React.useCallback( (message: { id: string }) => { + if (!requireThreadEditResolution()) return; if (openThreadHeadIdRef.current === message.id) { deferPanelState(() => { onOptimisticOpenThreadHeadIdChange(null); @@ -227,6 +238,7 @@ export function useChannelPaneHandlers({ [ deferPanelState, onOptimisticOpenThreadHeadIdChange, + requireThreadEditResolution, setEditTargetId, setExpandedThreadReplyIds, setOpenThreadHeadId, @@ -418,6 +430,7 @@ export function useChannelPaneHandlers({ handleEditSave, handleExpandThreadReplies, handleOpenThread, + requireThreadEditResolution, handleSendMessage, handleSendToChannel, handleSendThreadReply, diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 2252437e60a..1ca6986c160 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3206,6 +3206,139 @@ test("editing a thread root in focus mode dismisses the drawer before focusing t await expect(mainInput).toBeFocused(); }); +test("focus mode preserves an active reply edit, then Escape makes root editing available", async ({ + page, +}) => { + await page.addInitScript(() => { + localStorage.setItem("buzz.channels.threadViewMode", "focus"); + }); + const root = `Focus guarded root ${Date.now()}`; + const reply = `Focus guarded reply ${Date.now()}`; + const unsaved = `${reply} unsaved`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const drawer = page.getByTestId("focus-thread-drawer"); + const threadInput = drawer.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + const threadReply = drawer + .getByTestId("message-row") + .filter({ hasText: reply }) + .last(); + await expect(threadReply).toContainText(reply); + const threadReplyId = await threadReply.getAttribute("data-message-id"); + expect(threadReplyId).not.toBeNull(); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page + .locator('[role="menu"]:visible') + .getByTestId(`edit-message-${threadReplyId}`) + .click(); + await expect(page.locator('[role="menu"]:visible')).toHaveCount(0); + await threadInput.fill(unsaved); + + const threadRoot = drawer + .getByTestId("message-thread-head") + .getByTestId("message-row"); + const rootMessageId = await threadRoot.getAttribute("data-message-id"); + expect(rootMessageId).not.toBeNull(); + expect(rootMessageId).not.toBe(threadReplyId); + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "More actions" }).click(); + await page + .locator('[role="menu"]:visible') + .getByTestId(`edit-message-${rootMessageId}`) + .click(); + await expect(page.locator('[role="menu"]:visible')).toHaveCount(0); + await expect(drawer).toBeVisible(); + await expect(threadInput).toHaveText(unsaved); + await expect( + page.getByText("Finish or cancel your edit first."), + ).toBeVisible(); + + // A refused cross-message edit must not remain deferred and appear later. + await page.getByTestId("focus-thread-drawer-scrim").click({ + force: true, + position: { x: 10, y: 360 }, + }); + await expect(drawer).toBeVisible(); + await expect(threadInput).toHaveText(unsaved); + + // Selecting Edit for the active message keeps the existing toggle-to-cancel behavior. + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page + .locator('[role="menu"]:visible') + .getByTestId(`edit-message-${threadReplyId}`) + .click(); + await expect(drawer.getByTestId("edit-target")).toHaveCount(0); + await expect(threadInput).toHaveText(""); + + // Focus-mode Escape reaches the composer before the drawer close handler. + await threadInput.click(); + await page.keyboard.press("ArrowUp"); + await expect(drawer.getByTestId("edit-target")).toBeVisible(); + await threadInput.fill(unsaved); + await page.keyboard.press("Escape"); + await expect(drawer.getByTestId("edit-target")).toHaveCount(0); + await expect(drawer).toBeVisible(); + + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "More actions" }).click(); + await page + .locator('[role="menu"]:visible') + .getByTestId(`edit-message-${rootMessageId}`) + .click(); + await expect(drawer).toBeHidden(); + await expect(mainInput).toHaveText(root); +}); + +test("ArrowUp routes a narrow thread root without consuming into a hidden composer", async ({ + page, +}) => { + await page.setViewportSize({ width: 860, height: 720 }); + const root = `Narrow ArrowUp root ${Date.now()}`; + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const input = page.getByTestId("message-input"); + await input.fill(root); + await input.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + const threadInput = page + .getByTestId("message-thread-panel") + .getByTestId("message-input"); + await expect(threadInput).toBeFocused(); + await page.keyboard.press("ArrowUp"); + await expect(page.getByTestId("message-thread-panel")).toBeHidden(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await expect(mainInput).toHaveText(root); + await expect(mainInput).toBeFocused(); +}); + test("closing a thread while editing a reply preserves the typed edit", async ({ page, }) => { @@ -3246,7 +3379,7 @@ test("closing a thread while editing a reply preserves the typed edit", async ({ await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); await expect(threadInput).toHaveText(edited); await expect( - page.getByText("Finish or cancel your edit before closing the thread."), + page.getByText("Finish or cancel your edit before leaving the thread."), ).toBeVisible(); }); From 6467a367249976f700aab21b18977e5388f0e8d4 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 13:23:43 -0700 Subject: [PATCH 05/13] fix(messages): prevent hidden keyboard edit targets Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 13 +-- .../features/channels/ui/ChannelScreen.tsx | 20 ++-- .../channels/ui/FocusThreadDrawer.tsx | 8 +- .../channels/ui/useChannelRouteTarget.ts | 11 +++ .../channels/ui/useHuddleThreadIsolation.ts | 24 ----- desktop/tests/e2e/messaging.spec.ts | 93 +++++++++++++++++++ 6 files changed, 129 insertions(+), 40 deletions(-) delete mode 100644 desktop/src/features/channels/ui/useHuddleThreadIsolation.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 7a27ef3fc93..21927fa82fe 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -245,12 +245,6 @@ export const ChannelPane = React.memo(function ChannelPane({ }, [onEdit, currentPubkey], ); - const handleEditLastOwnMainMessage = React.useCallback((): boolean => { - const target = findLastOwnEditable(messages); - if (!target || !onEdit) return false; - onEdit(target); - return true; - }, [findLastOwnEditable, messages, onEdit]); const timeoutState = useTimeoutState(); // A moderation DM (1:1 with the relay identity) is read-only for the member; @@ -496,6 +490,12 @@ export const ChannelPane = React.memo(function ChannelPane({ }, [isSinglePanelView, onCloseThread, onEdit, useFocusThreadDrawer], ); + const handleEditLastOwnMainMessage = React.useCallback((): boolean => { + const target = findLastOwnEditable( + mainTimelineEntries.map((entry) => entry.message), + ); + return target ? handleRoutedEdit(target) : false; + }, [findLastOwnEditable, handleRoutedEdit, mainTimelineEntries]); const handleEditLastOwnThreadMessage = React.useCallback((): boolean => { const scope: TimelineMessage[] = []; if (threadHeadMessage) scope.push(threadHeadMessage); @@ -561,6 +561,7 @@ export const ChannelPane = React.memo(function ChannelPane({ useFocusThreadDrawer ? ( diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 0b838a2e347..1ba9474f46f 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -68,7 +68,6 @@ import { useIsHuddleTranscript, } from "@/features/channels/ui/useHuddleChannelMessages"; import { useHuddleReadMarker } from "@/features/channels/ui/useHuddleReadMarker"; -import { useHuddleThreadIsolation } from "@/features/channels/ui/useHuddleThreadIsolation"; import { AgentSessionProvider } from "@/shared/context/AgentSessionContext"; import { ProfilePanelProvider } from "@/shared/context/ProfilePanelContext"; import { useMainInsetRef } from "@/shared/layout/MainInsetContext"; @@ -171,12 +170,8 @@ export function ChannelScreen({ const activeChannelId = activeChannel?.id ?? null; const isHuddleTranscript = useIsHuddleTranscript(activeChannelId); const relaySelfPubkey = useRelaySelfQuery(activeChannel !== null).data; - const effectiveOpenThreadHeadId = useHuddleThreadIsolation({ - closeThread: setOpenThreadHeadId, - isHuddleTranscript, - openThreadHeadId, - optimisticOpenThreadHeadId, - }); + const effectiveOpenThreadHeadId = + optimisticOpenThreadHeadId ?? openThreadHeadId; const isNotifiedForEffectiveThread = effectiveOpenThreadHeadId != null ? isNotifiedForThread(effectiveOpenThreadHeadId) @@ -510,6 +505,16 @@ export function ChannelScreen({ threadReplyTargetId, toggleReactionMutation, }); + React.useEffect(() => { + if (!isHuddleTranscript || openThreadHeadId === null) return; + if (!requireThreadEditResolution()) return; + setOpenThreadHeadId(null); + }, [ + isHuddleTranscript, + openThreadHeadId, + requireThreadEditResolution, + setOpenThreadHeadId, + ]); const effectiveToggleReaction = React.useMemo( () => activeChannel && !activeChannel.archivedAt && activeChannel.isMember @@ -662,6 +667,7 @@ export function ChannelScreen({ activeChannel, activeChannelId, closeAgentSession: handleCloseAgentSession, + requireThreadEditResolution, setEditTargetId, setExpandedThreadReplyIds, setOpenThreadHeadId, diff --git a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx index 3a53983b552..410f2d8672d 100644 --- a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx +++ b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx @@ -11,6 +11,7 @@ import { cn } from "@/shared/lib/cn"; type FocusThreadDrawerProps = { channelName: string; children: React.ReactNode; + hasActiveEdit: boolean; onClose: () => void; }; @@ -139,6 +140,7 @@ const REDUCED_MOTION_TRANSITION = { duration: 0.12, ease: "linear" } as const; export function FocusThreadDrawer({ channelName, children, + hasActiveEdit, onClose, }: FocusThreadDrawerProps) { const prefersReducedMotion = useReducedMotion(); @@ -151,9 +153,9 @@ export function FocusThreadDrawer({ if (event.key !== "Escape") return; const target = event.target; if ( + hasActiveEdit && target instanceof Node && - drawerRef.current?.contains(target) && - drawerRef.current.querySelector('[data-testid="edit-target"]') + drawerRef.current?.contains(target) ) { return; } @@ -166,7 +168,7 @@ export function FocusThreadDrawer({ return () => { window.removeEventListener("keydown", handleEscape, { capture: true }); }; - }, [onClose]); + }, [hasActiveEdit, onClose]); React.useLayoutEffect(() => { previousFocusRef.current = diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.ts b/desktop/src/features/channels/ui/useChannelRouteTarget.ts index 0dc4b0e4d6d..71250461ca0 100644 --- a/desktop/src/features/channels/ui/useChannelRouteTarget.ts +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.ts @@ -56,6 +56,7 @@ export function useChannelRouteTarget({ activeChannel, activeChannelId, closeAgentSession, + requireThreadEditResolution, setEditTargetId, setExpandedThreadReplyIds, setOpenThreadHeadId, @@ -68,6 +69,7 @@ export function useChannelRouteTarget({ activeChannel: Channel | null; activeChannelId: string | null; closeAgentSession: () => void; + requireThreadEditResolution: () => boolean; setEditTargetId: React.Dispatch>; setExpandedThreadReplyIds: React.Dispatch>>; setOpenThreadHeadId: PanelValueSetter; @@ -115,6 +117,10 @@ export function useChannelRouteTarget({ } if (!targetMessage.parentId) { + if (!requireThreadEditResolution()) { + handledThreadRouteTargetRef.current = targetKey; + return; + } closeAgentSession(); // Root message links should open the reply panel for that root. The // timeline scroll/highlight target alone is not enough: root links have @@ -141,6 +147,10 @@ export function useChannelRouteTarget({ if (!routeTarget) { return; } + if (!requireThreadEditResolution()) { + handledThreadRouteTargetRef.current = targetKey; + return; + } closeAgentSession(); // Replace so the deep-link entry itself carries the opened thread — @@ -156,6 +166,7 @@ export function useChannelRouteTarget({ activeChannel, activeChannelId, closeAgentSession, + requireThreadEditResolution, setEditTargetId, setExpandedThreadReplyIds, setOpenThreadHeadId, diff --git a/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts b/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts deleted file mode 100644 index c32c809a887..00000000000 --- a/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts +++ /dev/null @@ -1,24 +0,0 @@ -import * as React from "react"; - -type HuddleThreadIsolationOptions = { - closeThread: (threadId: string | null) => void; - isHuddleTranscript: boolean; - openThreadHeadId: string | null; - optimisticOpenThreadHeadId: string | null | undefined; -}; - -export function useHuddleThreadIsolation({ - closeThread, - isHuddleTranscript, - openThreadHeadId, - optimisticOpenThreadHeadId, -}: HuddleThreadIsolationOptions): string | null { - React.useEffect(() => { - if (!isHuddleTranscript || openThreadHeadId === null) return; - closeThread(null); - }, [closeThread, isHuddleTranscript, openThreadHeadId]); - if (isHuddleTranscript) return null; - return optimisticOpenThreadHeadId === undefined - ? openThreadHeadId - : optimisticOpenThreadHeadId; -} diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 1ca6986c160..f7b745cb5b1 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3383,6 +3383,99 @@ test("closing a thread while editing a reply preserves the typed edit", async ({ ).toBeVisible(); }); +test("main ArrowUp ignores closed-thread replies and edits the visible timeline message", async ({ + page, +}) => { + const root = `Main ArrowUp root ${Date.now()}`; + const reply = `Main ArrowUp hidden reply ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + await expect(threadPanel).toContainText(reply); + await threadPanel.getByTestId("auxiliary-panel-close").click(); + await expect(threadPanel).toBeHidden(); + + await mainInput.click(); + await page.keyboard.press("ArrowUp"); + await expect(page.getByTestId("edit-target")).toBeVisible(); + await expect(mainInput).toHaveText(root); + + // No hidden reply edit may block reopening its thread. + await mainInput.press("Escape"); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + await expect(threadPanel).toBeVisible(); +}); + +test("main ArrowUp refuses to replace a dirty thread edit", async ({ + page, +}) => { + const root = `Main ArrowUp refusal root ${Date.now()}`; + const reply = `Main ArrowUp refusal reply ${Date.now()}`; + const unsaved = `${reply} with unsaved text`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(root); + await mainInput.press("Enter"); + const timelineRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .last(); + await timelineRoot.hover(); + await timelineRoot + .getByRole("button", { name: "Reply" }) + .click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + const threadReply = threadPanel.getByTestId("message-row").last(); + await expect(threadReply).toContainText(reply); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(unsaved); + + await mainInput.click(); + await page.keyboard.press("ArrowUp"); + await expect( + page.getByText("Finish or cancel your edit first."), + ).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(unsaved); + await expect(mainInput).toHaveText(""); + + // Refusal must not arm a deferred edit that appears after cancellation. + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await expect(mainInput).toHaveText(""); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From c02caeafef0624cedd8adc1eaf3c917be8d0e2fe Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 14:25:28 -0700 Subject: [PATCH 06/13] fix(messages): preserve guarded edit transitions Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 5 +- .../features/channels/ui/ChannelScreen.tsx | 24 ++-- .../channels/ui/useChannelRouteTarget.ts | 2 - .../ui/useHuddleThreadIsolation.test.mjs | 34 +++++ .../channels/ui/useHuddleThreadIsolation.ts | 47 +++++++ desktop/tests/e2e/messaging.spec.ts | 117 ++++++++++++++++++ 6 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 desktop/src/features/channels/ui/useHuddleThreadIsolation.test.mjs create mode 100644 desktop/src/features/channels/ui/useHuddleThreadIsolation.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 21927fa82fe..19a837d56fc 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -467,7 +467,10 @@ export const ChannelPane = React.memo(function ChannelPane({ const handleRoutedEdit = React.useCallback( (message: TimelineMessage): boolean => { const currentEditTarget = editTargetRef.current; - if (currentEditTarget && currentEditTarget.id !== message.id) { + if ( + currentEditTarget?.isThreadReply === true && + currentEditTarget.id !== message.id + ) { pendingMainEditRef.current = null; toast.info("Finish or cancel your edit first."); return false; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 1ba9474f46f..98a769ffc54 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -68,6 +68,7 @@ import { useIsHuddleTranscript, } from "@/features/channels/ui/useHuddleChannelMessages"; import { useHuddleReadMarker } from "@/features/channels/ui/useHuddleReadMarker"; +import { useHuddleThreadIsolation } from "@/features/channels/ui/useHuddleThreadIsolation"; import { AgentSessionProvider } from "@/shared/context/AgentSessionContext"; import { ProfilePanelProvider } from "@/shared/context/ProfilePanelContext"; import { useMainInsetRef } from "@/shared/layout/MainInsetContext"; @@ -170,8 +171,16 @@ export function ChannelScreen({ const activeChannelId = activeChannel?.id ?? null; const isHuddleTranscript = useIsHuddleTranscript(activeChannelId); const relaySelfPubkey = useRelaySelfQuery(activeChannel !== null).data; - const effectiveOpenThreadHeadId = - optimisticOpenThreadHeadId ?? openThreadHeadId; + const requireThreadEditResolutionRef = React.useRef<() => boolean>( + () => true, + ); + const effectiveOpenThreadHeadId = useHuddleThreadIsolation({ + closeThread: setOpenThreadHeadId, + isHuddleTranscript, + openThreadHeadId, + optimisticOpenThreadHeadId, + requireThreadEditResolutionRef, + }); const isNotifiedForEffectiveThread = effectiveOpenThreadHeadId != null ? isNotifiedForThread(effectiveOpenThreadHeadId) @@ -505,16 +514,7 @@ export function ChannelScreen({ threadReplyTargetId, toggleReactionMutation, }); - React.useEffect(() => { - if (!isHuddleTranscript || openThreadHeadId === null) return; - if (!requireThreadEditResolution()) return; - setOpenThreadHeadId(null); - }, [ - isHuddleTranscript, - openThreadHeadId, - requireThreadEditResolution, - setOpenThreadHeadId, - ]); + requireThreadEditResolutionRef.current = requireThreadEditResolution; const effectiveToggleReaction = React.useMemo( () => activeChannel && !activeChannel.archivedAt && activeChannel.isMember diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.ts b/desktop/src/features/channels/ui/useChannelRouteTarget.ts index 71250461ca0..fd4ae3e2f68 100644 --- a/desktop/src/features/channels/ui/useChannelRouteTarget.ts +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.ts @@ -118,7 +118,6 @@ export function useChannelRouteTarget({ if (!targetMessage.parentId) { if (!requireThreadEditResolution()) { - handledThreadRouteTargetRef.current = targetKey; return; } closeAgentSession(); @@ -148,7 +147,6 @@ export function useChannelRouteTarget({ return; } if (!requireThreadEditResolution()) { - handledThreadRouteTargetRef.current = targetKey; return; } diff --git a/desktop/src/features/channels/ui/useHuddleThreadIsolation.test.mjs b/desktop/src/features/channels/ui/useHuddleThreadIsolation.test.mjs new file mode 100644 index 00000000000..f75bc04b706 --- /dev/null +++ b/desktop/src/features/channels/ui/useHuddleThreadIsolation.test.mjs @@ -0,0 +1,34 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveHuddleOpenThreadHeadId } from "./useHuddleThreadIsolation.ts"; + +test("huddle transcripts synchronously hide URL thread state", () => { + assert.equal( + resolveHuddleOpenThreadHeadId({ + isHuddleTranscript: true, + openThreadHeadId: "url-thread", + optimisticOpenThreadHeadId: undefined, + }), + null, + ); +}); + +test("an optimistic null overrides the URL thread until navigation settles", () => { + assert.equal( + resolveHuddleOpenThreadHeadId({ + isHuddleTranscript: false, + openThreadHeadId: "url-thread", + optimisticOpenThreadHeadId: null, + }), + null, + ); + assert.equal( + resolveHuddleOpenThreadHeadId({ + isHuddleTranscript: false, + openThreadHeadId: "url-thread", + optimisticOpenThreadHeadId: undefined, + }), + "url-thread", + ); +}); diff --git a/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts b/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts new file mode 100644 index 00000000000..b794ce87be2 --- /dev/null +++ b/desktop/src/features/channels/ui/useHuddleThreadIsolation.ts @@ -0,0 +1,47 @@ +import * as React from "react"; + +type HuddleThreadIsolationOptions = { + closeThread: (threadId: string | null) => void; + isHuddleTranscript: boolean; + openThreadHeadId: string | null; + optimisticOpenThreadHeadId: string | null | undefined; + requireThreadEditResolutionRef: React.RefObject<() => boolean>; +}; + +export function resolveHuddleOpenThreadHeadId({ + isHuddleTranscript, + openThreadHeadId, + optimisticOpenThreadHeadId, +}: Pick< + HuddleThreadIsolationOptions, + "isHuddleTranscript" | "openThreadHeadId" | "optimisticOpenThreadHeadId" +>): string | null { + if (isHuddleTranscript) return null; + return optimisticOpenThreadHeadId === undefined + ? openThreadHeadId + : optimisticOpenThreadHeadId; +} + +export function useHuddleThreadIsolation({ + closeThread, + isHuddleTranscript, + openThreadHeadId, + optimisticOpenThreadHeadId, + requireThreadEditResolutionRef, +}: HuddleThreadIsolationOptions): string | null { + React.useEffect(() => { + if (!isHuddleTranscript || openThreadHeadId === null) return; + if (!requireThreadEditResolutionRef.current()) return; + closeThread(null); + }, [ + closeThread, + isHuddleTranscript, + openThreadHeadId, + requireThreadEditResolutionRef, + ]); + return resolveHuddleOpenThreadHeadId({ + isHuddleTranscript, + openThreadHeadId, + optimisticOpenThreadHeadId, + }); +} diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index f7b745cb5b1..ebbab93bb91 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3476,6 +3476,123 @@ test("main ArrowUp refuses to replace a dirty thread edit", async ({ await expect(mainInput).toHaveText(""); }); +test("main composer switches directly between visible message edits", async ({ + page, +}) => { + const first = `Main edit switch first ${Date.now()}`; + const second = `Main edit switch second ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(first); + await mainInput.press("Enter"); + await expect( + page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: first }), + ).toBeVisible(); + await page.waitForTimeout(1_100); + await mainInput.fill(second); + await mainInput.press("Enter"); + await expect( + page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: second }), + ).toBeVisible(); + + await mainInput.click(); + await page.keyboard.press("ArrowUp"); + await expect(mainInput).toHaveText(second); + + const firstMessage = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: first }) + .last(); + await firstMessage.hover(); + await firstMessage.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + + await expect(mainInput).toHaveText(first); + await expect(mainInput).toBeFocused(); + await expect(page.getByText("Finish or cancel your edit first.")).toHaveCount( + 0, + ); +}); + +test("a refused message deep link retries after the thread edit is canceled", async ({ + page, +}) => { + const sourceRoot = `Deep link retry source ${Date.now()}`; + const reply = `Deep link retry reply ${Date.now()}`; + const destinationRoot = `Deep link retry destination ${Date.now()}`; + + await page.goto("/"); + await page.getByTestId("channel-general").click(); + const mainInput = page + .getByTestId("channel-composer-overlay") + .getByTestId("message-input"); + await mainInput.fill(destinationRoot); + await mainInput.press("Enter"); + const destination = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: destinationRoot }) + .last(); + const destinationId = await destination.getAttribute("data-message-id"); + expect(destinationId).not.toBeNull(); + await mainInput.fill( + `Retry link buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=${destinationId}`, + ); + await mainInput.press("Enter"); + const destinationLink = page + .getByTestId("message-row") + .filter({ hasText: "Retry link" }) + .last() + .getByRole("button", { name: "Open message in channel general" }); + await expect(destinationLink).toBeVisible(); + + await mainInput.fill(sourceRoot); + await mainInput.press("Enter"); + const source = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: sourceRoot }) + .last(); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + await threadInput.fill(reply); + await threadInput.press("Enter"); + const threadReply = threadPanel + .getByTestId("message-row") + .filter({ hasText: reply }) + .last(); + await threadReply.hover(); + await threadReply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(`${reply} unsaved`); + + await destinationLink.click(); + await expect( + page.getByText("Finish or cancel your edit before leaving the thread."), + ).toBeVisible(); + await page.keyboard.press("Escape"); + await destinationLink.click(); + const routedDestination = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${destinationId}"]`); + await expect(threadPanel).toBeHidden(); + await expect(routedDestination).toBeVisible(); + await expect(routedDestination).toHaveClass(/route-target-highlight-fade/); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 2967fe53b99a230af88318c4f623ff8810a09169 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 15:32:45 -0700 Subject: [PATCH 07/13] fix(messages): guard composer surface transitions Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../src/features/channels/ui/ChannelPane.tsx | 7 +- .../features/channels/ui/ChannelScreen.tsx | 34 ++-- .../channels/ui/GuardedChannelPane.tsx | 21 +++ .../channels/ui/useChannelRouteTarget.ts | 6 +- .../ui/useMessageLinkNavigationGuard.ts | 12 ++ .../channels/useChannelPaneHandlers.ts | 2 + .../messages/ui/SentFromThreadLine.tsx | 5 +- desktop/src/shared/ui/markdown.tsx | 9 +- .../messageLinkNavigationGuardContext.ts | 14 ++ desktop/tests/e2e/messaging.spec.ts | 167 +++++++++++++++++- 10 files changed, 245 insertions(+), 32 deletions(-) create mode 100644 desktop/src/features/channels/ui/GuardedChannelPane.tsx create mode 100644 desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts create mode 100644 desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 19a837d56fc..dbdb2ed2345 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -468,8 +468,9 @@ export const ChannelPane = React.memo(function ChannelPane({ (message: TimelineMessage): boolean => { const currentEditTarget = editTargetRef.current; if ( - currentEditTarget?.isThreadReply === true && - currentEditTarget.id !== message.id + currentEditTarget && + currentEditTarget.id !== message.id && + currentEditTarget.isThreadReply !== isThreadReply(message.tags ?? []) ) { pendingMainEditRef.current = null; toast.info("Finish or cancel your edit first."); @@ -599,7 +600,6 @@ export const ChannelPane = React.memo(function ChannelPane({ data-testid="channel-shared-header-backdrop" /> ) : null} - {!isSinglePanelView ? (
) : null} - {/* * `AnimatePresence` keeps the focus thread drawer mounted through its exit * animation — without it the drawer's own existence condition diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 98a769ffc54..8385520b09c 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -17,7 +17,6 @@ import { } from "@/features/channels/readState/readStateFormat"; import { ChannelScreenEmptyState } from "@/features/channels/ui/ChannelScreenEmptyState"; import { ChannelScreenHeader } from "@/features/channels/ui/ChannelScreenHeader"; -import { ChannelPane } from "@/features/channels/ui/ChannelScreenLazyViews"; import { WelcomeAgentCreateDialog } from "@/features/channels/ui/WelcomeAgentCreateDialog"; import { ForumChannelContent } from "@/features/channels/ui/ForumChannelContent"; import { MembersSidebar } from "@/features/channels/ui/MembersSidebar"; @@ -87,6 +86,8 @@ import { useChannelRouteTarget } from "./useChannelRouteTarget"; import { useChannelOpenReadState } from "./useChannelOpenReadState"; import { useChannelUnreadState } from "./useChannelUnreadState"; import type { ChannelScreenProps } from "./ChannelScreen.types"; +import { GuardedChannelPane } from "./GuardedChannelPane"; +import { useMessageLinkNavigationGuard } from "./useMessageLinkNavigationGuard"; const EMPTY_RELAY_EVENTS: RelayEvent[] = []; export function ChannelScreen({ activeChannel, @@ -645,24 +646,24 @@ export function ChannelScreen({ timelineMessages, isTimelineLoading, ); - const resetComposerTargets = React.useCallback( - (_channelId: string | null) => { - setExpandedThreadReplyIds(new Set()); - setThreadScrollTargetId(null); - setThreadReplyTargetId(null); - setEditTargetId(null); - }, - [], - ); const handleThreadScrollTargetResolved = React.useCallback(() => { setThreadScrollTargetId(null); }, []); - const handleTargetReached = React.useCallback(() => { - clearMessageRouteTarget({ replace: true }); - }, [clearMessageRouteTarget]); + const handleTargetReached = React.useCallback( + () => clearMessageRouteTarget({ replace: true }), + [clearMessageRouteTarget], + ); React.useEffect(() => { - resetComposerTargets(activeChannelId); - }, [activeChannelId, resetComposerTargets]); + // The channel identity is intentionally the reset trigger. + void activeChannelId; + setExpandedThreadReplyIds(new Set()); + setThreadScrollTargetId(null); + setThreadReplyTargetId(null); + setEditTargetId(null); + }, [activeChannelId]); + const allowMessageLinkNavigation = useMessageLinkNavigationGuard( + requireThreadEditResolution, + ); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, activeChannelId, @@ -857,7 +858,8 @@ export function ChannelScreen({ /> } > - & { + messageLinkNavigationGuard: React.ComponentProps< + typeof MessageLinkNavigationGuardProvider + >["value"]; +}; + +export function GuardedChannelPane({ + messageLinkNavigationGuard, + ...props +}: GuardedChannelPaneProps) { + return ( + + + + ); +} diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.ts b/desktop/src/features/channels/ui/useChannelRouteTarget.ts index fd4ae3e2f68..39e8a6688d5 100644 --- a/desktop/src/features/channels/ui/useChannelRouteTarget.ts +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.ts @@ -121,12 +121,10 @@ export function useChannelRouteTarget({ return; } 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); + // Root message links open the reply panel. Navigation is refused before + // this route target is accepted when another composer owns a dirty edit. setOpenThreadHeadId(targetMessage.id, { replace: true }); setThreadReplyTargetId(targetMessage.id); setThreadScrollTargetId(null); diff --git a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts new file mode 100644 index 00000000000..f9a78509666 --- /dev/null +++ b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts @@ -0,0 +1,12 @@ +import * as React from "react"; + +import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; + +export function useMessageLinkNavigationGuard( + requireThreadEditResolution: () => boolean, +) { + return React.useCallback( + (_link: ParsedMessageLink) => requireThreadEditResolution(), + [requireThreadEditResolution], + ); +} diff --git a/desktop/src/features/channels/useChannelPaneHandlers.ts b/desktop/src/features/channels/useChannelPaneHandlers.ts index c7c97951214..465a0a6b612 100644 --- a/desktop/src/features/channels/useChannelPaneHandlers.ts +++ b/desktop/src/features/channels/useChannelPaneHandlers.ts @@ -150,6 +150,8 @@ export function useChannelPaneHandlers({ ]); const handleCancelEdit = React.useCallback(() => { + editTargetIdRef.current = null; + editTargetIsThreadReplyRef.current = false; setEditTargetId(null); }, [setEditTargetId]); diff --git a/desktop/src/features/messages/ui/SentFromThreadLine.tsx b/desktop/src/features/messages/ui/SentFromThreadLine.tsx index 84e3ef5d68b..f7da86ab1f9 100644 --- a/desktop/src/features/messages/ui/SentFromThreadLine.tsx +++ b/desktop/src/features/messages/ui/SentFromThreadLine.tsx @@ -5,6 +5,7 @@ import { getSentFromThreadReference } from "@/features/messages/lib/sentFromThre import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext"; import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; import { MessageLinkPill } from "@/shared/ui/markdown/MessageLinkPill"; +import { useMessageLinkNavigationGuard } from "@/shared/ui/markdown/messageLinkNavigationGuardContext"; import { MESSAGE_MARKDOWN_CLASS } from "@/shared/ui/mentionChip"; export function SentFromThreadLine({ @@ -16,15 +17,17 @@ export function SentFromThreadLine({ }) { const { channels } = useChannelNavigation(); const { goChannel } = useAppNavigation(); + const allowMessageLinkNavigation = useMessageLinkNavigationGuard(); const reference = getSentFromThreadReference(tags); const onOpenMessageLink = React.useCallback( (target: ParsedMessageLink) => { + if (!allowMessageLinkNavigation(target)) return; void goChannel(target.channelId, { messageId: target.messageId, threadRootId: target.threadRootId, }); }, - [goChannel], + [allowMessageLinkNavigation, goChannel], ); if (!channelId || !reference) return null; diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index adb525ae74e..284ba915d2f 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -118,6 +118,7 @@ import { ProgressiveImage } from "./markdown/ProgressiveImage"; import { MessageLinkPill } from "./markdown/MessageLinkPill"; import { renderCachedMarkdown } from "./markdown/nodeCache"; import { useMessageLinkPreviews } from "./markdown/useMessageLinkPreviews"; +import { useMessageLinkNavigationGuard } from "./markdown/messageLinkNavigationGuardContext"; import { MarkdownRuntimeContext, useMarkdownRuntime, @@ -1743,6 +1744,7 @@ function MarkdownInner({ const { channels: rawChannels } = useChannelNavigation(); const channels = useStableArray(rawChannels); const { goChannel, goAgents } = useAppNavigation(); + const allowMessageLinkNavigation = useMessageLinkNavigationGuard(); const onOpenChannel = React.useCallback( (channelId: string) => { void goChannel(channelId); @@ -1752,19 +1754,16 @@ function MarkdownInner({ const onOpenEntityLink = useOpenEntityLink(); const onOpenMessageLink = React.useCallback( (link: ParsedMessageLink) => { + if (!allowMessageLinkNavigation(link)) return; // Guard before URL mutation. // Always route through `goChannel` with `messageId` set: the channel // route already handles scroll-into-view + highlight via // `useAnchoredScroll` + `getEventById` backfill, and works for - // both stream-message replies and forum threads. Detecting "the thread - // root is a forum post" up front would require an event lookup we don't - // currently have synchronously; the brief explicitly allows skipping - // that detection and falling through. void goChannel(link.channelId, { messageId: link.messageId, threadRootId: link.threadRootId, }); }, - [goChannel], + [allowMessageLinkNavigation, goChannel], ); const relayOrigin = useRelayOrigin(); const resolvedLinkPreviews = useMessageLinkPreviews({ diff --git a/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts b/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts new file mode 100644 index 00000000000..f1f064e278c --- /dev/null +++ b/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts @@ -0,0 +1,14 @@ +import * as React from "react"; + +import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; + +export const MessageLinkNavigationGuardContext = React.createContext< + (link: ParsedMessageLink) => boolean +>(() => true); + +export const MessageLinkNavigationGuardProvider = + MessageLinkNavigationGuardContext.Provider; + +export function useMessageLinkNavigationGuard() { + return React.useContext(MessageLinkNavigationGuardContext); +} diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index ebbab93bb91..ab6e4ba688d 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3054,6 +3054,72 @@ test("editing a pre-seeded thread reply uses and focuses the thread composer", a await expect(threadInput).toBeFocused(); }); +test("thread composer switches directly between visible reply edits", async ({ + page, +}) => { + const root = `Thread edit switch root ${Date.now()}`; + const first = `Thread edit switch first ${Date.now()}`; + const second = `Thread edit switch second ${Date.now()}`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { firstId, rootId, secondId } = await page.evaluate( + ({ firstContent, rootContent, secondContent }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const rootEvent = emit({ + channelName: "general", + content: rootContent, + }); + const firstEvent = emit({ + channelName: "general", + content: firstContent, + parentEventId: rootEvent.id, + }); + const secondEvent = emit({ + channelName: "general", + content: secondContent, + parentEventId: rootEvent.id, + }); + return { + firstId: firstEvent.id, + rootId: rootEvent.id, + secondId: secondEvent.id, + }; + }, + { firstContent: first, rootContent: root, secondContent: second }, + ); + + await page.getByTestId("channel-general").click(); + const timelineRoot = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${rootId}"]`); + await timelineRoot.hover(); + await timelineRoot.getByRole("button", { name: "Reply" }).click(); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const secondReply = threadPanel.locator(`[data-message-id="${secondId}"]`); + await secondReply.hover(); + await secondReply.getByRole("button", { name: "More actions" }).click(); + await page.getByTestId(`edit-message-${secondId}`).click(); + await expect(threadInput).toHaveText(second); + + const firstReply = threadPanel.locator(`[data-message-id="${firstId}"]`); + await firstReply.hover(); + await firstReply.getByRole("button", { name: "More actions" }).click(); + await page.getByTestId(`edit-message-${firstId}`).click(); + + await expect(threadInput).toHaveText(first); + await expect(threadInput).toBeFocused(); + await expect(page.getByRole("menu")).toHaveCount(0); + await expect(page.getByText("Finish or cancel your edit first.")).toHaveCount( + 0, + ); +}); + test("editing a broadcast reply from a thread returns to the main composer", async ({ page, }) => { @@ -3579,20 +3645,117 @@ test("a refused message deep link retries after the thread edit is canceled", as await page.getByRole("menuitem", { name: "Edit message" }).click(); await threadInput.fill(`${reply} unsaved`); + const threadUrl = page.url(); + expect(threadUrl).toContain( + `thread=${await source.getAttribute("data-message-id")}`, + ); await destinationLink.click(); await expect( page.getByText("Finish or cancel your edit before leaving the thread."), ).toBeVisible(); - await page.keyboard.press("Escape"); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(`${reply} unsaved`); + await expect(page).toHaveURL(threadUrl); + + // The preserved edit remains rendered and cancelable rather than becoming a + // hidden target that soft-locks the route. + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await expect(threadInput).toHaveText(""); await destinationLink.click(); + await expect(page).not.toHaveURL(threadUrl); const routedDestination = page .getByTestId("message-timeline") .locator(`[data-message-id="${destinationId}"]`); - await expect(threadPanel).toBeHidden(); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("message-thread-head")).toContainText( + destinationRoot, + ); await expect(routedDestination).toBeVisible(); await expect(routedDestination).toHaveClass(/route-target-highlight-fade/); }); +test("a refused sent-from-thread link preserves the edit and retries after cancel", async ({ + page, +}) => { + const sourceRoot = `Sent-from-thread guard source ${Date.now()}`; + const sourceReply = `Sent-from-thread guard reply ${Date.now()}`; + const destinationRoot = `Sent-from-thread guard destination ${Date.now()}`; + const sharedMessage = `Sent-from-thread guard shared ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { destinationRootId, sourceRootId } = await page.evaluate( + ({ destinationRoot, sharedMessage, sourceReply, sourceRoot }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const destination = emit({ + channelName: "general", + content: destinationRoot, + }); + const source = emit({ channelName: "general", content: sourceRoot }); + emit({ + channelName: "general", + content: sourceReply, + parentEventId: source.id, + }); + emit({ + channelName: "general", + content: sharedMessage, + extraTags: [["buzz:sent-from-thread", destination.id, destinationRoot]], + }); + return { destinationRootId: destination.id, sourceRootId: source.id }; + }, + { destinationRoot, sharedMessage, sourceReply, sourceRoot }, + ); + + await page.getByTestId("channel-general").click(); + const timeline = page.getByTestId("message-timeline"); + const source = timeline.locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel + .getByTestId("message-row") + .filter({ hasText: sourceReply }) + .last(); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const threadUrl = page.url(); + expect(threadUrl).toContain(`thread=${sourceRootId}`); + const sentFromThreadLink = timeline + .getByTestId("message-row") + .filter({ hasText: sharedMessage }) + .getByTestId("sent-from-thread") + .locator("[data-message-link]"); + await sentFromThreadLink.click(); + await expect( + page.getByText("Finish or cancel your edit before leaving the thread."), + ).toBeVisible(); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + await expect(page).toHaveURL(threadUrl); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await sentFromThreadLink.click(); + await expect(page).not.toHaveURL(threadUrl); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("message-thread-head")).toContainText( + destinationRoot, + ); + await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`)); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 2c884973bcf94d2d4ede973be7cac7d53db835f2 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 17:24:29 -0700 Subject: [PATCH 08/13] fix(desktop): guard message navigation at boundary Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../messageTargetNavigationGuard.test.mjs | 28 +++++++ .../messageTargetNavigationGuard.ts | 16 ++++ .../src/app/navigation/useAppNavigation.ts | 11 ++- .../features/channels/ui/ChannelScreen.tsx | 5 +- .../channels/ui/GuardedChannelPane.tsx | 20 +---- .../ui/useMessageLinkNavigationGuard.ts | 6 +- .../messages/ui/SentFromThreadLine.tsx | 5 +- desktop/src/shared/ui/markdown.tsx | 9 +-- .../messageLinkNavigationGuardContext.ts | 14 ---- desktop/tests/e2e/messaging.spec.ts | 78 +++++++++++++++++++ 10 files changed, 142 insertions(+), 50 deletions(-) create mode 100644 desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs create mode 100644 desktop/src/app/navigation/messageTargetNavigationGuard.ts delete mode 100644 desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs new file mode 100644 index 00000000000..98a5064e2d2 --- /dev/null +++ b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +const { allowMessageTargetNavigation, registerMessageTargetNavigationGuard } = + await import("./messageTargetNavigationGuard.ts"); + +test("all message-target navigation consults the registered boundary guard", () => { + let calls = 0; + const unregister = registerMessageTargetNavigationGuard(() => { + calls += 1; + return false; + }); + + assert.equal(allowMessageTargetNavigation(), false); + assert.equal(calls, 1); + unregister(); + assert.equal(allowMessageTargetNavigation(), true); +}); + +test("stale cleanup cannot unregister a newer guard", () => { + const unregisterFirst = registerMessageTargetNavigationGuard(() => false); + const unregisterSecond = registerMessageTargetNavigationGuard(() => true); + + unregisterFirst(); + assert.equal(allowMessageTargetNavigation(), true); + unregisterSecond(); + assert.equal(allowMessageTargetNavigation(), true); +}); diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.ts b/desktop/src/app/navigation/messageTargetNavigationGuard.ts new file mode 100644 index 00000000000..0b729f90f1a --- /dev/null +++ b/desktop/src/app/navigation/messageTargetNavigationGuard.ts @@ -0,0 +1,16 @@ +type MessageTargetNavigationGuard = () => boolean; + +let activeGuard: MessageTargetNavigationGuard | null = null; + +export function allowMessageTargetNavigation(): boolean { + return activeGuard?.() ?? true; +} + +export function registerMessageTargetNavigationGuard( + guard: MessageTargetNavigationGuard, +): () => void { + activeGuard = guard; + return () => { + if (activeGuard === guard) activeGuard = null; + }; +} diff --git a/desktop/src/app/navigation/useAppNavigation.ts b/desktop/src/app/navigation/useAppNavigation.ts index c4776564e34..764f187bced 100644 --- a/desktop/src/app/navigation/useAppNavigation.ts +++ b/desktop/src/app/navigation/useAppNavigation.ts @@ -7,6 +7,7 @@ import { } from "@tanstack/react-router"; import { openSearchHitWithNavigation } from "@/app/navigation/searchHitNavigation"; +import { allowMessageTargetNavigation } from "@/app/navigation/messageTargetNavigationGuard"; import type { SearchHit } from "@/shared/api/types"; type NavigationBehavior = { @@ -256,8 +257,11 @@ export function useAppNavigation() { thread?: string; threadRootId?: string | null; }, - ) => - commitNavigation( + ) => { + if (options?.messageId && !allowMessageTargetNavigation()) { + return Promise.resolve(false); + } + return commitNavigation( { to: "/channels/$channelId", params: { @@ -282,7 +286,8 @@ export function useAppNavigation() { replace: options?.replace, resetScroll: options?.messageId ? true : undefined, }, - ), + ); + }, [commitNavigation], ); diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 8385520b09c..9818ae6fa6c 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -661,9 +661,7 @@ export function ChannelScreen({ setThreadReplyTargetId(null); setEditTargetId(null); }, [activeChannelId]); - const allowMessageLinkNavigation = useMessageLinkNavigationGuard( - requireThreadEditResolution, - ); + useMessageLinkNavigationGuard(requireThreadEditResolution); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, activeChannelId, @@ -859,7 +857,6 @@ export function ChannelScreen({ } > & { - messageLinkNavigationGuard: React.ComponentProps< - typeof MessageLinkNavigationGuardProvider - >["value"]; -}; - -export function GuardedChannelPane({ - messageLinkNavigationGuard, - ...props -}: GuardedChannelPaneProps) { - return ( - - - - ); +export function GuardedChannelPane( + props: React.ComponentProps, +) { + return ; } diff --git a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts index f9a78509666..2f56abfbf00 100644 --- a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts +++ b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts @@ -1,12 +1,12 @@ import * as React from "react"; -import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; +import { registerMessageTargetNavigationGuard } from "@/app/navigation/messageTargetNavigationGuard"; export function useMessageLinkNavigationGuard( requireThreadEditResolution: () => boolean, ) { - return React.useCallback( - (_link: ParsedMessageLink) => requireThreadEditResolution(), + React.useLayoutEffect( + () => registerMessageTargetNavigationGuard(requireThreadEditResolution), [requireThreadEditResolution], ); } diff --git a/desktop/src/features/messages/ui/SentFromThreadLine.tsx b/desktop/src/features/messages/ui/SentFromThreadLine.tsx index f7da86ab1f9..84e3ef5d68b 100644 --- a/desktop/src/features/messages/ui/SentFromThreadLine.tsx +++ b/desktop/src/features/messages/ui/SentFromThreadLine.tsx @@ -5,7 +5,6 @@ import { getSentFromThreadReference } from "@/features/messages/lib/sentFromThre import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext"; import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; import { MessageLinkPill } from "@/shared/ui/markdown/MessageLinkPill"; -import { useMessageLinkNavigationGuard } from "@/shared/ui/markdown/messageLinkNavigationGuardContext"; import { MESSAGE_MARKDOWN_CLASS } from "@/shared/ui/mentionChip"; export function SentFromThreadLine({ @@ -17,17 +16,15 @@ export function SentFromThreadLine({ }) { const { channels } = useChannelNavigation(); const { goChannel } = useAppNavigation(); - const allowMessageLinkNavigation = useMessageLinkNavigationGuard(); const reference = getSentFromThreadReference(tags); const onOpenMessageLink = React.useCallback( (target: ParsedMessageLink) => { - if (!allowMessageLinkNavigation(target)) return; void goChannel(target.channelId, { messageId: target.messageId, threadRootId: target.threadRootId, }); }, - [allowMessageLinkNavigation, goChannel], + [goChannel], ); if (!channelId || !reference) return null; diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index 284ba915d2f..af3997f8155 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -118,7 +118,6 @@ import { ProgressiveImage } from "./markdown/ProgressiveImage"; import { MessageLinkPill } from "./markdown/MessageLinkPill"; import { renderCachedMarkdown } from "./markdown/nodeCache"; import { useMessageLinkPreviews } from "./markdown/useMessageLinkPreviews"; -import { useMessageLinkNavigationGuard } from "./markdown/messageLinkNavigationGuardContext"; import { MarkdownRuntimeContext, useMarkdownRuntime, @@ -1744,7 +1743,6 @@ function MarkdownInner({ const { channels: rawChannels } = useChannelNavigation(); const channels = useStableArray(rawChannels); const { goChannel, goAgents } = useAppNavigation(); - const allowMessageLinkNavigation = useMessageLinkNavigationGuard(); const onOpenChannel = React.useCallback( (channelId: string) => { void goChannel(channelId); @@ -1754,16 +1752,15 @@ function MarkdownInner({ const onOpenEntityLink = useOpenEntityLink(); const onOpenMessageLink = React.useCallback( (link: ParsedMessageLink) => { - if (!allowMessageLinkNavigation(link)) return; // Guard before URL mutation. - // Always route through `goChannel` with `messageId` set: the channel - // route already handles scroll-into-view + highlight via + // Always route through `goChannel` with `messageId` set: the navigation + // boundary guards every message-targeting caller before URL mutation. // `useAnchoredScroll` + `getEventById` backfill, and works for void goChannel(link.channelId, { messageId: link.messageId, threadRootId: link.threadRootId, }); }, - [allowMessageLinkNavigation, goChannel], + [goChannel], ); const relayOrigin = useRelayOrigin(); const resolvedLinkPreviews = useMessageLinkPreviews({ diff --git a/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts b/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts deleted file mode 100644 index f1f064e278c..00000000000 --- a/desktop/src/shared/ui/markdown/messageLinkNavigationGuardContext.ts +++ /dev/null @@ -1,14 +0,0 @@ -import * as React from "react"; - -import type { ParsedMessageLink } from "@/features/messages/lib/messageLink"; - -export const MessageLinkNavigationGuardContext = React.createContext< - (link: ParsedMessageLink) => boolean ->(() => true); - -export const MessageLinkNavigationGuardProvider = - MessageLinkNavigationGuardContext.Provider; - -export function useMessageLinkNavigationGuard() { - return React.useContext(MessageLinkNavigationGuardContext); -} diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index ab6e4ba688d..ebf490fad0d 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3756,6 +3756,84 @@ test("a refused sent-from-thread link preserves the edit and retries after cance await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`)); }); +test("a refused search result preserves the edit and retries after cancel", async ({ + page, +}) => { + const sourceRoot = `Search guard source ${Date.now()}`; + const sourceReply = `Search guard reply ${Date.now()}`; + const destinationRoot = `Search guard destination ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved byte-for-byte`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { destinationRootId, sourceRootId } = await page.evaluate( + ({ destinationRoot, sourceReply, sourceRoot }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const destination = emit({ + channelName: "general", + content: destinationRoot, + }); + const source = emit({ channelName: "general", content: sourceRoot }); + emit({ + channelName: "general", + content: sourceReply, + parentEventId: source.id, + }); + return { destinationRootId: destination.id, sourceRootId: source.id }; + }, + { destinationRoot, sourceReply, sourceRoot }, + ); + + await page.getByTestId("channel-general").click(); + const timeline = page.getByTestId("message-timeline"); + const source = timeline.locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel + .getByTestId("message-row") + .filter({ hasText: sourceReply }) + .last(); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const threadUrl = page.url(); + expect(threadUrl).toContain(`thread=${sourceRootId}`); + await page.getByTestId("open-search").click(); + await page.getByTestId("search-dialog-input").fill(destinationRoot); + const destinationResult = page.getByTestId( + `search-result-${destinationRootId}`, + ); + await expect(destinationResult).toBeVisible(); + await destinationResult.click(); + + const refusal = page.getByText( + "Finish or cancel your edit before leaving the thread.", + ); + await expect(refusal).toHaveCount(1); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + await expect(page).toHaveURL(threadUrl); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await page.getByTestId("open-search").click(); + await page.getByTestId("search-dialog-input").fill(destinationRoot); + await destinationResult.click(); + await expect(page).not.toHaveURL(threadUrl); + await expect(threadPanel.getByTestId("message-thread-head")).toContainText( + destinationRoot, + ); + await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`)); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From c3e667eb63883f5a9772d32c7622d7ba2a2d6319 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 19:25:41 -0700 Subject: [PATCH 09/13] fix(desktop): preserve guarded message navigation Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../messageTargetNavigationGuard.test.mjs | 45 +++++++++-- .../messageTargetNavigationGuard.ts | 36 +++++++-- .../src/app/navigation/useAppNavigation.ts | 27 ++++++- .../features/channels/ui/ChannelScreen.tsx | 6 +- .../ui/useMessageLinkNavigationGuard.ts | 20 ++++- desktop/src/features/messages/hooks.ts | 10 ++- desktop/src/shared/deep-link.test.mjs | 52 ++++++++++++ desktop/src/shared/useMessageDeepLinks.ts | 3 +- desktop/tests/e2e/messaging.spec.ts | 80 +++++++++++++++++++ 9 files changed, 254 insertions(+), 25 deletions(-) diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs index 98a5064e2d2..c5d531688c7 100644 --- a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs +++ b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs @@ -4,17 +4,35 @@ import test from "node:test"; const { allowMessageTargetNavigation, registerMessageTargetNavigationGuard } = await import("./messageTargetNavigationGuard.ts"); +const target = { + kind: "channel-message", + channelId: "general", + messageId: "message-a", + threadRootId: "thread-a", +}; + test("all message-target navigation consults the registered boundary guard", () => { - let calls = 0; - const unregister = registerMessageTargetNavigationGuard(() => { - calls += 1; + let received; + const unregister = registerMessageTargetNavigationGuard((nextTarget) => { + received = nextTarget; return false; }); - assert.equal(allowMessageTargetNavigation(), false); - assert.equal(calls, 1); + assert.equal(allowMessageTargetNavigation(target), false); + assert.deepEqual(received, target); unregister(); - assert.equal(allowMessageTargetNavigation(), true); + assert.equal(allowMessageTargetNavigation(target), true); +}); + +test("unregistering the newer guard restores the prior live guard", () => { + const unregisterFirst = registerMessageTargetNavigationGuard(() => false); + const unregisterSecond = registerMessageTargetNavigationGuard(() => true); + + assert.equal(allowMessageTargetNavigation(target), true); + unregisterSecond(); + assert.equal(allowMessageTargetNavigation(target), false); + unregisterFirst(); + assert.equal(allowMessageTargetNavigation(target), true); }); test("stale cleanup cannot unregister a newer guard", () => { @@ -22,7 +40,18 @@ test("stale cleanup cannot unregister a newer guard", () => { const unregisterSecond = registerMessageTargetNavigationGuard(() => true); unregisterFirst(); - assert.equal(allowMessageTargetNavigation(), true); + assert.equal(allowMessageTargetNavigation(target), true); + unregisterSecond(); + assert.equal(allowMessageTargetNavigation(target), true); +}); + +test("duplicate callback registrations clean up by registration identity", () => { + const sharedGuard = () => false; + const unregisterFirst = registerMessageTargetNavigationGuard(sharedGuard); + const unregisterSecond = registerMessageTargetNavigationGuard(sharedGuard); + + unregisterFirst(); + assert.equal(allowMessageTargetNavigation(target), false); unregisterSecond(); - assert.equal(allowMessageTargetNavigation(), true); + assert.equal(allowMessageTargetNavigation(target), true); }); diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.ts b/desktop/src/app/navigation/messageTargetNavigationGuard.ts index 0b729f90f1a..7de85ec042b 100644 --- a/desktop/src/app/navigation/messageTargetNavigationGuard.ts +++ b/desktop/src/app/navigation/messageTargetNavigationGuard.ts @@ -1,16 +1,40 @@ -type MessageTargetNavigationGuard = () => boolean; +export type MessageTargetNavigation = + | { + kind: "channel-message"; + channelId: string; + messageId: string; + threadRootId: string | null; + } + | { + kind: "forum-post"; + channelId: string; + postId: string; + replyId: string | null; + }; -let activeGuard: MessageTargetNavigationGuard | null = null; +type MessageTargetNavigationGuard = ( + target: MessageTargetNavigation, +) => boolean; -export function allowMessageTargetNavigation(): boolean { - return activeGuard?.() ?? true; +type GuardRegistration = { + guard: MessageTargetNavigationGuard; +}; + +const activeGuards: GuardRegistration[] = []; + +export function allowMessageTargetNavigation( + target: MessageTargetNavigation, +): boolean { + return activeGuards.at(-1)?.guard(target) ?? true; } export function registerMessageTargetNavigationGuard( guard: MessageTargetNavigationGuard, ): () => void { - activeGuard = guard; + const registration = { guard }; + activeGuards.push(registration); return () => { - if (activeGuard === guard) activeGuard = null; + const index = activeGuards.lastIndexOf(registration); + if (index >= 0) activeGuards.splice(index, 1); }; } diff --git a/desktop/src/app/navigation/useAppNavigation.ts b/desktop/src/app/navigation/useAppNavigation.ts index 764f187bced..7dd9d3a45a7 100644 --- a/desktop/src/app/navigation/useAppNavigation.ts +++ b/desktop/src/app/navigation/useAppNavigation.ts @@ -258,7 +258,15 @@ export function useAppNavigation() { threadRootId?: string | null; }, ) => { - if (options?.messageId && !allowMessageTargetNavigation()) { + if ( + options?.messageId && + !allowMessageTargetNavigation({ + kind: "channel-message", + channelId, + messageId: options.messageId, + threadRootId: options.threadRootId ?? null, + }) + ) { return Promise.resolve(false); } return commitNavigation( @@ -312,8 +320,18 @@ export function useAppNavigation() { replace?: boolean; replyId?: string; }, - ) => - commitNavigation( + ) => { + if ( + !allowMessageTargetNavigation({ + kind: "forum-post", + channelId, + postId, + replyId: options?.replyId ?? null, + }) + ) { + return Promise.resolve(false); + } + return commitNavigation( { to: "/channels/$channelId/posts/$postId", params: { @@ -327,7 +345,8 @@ export function useAppNavigation() { replace: options?.replace, resetScroll: false, }, - ), + ); + }, [commitNavigation], ); diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 9818ae6fa6c..b6737e8acce 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -661,7 +661,11 @@ export function ChannelScreen({ setThreadReplyTargetId(null); setEditTargetId(null); }, [activeChannelId]); - useMessageLinkNavigationGuard(requireThreadEditResolution); + useMessageLinkNavigationGuard( + requireThreadEditResolution, + activeChannelId, + effectiveOpenThreadHeadId, + ); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, activeChannelId, diff --git a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts index 2f56abfbf00..a49bcd0f37d 100644 --- a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts +++ b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts @@ -1,12 +1,28 @@ import * as React from "react"; +import type { MessageTargetNavigation } from "@/app/navigation/messageTargetNavigationGuard"; import { registerMessageTargetNavigationGuard } from "@/app/navigation/messageTargetNavigationGuard"; export function useMessageLinkNavigationGuard( requireThreadEditResolution: () => boolean, + activeChannelId: string | null, + openThreadHeadId: string | null, ) { + const guard = React.useCallback( + (target: MessageTargetNavigation) => { + if ( + target.kind === "channel-message" && + target.channelId === activeChannelId && + target.threadRootId === openThreadHeadId + ) { + return true; + } + return requireThreadEditResolution(); + }, + [activeChannelId, openThreadHeadId, requireThreadEditResolution], + ); React.useLayoutEffect( - () => registerMessageTargetNavigationGuard(requireThreadEditResolution), - [requireThreadEditResolution], + () => registerMessageTargetNavigationGuard(guard), + [guard], ); } diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index a3c1e7f172b..d28f2926081 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -683,11 +683,17 @@ export function useSendMessageMutation( } const queryKey = channelMessagesKey(effectiveChannel.id); - await queryClient.cancelQueries({ queryKey }); + const windowKey = channelWindowKey(effectiveChannel.id); + // The rendered timeline is projected from the channel-window cache. Cancel + // both reads before snapshotting either cache so an older window response + // cannot replace the optimistic row between onMutate and onSuccess. + await Promise.all([ + queryClient.cancelQueries({ queryKey }), + queryClient.cancelQueries({ queryKey: windowKey }), + ]); const previousMessages = queryClient.getQueryData(queryKey) ?? []; - const windowKey = channelWindowKey(effectiveChannel.id); const previousWindow = queryClient.getQueryData(windowKey); const optimisticMessage = createOptimisticMessage( diff --git a/desktop/src/shared/deep-link.test.mjs b/desktop/src/shared/deep-link.test.mjs index ea478aff4ef..b6cad59567a 100644 --- a/desktop/src/shared/deep-link.test.mjs +++ b/desktop/src/shared/deep-link.test.mjs @@ -359,6 +359,58 @@ test("failed community clear quarantines stale navigation from the next listener await resetNavigationDeepLinkDrain(); }); +test("refused navigation stays at the FIFO head and retries with one acknowledgement", async () => { + const pending = { + id: "retry-me", + kind: "message", + channelId: "channel-1", + messageId: "message-1", + threadRootId: "root-1", + }; + const queue = [pending]; + const opened = []; + const acknowledged = []; + + ipcHandlers.set("plugin:event|listen", () => nextCallbackId); + ipcHandlers.set("plugin:event|unlisten", () => {}); + ipcHandlers.set("take_pending_navigation_deep_link", () => queue[0] ?? null); + ipcHandlers.set("acknowledge_pending_navigation_deep_link", ({ id }) => { + assert.equal(queue[0]?.id, id); + acknowledged.push(id); + queue.shift(); + return true; + }); + + const firstUnlisten = await listenForNavigationDeepLinks( + () => true, + (payload) => { + opened.push(`refused:${payload.messageId}`); + return false; + }, + ); + await settle(); + + assert.deepEqual(opened, ["refused:message-1"]); + assert.equal(queue[0], pending); + assert.deepEqual(acknowledged, []); + firstUnlisten(); + + const secondUnlisten = await listenForNavigationDeepLinks( + () => true, + (payload) => { + opened.push(`accepted:${payload.messageId}`); + return true; + }, + ); + await settle(); + await settle(); + + assert.deepEqual(opened, ["refused:message-1", "accepted:message-1"]); + assert.deepEqual(acknowledged, ["retry-me"]); + assert.equal(queue.length, 0); + secondUnlisten(); +}); + test("rejected navigation remains queued and is not acknowledged", async () => { const pending = { id: "retry-me", diff --git a/desktop/src/shared/useMessageDeepLinks.ts b/desktop/src/shared/useMessageDeepLinks.ts index fbbe4b9f67a..288b3812919 100644 --- a/desktop/src/shared/useMessageDeepLinks.ts +++ b/desktop/src/shared/useMessageDeepLinks.ts @@ -32,11 +32,10 @@ export function useMessageDeepLinks(enabled = true) { }, async (payload) => { if (cancelled) return false; - await goChannel(payload.channelId, { + return goChannel(payload.channelId, { messageId: payload.messageId, threadRootId: payload.threadRootId, }); - return true; }, ); return () => { diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index ebf490fad0d..b7bc97b6cdc 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3834,6 +3834,86 @@ test("a refused search result preserves the edit and retries after cancel", asyn await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`)); }); +test("a refused forum search result preserves the edit and retries after cancel", async ({ + page, +}) => { + const sourceRoot = `Forum guard source ${Date.now()}`; + const sourceReply = `Forum guard reply ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved byte-for-byte`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const sourceRootId = await page.evaluate( + ({ sourceReply, sourceRoot }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const source = emit({ channelName: "general", content: sourceRoot }); + emit({ + channelName: "general", + content: sourceReply, + parentEventId: source.id, + }); + return source.id; + }, + { sourceReply, sourceRoot }, + ); + + await page.getByTestId("channel-general").click(); + const source = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel + .getByTestId("message-row") + .filter({ hasText: sourceReply }) + .last(); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const threadUrl = page.url(); + const editTarget = threadPanel.getByTestId("edit-target"); + await expect(editTarget).toBeVisible(); + await page.getByTestId("open-search").click(); + await page + .getByTestId("search-dialog-input") + .fill("Release checklist: async feedback thread."); + const forumResult = page.getByTestId( + "search-result-mock-forum-release-thread", + ); + await expect(forumResult).toBeVisible(); + await forumResult.click(); + + const refusal = page.getByText( + "Finish or cancel your edit before leaving the thread.", + ); + await expect(refusal).toHaveCount(1); + await expect(threadPanel).toBeVisible(); + await expect(editTarget).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + await expect(page).toHaveURL(threadUrl); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await page.getByTestId("open-search").click(); + await page + .getByTestId("search-dialog-input") + .fill("Release checklist: async feedback thread."); + await forumResult.click(); + await expect(page).toHaveURL( + /#\/channels\/a27e1ee9-76a6-5bdf-a5d5-1d85610dad11\/posts\/mock-forum-release-thread$/, + ); + await expect( + page.locator('[data-forum-event-id="mock-forum-release-thread"]'), + ).toContainText("Release checklist: async feedback thread."); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 1b207bac111275dff70ec0c1ef4028dee0d98c81 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Sat, 22 Aug 2026 21:15:52 -0700 Subject: [PATCH 10/13] fix(desktop): guard same-thread message navigation Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho --- .../features/channels/ui/ChannelScreen.tsx | 6 +- .../ui/useMessageLinkNavigationGuard.ts | 21 +--- desktop/tests/e2e/messaging.spec.ts | 99 +++++++++++++++++++ 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index b6737e8acce..9818ae6fa6c 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -661,11 +661,7 @@ export function ChannelScreen({ setThreadReplyTargetId(null); setEditTargetId(null); }, [activeChannelId]); - useMessageLinkNavigationGuard( - requireThreadEditResolution, - activeChannelId, - effectiveOpenThreadHeadId, - ); + useMessageLinkNavigationGuard(requireThreadEditResolution); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, activeChannelId, diff --git a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts index a49bcd0f37d..68b5c83c66b 100644 --- a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts +++ b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts @@ -1,28 +1,13 @@ import * as React from "react"; -import type { MessageTargetNavigation } from "@/app/navigation/messageTargetNavigationGuard"; import { registerMessageTargetNavigationGuard } from "@/app/navigation/messageTargetNavigationGuard"; export function useMessageLinkNavigationGuard( requireThreadEditResolution: () => boolean, - activeChannelId: string | null, - openThreadHeadId: string | null, ) { - const guard = React.useCallback( - (target: MessageTargetNavigation) => { - if ( - target.kind === "channel-message" && - target.channelId === activeChannelId && - target.threadRootId === openThreadHeadId - ) { - return true; - } - return requireThreadEditResolution(); - }, - [activeChannelId, openThreadHeadId, requireThreadEditResolution], - ); React.useLayoutEffect( - () => registerMessageTargetNavigationGuard(guard), - [guard], + () => + registerMessageTargetNavigationGuard(() => requireThreadEditResolution()), + [requireThreadEditResolution], ); } diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index b7bc97b6cdc..5d3c6445a7f 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3914,6 +3914,105 @@ test("a refused forum search result preserves the edit and retries after cancel" ).toContainText("Release checklist: async feedback thread."); }); +for (const targetKind of ["reply", "root"] as const) { + test(`a refused same-thread ${targetKind} target preserves the edit and retries after cancel`, async ({ + page, + }) => { + const sourceRoot = `Same-thread ${targetKind} guard root ${Date.now()}`; + const sourceReply = `Same-thread ${targetKind} guard reply ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved byte-for-byte 🧵`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { sourceReplyId, sourceRootId } = await page.evaluate( + ({ sourceReply, sourceRoot, targetKind }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const root = emit({ channelName: "general", content: sourceRoot }); + const reply = emit({ + channelName: "general", + content: sourceReply, + parentEventId: root.id, + }); + const targetId = targetKind === "reply" ? reply.id : root.id; + emit({ + channelName: "general", + content: `Same-thread ${targetKind} target buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=${targetId}&thread=${root.id}`, + }); + return { sourceReplyId: reply.id, sourceRootId: root.id }; + }, + { sourceReply, sourceRoot, targetKind }, + ); + + await page.getByTestId("channel-general").click(); + const timeline = page.getByTestId("message-timeline"); + const source = timeline.locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel.locator(`[data-message-id="${sourceReplyId}"]`); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const targetLink = timeline + .getByTestId("message-row") + .filter({ hasText: `Same-thread ${targetKind} target` }) + .getByRole("button", { name: "Open message in channel general" }); + const navigationBefore = await page.evaluate(() => ({ + historyLength: history.length, + url: location.href, + })); + const sendsBefore = await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ); + + await targetLink.click(); + + const refusal = page.getByText( + "Finish or cancel your edit before leaving the thread.", + ); + await expect(refusal).toHaveCount(1); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + expect(await threadInput.textContent()).toBe(dirtyReply); + await expect(page).toHaveURL(navigationBefore.url); + expect(await page.evaluate(() => history.length)).toBe( + navigationBefore.historyLength, + ); + expect( + await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ), + ).toBe(sendsBefore); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await targetLink.click(); + await expect + .poll(() => page.evaluate(() => history.length)) + .toBeGreaterThan(navigationBefore.historyLength); + await expect(threadPanel).toBeVisible(); + await expect( + threadPanel.locator( + `[data-message-id="${targetKind === "reply" ? sourceReplyId : sourceRootId}"]`, + ), + ).toBeVisible(); + }); +} + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From 7ae53342a8e8052edd0bbfe5c3ea81e4a96ef6d6 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Mon, 24 Aug 2026 09:26:12 -0700 Subject: [PATCH 11/13] fix(desktop): preserve reply edits across navigation Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../messageTargetNavigationGuard.test.mjs | 57 ------------- .../app/navigation/navigationGuard.test.mjs | 58 +++++++++++++ ...tNavigationGuard.ts => navigationGuard.ts} | 20 ++--- .../src/app/navigation/useAppNavigation.ts | 49 ++++++----- .../features/channels/ui/ChannelScreen.tsx | 4 +- .../ui/useMessageLinkNavigationGuard.ts | 13 --- .../channels/ui/useNavigationGuard.ts | 10 +++ desktop/tests/e2e/messaging.spec.ts | 82 +++++++++++++++++++ 8 files changed, 188 insertions(+), 105 deletions(-) delete mode 100644 desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs create mode 100644 desktop/src/app/navigation/navigationGuard.test.mjs rename desktop/src/app/navigation/{messageTargetNavigationGuard.ts => navigationGuard.ts} (61%) delete mode 100644 desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts create mode 100644 desktop/src/features/channels/ui/useNavigationGuard.ts diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs deleted file mode 100644 index c5d531688c7..00000000000 --- a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs +++ /dev/null @@ -1,57 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -const { allowMessageTargetNavigation, registerMessageTargetNavigationGuard } = - await import("./messageTargetNavigationGuard.ts"); - -const target = { - kind: "channel-message", - channelId: "general", - messageId: "message-a", - threadRootId: "thread-a", -}; - -test("all message-target navigation consults the registered boundary guard", () => { - let received; - const unregister = registerMessageTargetNavigationGuard((nextTarget) => { - received = nextTarget; - return false; - }); - - assert.equal(allowMessageTargetNavigation(target), false); - assert.deepEqual(received, target); - unregister(); - assert.equal(allowMessageTargetNavigation(target), true); -}); - -test("unregistering the newer guard restores the prior live guard", () => { - const unregisterFirst = registerMessageTargetNavigationGuard(() => false); - const unregisterSecond = registerMessageTargetNavigationGuard(() => true); - - assert.equal(allowMessageTargetNavigation(target), true); - unregisterSecond(); - assert.equal(allowMessageTargetNavigation(target), false); - unregisterFirst(); - assert.equal(allowMessageTargetNavigation(target), true); -}); - -test("stale cleanup cannot unregister a newer guard", () => { - const unregisterFirst = registerMessageTargetNavigationGuard(() => false); - const unregisterSecond = registerMessageTargetNavigationGuard(() => true); - - unregisterFirst(); - assert.equal(allowMessageTargetNavigation(target), true); - unregisterSecond(); - assert.equal(allowMessageTargetNavigation(target), true); -}); - -test("duplicate callback registrations clean up by registration identity", () => { - const sharedGuard = () => false; - const unregisterFirst = registerMessageTargetNavigationGuard(sharedGuard); - const unregisterSecond = registerMessageTargetNavigationGuard(sharedGuard); - - unregisterFirst(); - assert.equal(allowMessageTargetNavigation(target), false); - unregisterSecond(); - assert.equal(allowMessageTargetNavigation(target), true); -}); diff --git a/desktop/src/app/navigation/navigationGuard.test.mjs b/desktop/src/app/navigation/navigationGuard.test.mjs new file mode 100644 index 00000000000..48490ebe9e3 --- /dev/null +++ b/desktop/src/app/navigation/navigationGuard.test.mjs @@ -0,0 +1,58 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +const { allowNavigation, registerNavigationGuard } = await import( + "./navigationGuard.ts" +); + +const target = { + kind: "channel-message", + channelId: "general", + messageId: "message-a", + threadRootId: "thread-a", +}; + +test("all navigation consults the registered boundary guard", () => { + let received; + const unregister = registerNavigationGuard((nextTarget) => { + received = nextTarget; + return false; + }); + + assert.equal(allowNavigation(target), false); + assert.deepEqual(received, target); + unregister(); + assert.equal(allowNavigation(target), true); +}); + +test("unregistering the newer guard restores the prior live guard", () => { + const unregisterFirst = registerNavigationGuard(() => false); + const unregisterSecond = registerNavigationGuard(() => true); + + assert.equal(allowNavigation(target), true); + unregisterSecond(); + assert.equal(allowNavigation(target), false); + unregisterFirst(); + assert.equal(allowNavigation(target), true); +}); + +test("stale cleanup cannot unregister a newer guard", () => { + const unregisterFirst = registerNavigationGuard(() => false); + const unregisterSecond = registerNavigationGuard(() => true); + + unregisterFirst(); + assert.equal(allowNavigation(target), true); + unregisterSecond(); + assert.equal(allowNavigation(target), true); +}); + +test("duplicate callback registrations clean up by registration identity", () => { + const sharedGuard = () => false; + const unregisterFirst = registerNavigationGuard(sharedGuard); + const unregisterSecond = registerNavigationGuard(sharedGuard); + + unregisterFirst(); + assert.equal(allowNavigation(target), false); + unregisterSecond(); + assert.equal(allowNavigation(target), true); +}); diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.ts b/desktop/src/app/navigation/navigationGuard.ts similarity index 61% rename from desktop/src/app/navigation/messageTargetNavigationGuard.ts rename to desktop/src/app/navigation/navigationGuard.ts index 7de85ec042b..8029aa5d0a4 100644 --- a/desktop/src/app/navigation/messageTargetNavigationGuard.ts +++ b/desktop/src/app/navigation/navigationGuard.ts @@ -1,4 +1,8 @@ -export type MessageTargetNavigation = +export type GuardedNavigation = + | { + kind: "route"; + href: string; + } | { kind: "channel-message"; channelId: string; @@ -12,25 +16,19 @@ export type MessageTargetNavigation = replyId: string | null; }; -type MessageTargetNavigationGuard = ( - target: MessageTargetNavigation, -) => boolean; +type NavigationGuard = (target: GuardedNavigation) => boolean; type GuardRegistration = { - guard: MessageTargetNavigationGuard; + guard: NavigationGuard; }; const activeGuards: GuardRegistration[] = []; -export function allowMessageTargetNavigation( - target: MessageTargetNavigation, -): boolean { +export function allowNavigation(target: GuardedNavigation): boolean { return activeGuards.at(-1)?.guard(target) ?? true; } -export function registerMessageTargetNavigationGuard( - guard: MessageTargetNavigationGuard, -): () => void { +export function registerNavigationGuard(guard: NavigationGuard): () => void { const registration = { guard }; activeGuards.push(registration); return () => { diff --git a/desktop/src/app/navigation/useAppNavigation.ts b/desktop/src/app/navigation/useAppNavigation.ts index 7dd9d3a45a7..bc0a92ff748 100644 --- a/desktop/src/app/navigation/useAppNavigation.ts +++ b/desktop/src/app/navigation/useAppNavigation.ts @@ -7,7 +7,10 @@ import { } from "@tanstack/react-router"; import { openSearchHitWithNavigation } from "@/app/navigation/searchHitNavigation"; -import { allowMessageTargetNavigation } from "@/app/navigation/messageTargetNavigationGuard"; +import { + allowNavigation, + type GuardedNavigation, +} from "@/app/navigation/navigationGuard"; import type { SearchHit } from "@/shared/api/types"; type NavigationBehavior = { @@ -31,6 +34,7 @@ export function useAppNavigation() { state?: Record; }, behavior: NavigationBehavior = {}, + guardedTarget?: GuardedNavigation, ) => { const nextLocation = router.buildLocation(next as never); @@ -38,6 +42,14 @@ export function useAppNavigation() { return false; } + if ( + !allowNavigation( + guardedTarget ?? { kind: "route", href: nextLocation.href }, + ) + ) { + return false; + } + await navigate({ ...next, replace: behavior.replace, @@ -258,17 +270,6 @@ export function useAppNavigation() { threadRootId?: string | null; }, ) => { - if ( - options?.messageId && - !allowMessageTargetNavigation({ - kind: "channel-message", - channelId, - messageId: options.messageId, - threadRootId: options.threadRootId ?? null, - }) - ) { - return Promise.resolve(false); - } return commitNavigation( { to: "/channels/$channelId", @@ -294,6 +295,14 @@ export function useAppNavigation() { replace: options?.replace, resetScroll: options?.messageId ? true : undefined, }, + options?.messageId + ? { + kind: "channel-message", + channelId, + messageId: options.messageId, + threadRootId: options.threadRootId ?? null, + } + : undefined, ); }, [commitNavigation], @@ -321,16 +330,6 @@ export function useAppNavigation() { replyId?: string; }, ) => { - if ( - !allowMessageTargetNavigation({ - kind: "forum-post", - channelId, - postId, - replyId: options?.replyId ?? null, - }) - ) { - return Promise.resolve(false); - } return commitNavigation( { to: "/channels/$channelId/posts/$postId", @@ -345,6 +344,12 @@ export function useAppNavigation() { replace: options?.replace, resetScroll: false, }, + { + kind: "forum-post", + channelId, + postId, + replyId: options?.replyId ?? null, + }, ); }, [commitNavigation], diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 9818ae6fa6c..142468e6485 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -87,7 +87,7 @@ import { useChannelOpenReadState } from "./useChannelOpenReadState"; import { useChannelUnreadState } from "./useChannelUnreadState"; import type { ChannelScreenProps } from "./ChannelScreen.types"; import { GuardedChannelPane } from "./GuardedChannelPane"; -import { useMessageLinkNavigationGuard } from "./useMessageLinkNavigationGuard"; +import { useNavigationGuard } from "./useNavigationGuard"; const EMPTY_RELAY_EVENTS: RelayEvent[] = []; export function ChannelScreen({ activeChannel, @@ -661,7 +661,7 @@ export function ChannelScreen({ setThreadReplyTargetId(null); setEditTargetId(null); }, [activeChannelId]); - useMessageLinkNavigationGuard(requireThreadEditResolution); + useNavigationGuard(requireThreadEditResolution); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, activeChannelId, diff --git a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts b/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts deleted file mode 100644 index 68b5c83c66b..00000000000 --- a/desktop/src/features/channels/ui/useMessageLinkNavigationGuard.ts +++ /dev/null @@ -1,13 +0,0 @@ -import * as React from "react"; - -import { registerMessageTargetNavigationGuard } from "@/app/navigation/messageTargetNavigationGuard"; - -export function useMessageLinkNavigationGuard( - requireThreadEditResolution: () => boolean, -) { - React.useLayoutEffect( - () => - registerMessageTargetNavigationGuard(() => requireThreadEditResolution()), - [requireThreadEditResolution], - ); -} diff --git a/desktop/src/features/channels/ui/useNavigationGuard.ts b/desktop/src/features/channels/ui/useNavigationGuard.ts new file mode 100644 index 00000000000..bda71513c07 --- /dev/null +++ b/desktop/src/features/channels/ui/useNavigationGuard.ts @@ -0,0 +1,10 @@ +import * as React from "react"; + +import { registerNavigationGuard } from "@/app/navigation/navigationGuard"; + +export function useNavigationGuard(requireThreadEditResolution: () => boolean) { + React.useLayoutEffect( + () => registerNavigationGuard(() => requireThreadEditResolution()), + [requireThreadEditResolution], + ); +} diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 5d3c6445a7f..30e608ca826 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -4013,6 +4013,88 @@ for (const targetKind of ["reply", "root"] as const) { }); } +test("a refused channel switch preserves the reply edit and retries after cancel", async ({ + page, +}) => { + const sourceRoot = `Channel-switch guard root ${Date.now()}`; + const sourceReply = `Channel-switch guard reply ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved byte-for-byte 🧵`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { sourceReplyId, sourceRootId } = await page.evaluate( + ({ sourceReply, sourceRoot }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const root = emit({ channelName: "general", content: sourceRoot }); + const reply = emit({ + channelName: "general", + content: sourceReply, + parentEventId: root.id, + }); + return { sourceReplyId: reply.id, sourceRootId: root.id }; + }, + { sourceReply, sourceRoot }, + ); + + await page.getByTestId("channel-general").click(); + const source = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel.locator(`[data-message-id="${sourceReplyId}"]`); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const navigationBefore = await page.evaluate(() => ({ + historyLength: history.length, + url: location.href, + })); + const sendsBefore = await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ); + + await page.getByTestId("channel-random").click(); + + await expect( + page.getByText("Finish or cancel your edit before leaving the thread."), + ).toHaveCount(1); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + expect(await threadInput.textContent()).toBe(dirtyReply); + await expect(page).toHaveURL(navigationBefore.url); + expect(await page.evaluate(() => history.length)).toBe( + navigationBefore.historyLength, + ); + expect( + await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ), + ).toBe(sendsBefore); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await page.getByTestId("channel-random").click(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + await expect(page).not.toHaveURL(navigationBefore.url); +}); + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => { From b6e34e9975917dba39a4d94cc74a216a067f9f54 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Mon, 24 Aug 2026 09:44:30 -0700 Subject: [PATCH 12/13] fix(desktop): satisfy channel screen size ratchet Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../features/channels/ui/ChannelScreen.tsx | 31 ++++++++----------- .../channels/ui/useChannelTargetReset.ts | 30 ++++++++++++++++++ 2 files changed, 43 insertions(+), 18 deletions(-) create mode 100644 desktop/src/features/channels/ui/useChannelTargetReset.ts diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 142468e6485..aac93d7f8d7 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -82,6 +82,7 @@ import { useChannelAgentSessions } from "./useChannelAgentSessions"; import { useMessageProfiles } from "./useMessageProfiles"; import { useChannelPanelHistoryState } from "./useChannelPanelHistoryState"; import { useChannelProfilePanel } from "./useChannelProfilePanel"; +import { useChannelTargetReset } from "./useChannelTargetReset"; import { useChannelRouteTarget } from "./useChannelRouteTarget"; import { useChannelOpenReadState } from "./useChannelOpenReadState"; import { useChannelUnreadState } from "./useChannelUnreadState"; @@ -646,21 +647,13 @@ export function ChannelScreen({ timelineMessages, isTimelineLoading, ); - const handleThreadScrollTargetResolved = React.useCallback(() => { - setThreadScrollTargetId(null); - }, []); - const handleTargetReached = React.useCallback( - () => clearMessageRouteTarget({ replace: true }), - [clearMessageRouteTarget], - ); - React.useEffect(() => { - // The channel identity is intentionally the reset trigger. - void activeChannelId; - setExpandedThreadReplyIds(new Set()); - setThreadScrollTargetId(null); - setThreadReplyTargetId(null); - setEditTargetId(null); - }, [activeChannelId]); + useChannelTargetReset({ + activeChannelId, + setEditTargetId, + setExpandedThreadReplyIds, + setThreadReplyTargetId, + setThreadScrollTargetId, + }); useNavigationGuard(requireThreadEditResolution); const mainTimelineTargetMessageId = useChannelRouteTarget({ activeChannel, @@ -947,11 +940,13 @@ export function ChannelScreen({ onSendToChannel={handleSendToChannel} onSendVideoReviewComment={effectiveSendVideoReviewComment} onSendThreadReply={handleSendThreadReply} - onThreadScrollTargetResolved={ - handleThreadScrollTargetResolved + onThreadScrollTargetResolved={() => + setThreadScrollTargetId(null) } onThreadPanelResizeStart={handleThreadPanelResizeStart} - onTargetReached={handleTargetReached} + onTargetReached={() => + clearMessageRouteTarget({ replace: true }) + } onToggleReaction={effectiveToggleReaction} openAgentSessionChannelId={openAgentSessionChannelId} openAgentSessionPubkey={openAgentSessionPubkey} diff --git a/desktop/src/features/channels/ui/useChannelTargetReset.ts b/desktop/src/features/channels/ui/useChannelTargetReset.ts new file mode 100644 index 00000000000..83e1343db63 --- /dev/null +++ b/desktop/src/features/channels/ui/useChannelTargetReset.ts @@ -0,0 +1,30 @@ +import * as React from "react"; + +export function useChannelTargetReset({ + activeChannelId, + setEditTargetId, + setExpandedThreadReplyIds, + setThreadReplyTargetId, + setThreadScrollTargetId, +}: { + activeChannelId: string | null; + setEditTargetId: (id: string | null) => void; + setExpandedThreadReplyIds: (ids: Set) => void; + setThreadReplyTargetId: (id: string | null) => void; + setThreadScrollTargetId: (id: string | null) => void; +}) { + React.useEffect(() => { + // The channel identity is intentionally the reset trigger. + void activeChannelId; + setExpandedThreadReplyIds(new Set()); + setThreadScrollTargetId(null); + setThreadReplyTargetId(null); + setEditTargetId(null); + }, [ + activeChannelId, + setEditTargetId, + setExpandedThreadReplyIds, + setThreadReplyTargetId, + setThreadScrollTargetId, + ]); +} From e43d2835a68a697dd80d3aba4386922337cf75cb Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Mon, 24 Aug 2026 12:00:18 -0700 Subject: [PATCH 13/13] fix(desktop): preserve reply edits during history navigation Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../app/navigation/navigationGuard.test.mjs | 50 +++++++++- desktop/src/app/navigation/navigationGuard.ts | 16 ++++ .../src/app/navigation/useAppNavigation.ts | 7 +- .../app/navigation/useBackForwardControls.ts | 5 +- .../src/app/routes/WorkflowsRouteScreen.tsx | 5 +- desktop/tests/e2e/messaging.spec.ts | 94 +++++++++++++++++++ 6 files changed, 166 insertions(+), 11 deletions(-) diff --git a/desktop/src/app/navigation/navigationGuard.test.mjs b/desktop/src/app/navigation/navigationGuard.test.mjs index 48490ebe9e3..4fb72329b7c 100644 --- a/desktop/src/app/navigation/navigationGuard.test.mjs +++ b/desktop/src/app/navigation/navigationGuard.test.mjs @@ -1,10 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -const { allowNavigation, registerNavigationGuard } = await import( - "./navigationGuard.ts" -); - const target = { kind: "channel-message", channelId: "general", @@ -12,6 +8,9 @@ const target = { threadRootId: "thread-a", }; +const { allowNavigation, registerNavigationGuard, traverseHistory } = + await import("./navigationGuard.ts"); + test("all navigation consults the registered boundary guard", () => { let received; const unregister = registerNavigationGuard((nextTarget) => { @@ -25,6 +24,49 @@ test("all navigation consults the registered boundary guard", () => { assert.equal(allowNavigation(target), true); }); +test("guarded history traversal blocks before mutating history", () => { + let received; + let backCalls = 0; + const unregister = registerNavigationGuard((nextTarget) => { + received = nextTarget; + return false; + }); + + assert.equal( + traverseHistory( + { + back: () => { + backCalls += 1; + }, + forward: () => {}, + }, + "back", + ), + false, + ); + assert.deepEqual(received, { kind: "history", direction: "back" }); + assert.equal(backCalls, 0); + unregister(); +}); + +test("guarded history traversal invokes the selected direction when allowed", () => { + let forwardCalls = 0; + + assert.equal( + traverseHistory( + { + back: () => {}, + forward: () => { + forwardCalls += 1; + }, + }, + "forward", + ), + true, + ); + assert.equal(forwardCalls, 1); +}); + test("unregistering the newer guard restores the prior live guard", () => { const unregisterFirst = registerNavigationGuard(() => false); const unregisterSecond = registerNavigationGuard(() => true); diff --git a/desktop/src/app/navigation/navigationGuard.ts b/desktop/src/app/navigation/navigationGuard.ts index 8029aa5d0a4..5ff853720b4 100644 --- a/desktop/src/app/navigation/navigationGuard.ts +++ b/desktop/src/app/navigation/navigationGuard.ts @@ -1,4 +1,8 @@ export type GuardedNavigation = + | { + kind: "history"; + direction: "back" | "forward"; + } | { kind: "route"; href: string; @@ -28,6 +32,18 @@ export function allowNavigation(target: GuardedNavigation): boolean { return activeGuards.at(-1)?.guard(target) ?? true; } +export function traverseHistory( + history: Pick, + direction: "back" | "forward", +): boolean { + if (!allowNavigation({ kind: "history", direction })) { + return false; + } + + history[direction](); + return true; +} + export function registerNavigationGuard(guard: NavigationGuard): () => void { const registration = { guard }; activeGuards.push(registration); diff --git a/desktop/src/app/navigation/useAppNavigation.ts b/desktop/src/app/navigation/useAppNavigation.ts index bc0a92ff748..7a21f0dfbe1 100644 --- a/desktop/src/app/navigation/useAppNavigation.ts +++ b/desktop/src/app/navigation/useAppNavigation.ts @@ -10,6 +10,7 @@ import { openSearchHitWithNavigation } from "@/app/navigation/searchHitNavigatio import { allowNavigation, type GuardedNavigation, + traverseHistory, } from "@/app/navigation/navigationGuard"; import type { SearchHit } from "@/shared/api/types"; @@ -369,7 +370,7 @@ export function useAppNavigation() { const closeSettings = React.useCallback(() => { if (canGoBack) { - router.history.back(); + traverseHistory(router.history, "back"); return; } @@ -378,7 +379,7 @@ export function useAppNavigation() { const closeWorkflowDetail = React.useCallback(() => { if (canGoBack) { - router.history.back(); + traverseHistory(router.history, "back"); return; } @@ -388,7 +389,7 @@ export function useAppNavigation() { const closeForumPost = React.useCallback( (channelId: string) => { if (canGoBack) { - router.history.back(); + traverseHistory(router.history, "back"); return; } diff --git a/desktop/src/app/navigation/useBackForwardControls.ts b/desktop/src/app/navigation/useBackForwardControls.ts index e5513247d50..717e62153e1 100644 --- a/desktop/src/app/navigation/useBackForwardControls.ts +++ b/desktop/src/app/navigation/useBackForwardControls.ts @@ -8,6 +8,7 @@ import { isTauri } from "@tauri-apps/api/core"; import { listen } from "@tauri-apps/api/event"; import { matchBackForwardChord } from "@/app/navigation/backForwardChords"; +import { traverseHistory } from "@/app/navigation/navigationGuard"; import { isMacPlatform } from "@/shared/lib/platform"; import { trimMapToSize } from "@/shared/lib/trimMapToSize"; @@ -59,7 +60,7 @@ export function useBackForwardControls() { return; } - router.history.back(); + traverseHistory(router.history, "back"); }, [canGoBack, router.history]); const goForward = React.useCallback(() => { @@ -67,7 +68,7 @@ export function useBackForwardControls() { return; } - router.history.forward(); + traverseHistory(router.history, "forward"); }, [canGoForward, router.history]); const handleKeyDown = React.useEffectEvent((event: KeyboardEvent) => { diff --git a/desktop/src/app/routes/WorkflowsRouteScreen.tsx b/desktop/src/app/routes/WorkflowsRouteScreen.tsx index 193695f0cd2..8c476b2863f 100644 --- a/desktop/src/app/routes/WorkflowsRouteScreen.tsx +++ b/desktop/src/app/routes/WorkflowsRouteScreen.tsx @@ -18,6 +18,7 @@ export function WorkflowsRouteScreen({ onEditorPaneChange, }: WorkflowsRouteScreenProps) { const { + closeWorkflowDetail, goDuplicateWorkflow, goEditWorkflow, goNewWorkflow, @@ -26,11 +27,11 @@ export function WorkflowsRouteScreen({ } = useAppNavigation(); const closeEditor = React.useCallback(() => { if (editor?.hasOrigin) { - window.history.back(); + closeWorkflowDetail(); return; } void goWorkflows({ replace: true }); - }, [editor?.hasOrigin, goWorkflows]); + }, [closeWorkflowDetail, editor?.hasOrigin, goWorkflows]); const channelsQuery = useChannelsQuery(); const channels = channelsQuery.data ?? []; const memberChannels = channels.filter((channel) => channel.isMember); diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index 30e608ca826..d24c2744133 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -4095,6 +4095,100 @@ test("a refused channel switch preserves the reply edit and retries after cancel await expect(page).not.toHaveURL(navigationBefore.url); }); +for (const backInput of ["button", "keyboard"] as const) { + test(`a refused ${backInput} Back preserves the reply edit and retries after cancel`, async ({ + page, + }) => { + const sourceRoot = `History guard root ${backInput} ${Date.now()}`; + const sourceReply = `History guard reply ${backInput} ${Date.now()}`; + const dirtyReply = `${sourceReply} unsaved byte-for-byte 🧵`; + + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const { sourceReplyId, sourceRootId } = await page.evaluate( + ({ sourceReply, sourceRoot }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const root = emit({ channelName: "general", content: sourceRoot }); + const reply = emit({ + channelName: "general", + content: sourceReply, + parentEventId: root.id, + }); + return { sourceReplyId: reply.id, sourceRootId: root.id }; + }, + { sourceReply, sourceRoot }, + ); + + await page.getByTestId("channel-random").click(); + await page.getByTestId("channel-general").click(); + const source = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${sourceRootId}"]`); + await source.hover(); + await source.getByRole("button", { name: "Reply" }).click({ force: true }); + + const threadPanel = page.getByTestId("message-thread-panel"); + const threadInput = threadPanel.getByTestId("message-input"); + const reply = threadPanel.locator(`[data-message-id="${sourceReplyId}"]`); + await reply.hover(); + await reply.getByRole("button", { name: "More actions" }).click(); + await page.getByRole("menuitem", { name: "Edit message" }).click(); + await threadInput.fill(dirtyReply); + + const navigationBefore = await page.evaluate(() => ({ + historyLength: history.length, + url: location.href, + })); + const sendsBefore = await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ); + const invokeBack = async () => { + if (backInput === "button") { + await page.getByTestId("global-back").click(); + return; + } + await page.keyboard.press( + process.platform === "darwin" ? "Meta+[" : "Alt+ArrowLeft", + ); + }; + + await invokeBack(); + + await expect( + page.getByText("Finish or cancel your edit before leaving the thread."), + ).toHaveCount(1); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect(threadPanel).toBeVisible(); + await expect(threadPanel.getByTestId("edit-target")).toBeVisible(); + await expect(threadInput).toHaveText(dirtyReply); + expect(await threadInput.textContent()).toBe(dirtyReply); + await expect(page).toHaveURL(navigationBefore.url); + expect(await page.evaluate(() => history.length)).toBe( + navigationBefore.historyLength, + ); + expect( + await page.evaluate( + () => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === "send_channel_message", + ).length, + ), + ).toBe(sendsBefore); + + await threadInput.press("Escape"); + await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0); + await expect(page.getByTestId("global-back")).toBeEnabled(); + await invokeBack(); + await expect(page).not.toHaveURL(navigationBefore.url); + }); +} + test("ArrowUp in an empty composer edits your last message right after sending", async ({ page, }) => {