From c21fb0d1d03c5d00d5bd232e5e5c4ea5be82ba9f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 26 Jul 2026 17:23:48 -0700 Subject: [PATCH] fix(dashboard): stop Mailbox reply-context rows collapsing on unrelated re-renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReplyContextExpandable was declared inside MailboxModal's render, making it a new element type on every render. Any parent update remounted the whole recursive reply thread, so expanding one reply row collapsed the others and discarded their DOM identity, focus, and scroll position. Hoist it to module scope and pass the parent's reply state and handlers through an explicit env prop so the element type stays stable. Adds a regression test that expands one row, expands a second, and asserts the first keeps both its node identity and aria-expanded state — same defect class as the FN-8606 ModalShell fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../fix-mailbox-reply-context-remount.md | 7 + .../dashboard/app/components/MailboxModal.tsx | 196 +++++++++++------- .../__tests__/MailboxModal.test.tsx | 52 +++++ 3 files changed, 178 insertions(+), 77 deletions(-) create mode 100644 .changeset/fix-mailbox-reply-context-remount.md diff --git a/.changeset/fix-mailbox-reply-context-remount.md b/.changeset/fix-mailbox-reply-context-remount.md new file mode 100644 index 0000000000..e783da0271 --- /dev/null +++ b/.changeset/fix-mailbox-reply-context-remount.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep expanded Mailbox reply-context rows open when another row is expanded. +category: fix +dev: `ReplyContextExpandable` was declared inside `MailboxModal`'s render, so every parent update produced a new element type and remounted the recursive reply thread, collapsing already-expanded rows. Hoisted to module scope with an explicit `env` prop. diff --git a/packages/dashboard/app/components/MailboxModal.tsx b/packages/dashboard/app/components/MailboxModal.tsx index 336b6cf385..c9b932067e 100644 --- a/packages/dashboard/app/components/MailboxModal.tsx +++ b/packages/dashboard/app/components/MailboxModal.tsx @@ -170,6 +170,109 @@ function buildReplyThread(messages: Message[], selectedMessage: Message): Messag .sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()); } +/* +FNXC:Mailbox 2026-07-26-20:10: +Reply-context rows MUST stay a module-scope component, never one declared inside MailboxModal's render. +A component declared in render is a fresh element type on every render, so React unmounts and remounts the +whole recursive reply thread on each parent update — expanded rows lose their DOM identity, in-flight focus +and scroll position are discarded, and every nested level re-mounts on unrelated mailbox state changes. +The parent's reply state and handlers arrive through `env` so the element type stays stable. +(Same defect class as FN-8606's Planning/Settings ModalShell, which made those surfaces untypable.) +*/ +interface ReplyContextEnv { + replyContextCache: ReadonlyMap; + replyContextExpanded: Record; + replyContextLoading: Record; + replyContextErrors: Record; + setReplyExpanded: (key: string, isExpanded: boolean) => void; + loadReplyMessage: (messageId: string) => Promise; + agentNamesById: ReadonlyMap; + t: TFunction<"app">; +} + +function ReplyContextExpandable({ + ownerMessageId, + replyToId, + initialMessage, + ancestorIds, + testId, + env, +}: { + ownerMessageId: string; + replyToId: string; + initialMessage?: Message; + ancestorIds: Set; + testId?: string; + env: ReplyContextEnv; +}) { + const { replyContextCache, replyContextExpanded, replyContextLoading, replyContextErrors, setReplyExpanded, loadReplyMessage, agentNamesById, t } = env; + const cacheMessage = replyContextCache.get(replyToId) ?? initialMessage; + const rowKey = `${ownerMessageId}-${replyToId}`; + const isExpanded = Boolean(replyContextExpanded[rowKey]); + const isLoadingReply = Boolean(replyContextLoading[replyToId]); + const errorMessage = replyContextErrors[replyToId]; + const hasCycle = ancestorIds.has(replyToId); + + const handleToggle = async () => { + if (isExpanded) { + setReplyExpanded(rowKey, false); + return; + } + setReplyExpanded(rowKey, true); + if (!cacheMessage && !hasCycle) { + await loadReplyMessage(replyToId); + } + }; + + const nextAncestorIds = new Set(ancestorIds); + nextAncestorIds.add(replyToId); + + return ( +
+ + + {isExpanded && ( +
+ {errorMessage &&
{errorMessage}
} + {cacheMessage && ( + <> +
+ {participantLabel(cacheMessage.fromId, cacheMessage.fromType, agentNamesById, t)} + {formatTimestamp(cacheMessage.createdAt, t)} +
+
{cacheMessage.content}
+ {cacheMessage.metadata?.replyTo?.messageId && !nextAncestorIds.has(cacheMessage.metadata.replyTo.messageId) && ( + + )} + + )} +
+ )} +
+ ); +} + // ── Component ───────────────────────────────────────────────────────────── export function MailboxModal({ @@ -645,83 +748,20 @@ export function MailboxModal({ if (!isOpen) return null; - const ReplyContextExpandable = ({ - ownerMessageId, - replyToId, - initialMessage, - ancestorIds, - testId, - }: { - ownerMessageId: string; - replyToId: string; - initialMessage?: Message; - ancestorIds: Set; - testId?: string; - }) => { - const cacheMessage = replyContextCache.get(replyToId) ?? initialMessage; - const rowKey = `${ownerMessageId}-${replyToId}`; - const isExpanded = Boolean(replyContextExpanded[rowKey]); - const isLoadingReply = Boolean(replyContextLoading[replyToId]); - const errorMessage = replyContextErrors[replyToId]; - const hasCycle = ancestorIds.has(replyToId); - - const handleToggle = async () => { - if (isExpanded) { - setReplyExpanded(rowKey, false); - return; - } - setReplyExpanded(rowKey, true); - if (!cacheMessage && !hasCycle) { - await loadReplyMessage(replyToId); - } - }; - - const nextAncestorIds = new Set(ancestorIds); - nextAncestorIds.add(replyToId); - - return ( -
- - - {isExpanded && ( -
- {errorMessage &&
{errorMessage}
} - {cacheMessage && ( - <> -
- {participantLabel(cacheMessage.fromId, cacheMessage.fromType, agentNamesById, t)} - {formatTimestamp(cacheMessage.createdAt, t)} -
-
{cacheMessage.content}
- {cacheMessage.metadata?.replyTo?.messageId && !nextAncestorIds.has(cacheMessage.metadata.replyTo.messageId) && ( - - )} - - )} -
- )} -
- ); + /* + FNXC:Mailbox 2026-07-26-20:10: + The reply-context environment is assembled once per render and handed to the module-scope + ReplyContextExpandable below, which owns the recursive thread rendering. + */ + const replyContextEnv: ReplyContextEnv = { + replyContextCache, + replyContextExpanded, + replyContextLoading, + replyContextErrors, + setReplyExpanded, + loadReplyMessage, + agentNamesById, + t, }; // ── Render ──────────────────────────────────────────────────────────── @@ -906,6 +946,7 @@ export function MailboxModal({ initialMessage={replyToMessage} ancestorIds={new Set([msg.id])} testId={`mailbox-reply-context-${msg.id}`} + env={replyContextEnv} /> )} candidate.id === selectedMessage.metadata?.replyTo?.messageId)} ancestorIds={new Set([selectedMessage.id])} testId="mailbox-selected-reply-context" + env={replyContextEnv} /> )} { }); }); + /* + FNXC:Mailbox 2026-07-26-20:10: + An expanded reply-context row must survive unrelated mailbox re-renders with its DOM node intact. + ReplyContextExpandable used to be declared inside MailboxModal's render, making it a new element type + every render: opening the composer (or any parent state change) remounted the whole recursive thread, + discarding row identity, focus, and scroll position. Assert node identity, not just visible text — + a remount reproduces identical markup and would pass a text-only assertion. + */ + it("keeps an expanded reply-context row mounted across unrelated re-renders", async () => { + const grandparent: Message = { ...mockMessage, id: "msg-grandparent", content: "Original message" }; + const parent: Message = { + ...mockMessage, + id: "msg-parent", + fromId: "dashboard", + fromType: "user", + toId: "agent-001", + toType: "agent", + type: "user-to-agent", + content: "Second reply", + metadata: { replyTo: { messageId: "msg-grandparent" } }, + }; + const child: Message = { + ...mockMessage, + id: "msg-child", + fromId: "agent-001", + fromType: "agent", + toId: "dashboard", + toType: "user", + content: "Third reply", + metadata: { replyTo: { messageId: "msg-parent" } }, + }; + + mockFetchInbox.mockResolvedValue({ messages: [grandparent], total: 1, unreadCount: 1 }); + mockFetchConversation.mockResolvedValue([grandparent, parent, child]); + + render(); + + await waitFor(() => expect(screen.getByTestId("mailbox-item-msg-grandparent")).toBeDefined()); + fireEvent.click(screen.getByTestId("mailbox-item-msg-grandparent")); + + const parentContext = await screen.findByTestId("mailbox-reply-context-msg-parent"); + fireEvent.click(parentContext); + await waitFor(() => expect(parentContext.getAttribute("aria-expanded")).toBe("true")); + + // Unrelated parent state change: expanding a different row re-renders MailboxModal. + fireEvent.click(screen.getByTestId("mailbox-reply-context-msg-child")); + await screen.findByTestId("mailbox-reply-expanded-msg-parent"); + + expect(screen.getByTestId("mailbox-reply-context-msg-parent")).toBe(parentContext); + expect(parentContext.getAttribute("aria-expanded")).toBe("true"); + }); + it("renders nested reply context rows for multi-level thread metadata", async () => { const grandparent: Message = { ...mockMessage, id: "msg-grandparent", content: "Original message" }; const parent: Message = {