feat(site): harden Agents embed frame communication and add theme sync#23574
feat(site): harden Agents embed frame communication and add theme sync#23574
Conversation
5b19e4b to
28ed743
Compare
| // When embedded, scroll to bottom once messages load and | ||
| // notify the parent frame that the chat is ready. Comparing | ||
| // against the last signaled agentId re-fires the notification | ||
| // when the user switches agents without a full remount. |
There was a problem hiding this comment.
Can we do this in the AgentEmbedPage itself rather than here? Conflating them is kinda unfortunate.
There was a problem hiding this comment.
Oh right, I've added a callback that we can just listen to in AgentEmbedPage (onChatReady)
There was a problem hiding this comment.
So I can remove the whole embedding logic from here but then if I want to scroll to the bottom I'd have to do this:
document.querySelector<HTMLElement>(
'[data-testid="scroll-container"]',
);instead of using refs
There was a problem hiding this comment.
Pass a scroll ref instead and seems to work better now!
28ed743 to
477fca0
Compare
DanielleMaywood
left a comment
There was a problem hiding this comment.
Code review by Pluto 🐕 and team — 3 verified findings (all P3 nits).
No blocking issues. The core implementation is solid: message validation is defensive, effect cleanup is correct for the primary flow, layout/effect timing is well-coordinated, and the useBlocker + postMessage forwarding pattern works cleanly.
🤖 This comment was written by Coder Agent on behalf of Danielle Maywood 🤖
Move the scroll-to-bottom message listener from AgentEmbedPage into AgentDetail so it uses the existing scrollContainerRef instead of a brittle querySelector lookup. Track the last signaled agentId in the chat-ready effect so the notification re-fires when agents change without a full remount. Include location hash in the navigation blocker payload.
Let the parent frame control the embed's color scheme with coder:set-theme messages. On mount the embed applies prefers-color-scheme as an initial value, then listens for explicit theme messages. ThemeProvider skips its own class manipulation when the embed is active, preventing races. Also adds a no-op guard to applyEmbedTheme so duplicate messages skip DOM mutations.
…tDetail Move all embed-specific logic out of AgentDetail into AgentEmbedPage. Chat-ready notification uses the onChatReady outlet context callback. Scroll-to-bottom handling moves to the parent frame message listener in AgentEmbedPage. AgentDetail no longer imports useEmbedContext.
- Remove useCallback from useBlocker (React Compiler handles memoization) - Add classList.remove in theme cleanup to match setup symmetry - Pass scrollContainerRef through outlet context so AgentEmbedPage uses the ref directly instead of querySelector for scroll-to-bottom
580daeb to
66817fa
Compare
Documentation CheckNew Documentation Needed
Automated review via Coder Tasks |
DanielleMaywood
left a comment
There was a problem hiding this comment.
I'm okay with this but wanna make sure Kyle is
AgentEmbedPageintoAgentDetailso it uses the existingscrollContainerRefinstead of a brittlequerySelectorlookup.coder:chat-readynotification re-fires when the user switches agents without a full remount.coder:navigate.coder:set-thememessage support so the parent frame (e.g. VS Code) can control the embed's color scheme. On mount,themequery is applied immediately to avoid a theme flash; explicit messages override it at any time. ThemeProvider skips its own class manipulation while the embed is active.