diff --git a/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs b/desktop/src/app/navigation/messageTargetNavigationGuard.test.mjs new file mode 100644 index 0000000000..98a5064e2d --- /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 0000000000..0b729f90f1 --- /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 c4776564e3..764f187bce 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 8385520b09..9818ae6fa6 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 f9a7850966..2f56abfbf0 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 f7da86ab1f..84e3ef5d68 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 284ba915d2..af3997f815 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 f1f064e278..0000000000 --- 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 ab6e4ba688..ebf490fad0 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, }) => {