fix(dashboard): stop Mailbox reply-context rows collapsing on unrelated re-renders
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) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/fix-mailbox-reply-context-remount.md
Normal file
7
.changeset/fix-mailbox-reply-context-remount.md
Normal file
@@ -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.
|
||||
@@ -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<string, Message>;
|
||||
replyContextExpanded: Record<string, boolean>;
|
||||
replyContextLoading: Record<string, boolean>;
|
||||
replyContextErrors: Record<string, string>;
|
||||
setReplyExpanded: (key: string, isExpanded: boolean) => void;
|
||||
loadReplyMessage: (messageId: string) => Promise<Message | null>;
|
||||
agentNamesById: ReadonlyMap<string, string>;
|
||||
t: TFunction<"app">;
|
||||
}
|
||||
|
||||
function ReplyContextExpandable({
|
||||
ownerMessageId,
|
||||
replyToId,
|
||||
initialMessage,
|
||||
ancestorIds,
|
||||
testId,
|
||||
env,
|
||||
}: {
|
||||
ownerMessageId: string;
|
||||
replyToId: string;
|
||||
initialMessage?: Message;
|
||||
ancestorIds: Set<string>;
|
||||
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 (
|
||||
<div className="mailbox-reply-context-wrapper">
|
||||
<button
|
||||
type="button"
|
||||
className="mailbox-reply-context"
|
||||
onClick={() => {
|
||||
void handleToggle();
|
||||
}}
|
||||
aria-expanded={isExpanded}
|
||||
data-testid={testId}
|
||||
>
|
||||
<span className="mailbox-reply-context__chevron" aria-hidden="true">
|
||||
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
|
||||
</span>
|
||||
<span>
|
||||
↪ {t("mailbox.replyingTo", "Replying to {{preview}}", { preview: cacheMessage ? messagePreview(cacheMessage.content, 60) : `message ${replyToId}` })}
|
||||
</span>
|
||||
{isLoadingReply && <Loader2 size={14} className="spin" />}
|
||||
</button>
|
||||
|
||||
{isExpanded && (
|
||||
<div className="mailbox-reply-context__nested" data-testid={`mailbox-reply-expanded-${replyToId}`}>
|
||||
{errorMessage && <div className="mailbox-reply-context__error">{errorMessage}</div>}
|
||||
{cacheMessage && (
|
||||
<>
|
||||
<div className="mailbox-conversation-msg-header">
|
||||
<span>{participantLabel(cacheMessage.fromId, cacheMessage.fromType, agentNamesById, t)}</span>
|
||||
<span className="mailbox-message-time">{formatTimestamp(cacheMessage.createdAt, t)}</span>
|
||||
</div>
|
||||
<div className="mailbox-conversation-msg-body">{cacheMessage.content}</div>
|
||||
{cacheMessage.metadata?.replyTo?.messageId && !nextAncestorIds.has(cacheMessage.metadata.replyTo.messageId) && (
|
||||
<ReplyContextExpandable
|
||||
ownerMessageId={cacheMessage.id}
|
||||
replyToId={cacheMessage.metadata.replyTo.messageId}
|
||||
ancestorIds={nextAncestorIds}
|
||||
env={env}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ── 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<string>;
|
||||
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 (
|
||||
<div className="mailbox-reply-context-wrapper">
|
||||
<button
|
||||
type="button"
|
||||
className="mailbox-reply-context"
|
||||
onClick={() => {
|
||||
void handleToggle();
|
||||
}}
|
||||
aria-expanded={isExpanded}
|
||||
data-testid={testId}
|
||||
>
|
||||
<span className="mailbox-reply-context__chevron" aria-hidden="true">
|
||||
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
|
||||
</span>
|
||||
<span>
|
||||
↪ {t("mailbox.replyingTo", "Replying to {{preview}}", { preview: cacheMessage ? messagePreview(cacheMessage.content, 60) : `message ${replyToId}` })}
|
||||
</span>
|
||||
{isLoadingReply && <Loader2 size={14} className="spin" />}
|
||||
</button>
|
||||
|
||||
{isExpanded && (
|
||||
<div className="mailbox-reply-context__nested" data-testid={`mailbox-reply-expanded-${replyToId}`}>
|
||||
{errorMessage && <div className="mailbox-reply-context__error">{errorMessage}</div>}
|
||||
{cacheMessage && (
|
||||
<>
|
||||
<div className="mailbox-conversation-msg-header">
|
||||
<span>{participantLabel(cacheMessage.fromId, cacheMessage.fromType, agentNamesById, t)}</span>
|
||||
<span className="mailbox-message-time">{formatTimestamp(cacheMessage.createdAt, t)}</span>
|
||||
</div>
|
||||
<div className="mailbox-conversation-msg-body">{cacheMessage.content}</div>
|
||||
{cacheMessage.metadata?.replyTo?.messageId && !nextAncestorIds.has(cacheMessage.metadata.replyTo.messageId) && (
|
||||
<ReplyContextExpandable
|
||||
ownerMessageId={cacheMessage.id}
|
||||
replyToId={cacheMessage.metadata.replyTo.messageId}
|
||||
ancestorIds={nextAncestorIds}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
/*
|
||||
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}
|
||||
/>
|
||||
)}
|
||||
<MailboxMessageContent
|
||||
@@ -944,6 +985,7 @@ export function MailboxModal({
|
||||
initialMessage={threadMessages.find((candidate) => candidate.id === selectedMessage.metadata?.replyTo?.messageId)}
|
||||
ancestorIds={new Set([selectedMessage.id])}
|
||||
testId="mailbox-selected-reply-context"
|
||||
env={replyContextEnv}
|
||||
/>
|
||||
)}
|
||||
<MailboxMessageContent
|
||||
|
||||
@@ -775,6 +775,58 @@ describe("MailboxModal", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
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(<MailboxModal {...defaultProps} />);
|
||||
|
||||
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 = {
|
||||
|
||||
Reference in New Issue
Block a user