Fullscreen Chat #1103
Fullscreen Chat #1103
Conversation
fullscreen-caht.mp4 |
dosubot
bot
added
the
size:L
WalkthroughAdds a new FullscreenChatOverlay component and exports it. Thread gains props (enableFullscreenChatOverlay, fullscreenChatPlaceholder, fullscreenChatDisabled), state for overlay open/input, and renders FullscreenChatOverlay when enabled. A fullscreenWidgetId prop is threaded through MessageView, PartSwitch, ToolPart, ChatGPTAppRenderer, and MCPAppsRenderer; those renderers compute an effective display mode using the widget IDs. PlaygroundMain now renders the overlay, adjusts submit flow to open it when appropriate, and updates container positioning to host the fullscreen overlay. Minor formatting changes in unrelated files. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
client/src/components/chat-v2/thread/parts/tool-part.tsx (1)
130-139: Exit target indirection warrants clarification.The computed
exitPipTargetandexitFullscreenTargetallow this widget to exit another widget's mode when provided. This cross-widget coordination is intentional for unified display state management, yet a brief inline comment would illuminate the intent for future maintainers.const handleDisplayModeChange = (mode: DisplayMode) => { if (toolCallId) { + // Use provided widget IDs to coordinate mode transitions across widgets const exitPipTarget = pipWidgetId ?? toolCallId; const exitFullscreenTarget = fullscreenWidgetId ?? toolCallId;client/src/components/chat-v2/fullscreen-chat-overlay.tsx (2)
189-190: RedundantreadOnlyalongsidedisabled.When
disabledis true, the textarea already prevents user input. ThereadOnlyattribute is superfluous here.disabled={disabled} - readOnly={disabled}
124-126: Consider addingrole="log"for accessibility.Chat message containers benefit from
role="log"with implicitaria-live="polite", enabling screen readers to announce new messages appropriately.- <div className="mb-4 rounded-3xl border border-border/40 bg-background/95 shadow-2xl backdrop-blur-xl"> - <div className="max-h-[45vh] overflow-y-auto px-4 py-3 space-y-3"> + <div className="mb-4 rounded-3xl border border-border/40 bg-background/95 shadow-2xl backdrop-blur-xl"> + <div + role="log" + aria-label="Chat messages" + className="max-h-[45vh] overflow-y-auto px-4 py-3 space-y-3" + >
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/src/components/ChatTabV2.tsx(1 hunks)client/src/components/chat-v2/fullscreen-chat-overlay.tsx(1 hunks)client/src/components/chat-v2/thread.tsx(6 hunks)client/src/components/chat-v2/thread/chatgpt-app-renderer.tsx(15 hunks)client/src/components/chat-v2/thread/mcp-apps-renderer.tsx(14 hunks)client/src/components/chat-v2/thread/message-view.tsx(4 hunks)client/src/components/chat-v2/thread/part-switch.tsx(6 hunks)client/src/components/chat-v2/thread/parts/tool-part.tsx(3 hunks)client/src/components/ui-playground/PlaygroundMain.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Browser
console.*methods are acceptable for client-side debugging in client code
Files:
client/src/components/ChatTabV2.tsxclient/src/components/chat-v2/thread/message-view.tsxclient/src/components/chat-v2/thread/part-switch.tsxclient/src/components/chat-v2/fullscreen-chat-overlay.tsxclient/src/components/chat-v2/thread/mcp-apps-renderer.tsxclient/src/components/chat-v2/thread/parts/tool-part.tsxclient/src/components/chat-v2/thread/chatgpt-app-renderer.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/ui-playground/PlaygroundMain.tsx
🧬 Code graph analysis (3)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (2)
client/src/components/ui/textarea-autosize.tsx (1)
TextareaAutosize(22-22)client/src/components/ui/button.tsx (1)
Button(59-59)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (1)
client/src/stores/ui-playground-store.ts (1)
DisplayMode(30-30)
client/src/components/chat-v2/thread.tsx (2)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (1)
FullscreenChatOverlay(218-269)server/routes/apps/widget-runtime.ts (1)
sendFollowUpMessage(413-419)
🔍 Remote MCP
Based on my research, I've gathered relevant context about best practices and considerations for reviewing this fullscreen chat overlay PR. Here's the summary:
Additional Context for PR Review
React Component Composition & Prop Forwarding
Components that forward props to children should use spread syntax carefully, as excessive use can indicate that components should be split or that children should be passed as JSX instead. The PR's pattern of forwarding fullscreenWidgetId through the component hierarchy (ChatTabV2 → Thread → MessageView → PartSwitch → child renderers) follows a reasonable composition approach given the need to thread this specific state through multiple layers.
Chat UI Accessibility Considerations
This is particularly important for fullscreen overlays. Each element of a chat UI should be properly marked up, and assistive technology should be able to identify the role, name, value, and state of an element. The PR should be reviewed for:
- Keyboard focus management: focus must not be obscured, and focus containment is critical within modals and chat windows
- Using
role="log"for accessible chat windows, which uses aria-polite to queue screen reader messages - Support for assistive technologies and keyboard navigation to help users with cognitive disabilities
Fullscreen Overlay Pattern Best Practices
Fullscreen overlays should be lightweight and performant, using React portals to render anywhere needed. The auto-scroll and message handling in the new FullscreenChatOverlay component align with chat overlay design that uses smart positioning while maintaining user access without disrupting the main interface.
Key Review Focus Areas
- State Management: The PR introduces
isFullscreenChatOpenandfullscreenChatInputstate, pluseffectiveDisplayModecomputed logic in renderers—verify these don't create stale state issues - Display Mode Resolution: The
effectiveDisplayModepattern in ChatGPTAppRenderer and MCPAppsRenderer maps controlled vs. internal display modes—confirm the logic correctly prioritizes fullscreen/pip states - Effect Dependencies: Multiple useEffect dependency updates needed to reference
effectiveDisplayModeinstead ofdisplayMode—ensure all dependent effects are captured - UI Visibility Logic: FullscreenChatOverlay visibility is suppressed while the input is active; verify this toggle behavior doesn't break compose/send flows
[::web_search::]
🔇 Additional comments (25)
client/src/components/chat-v2/thread/part-switch.tsx (1)
43-43: Prop threading implemented consistently.The
fullscreenWidgetIdmirrors the existingpipWidgetIdpattern—both in type signature and propagation. Clean, symmetrical addition.Also applies to: 58-58
client/src/components/chat-v2/thread/parts/tool-part.tsx (1)
36-37: Optional props are appropriately typed.Both
pipWidgetIdandfullscreenWidgetIdare optional, preserving backward compatibility forToolPartusages that don't participate in coordinated display mode management.Also applies to: 47-48
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (2)
109-120: Message filtering and auto-scroll logic are well-structured.The
visibleMessagesmemo correctly excludes widget-state messages, and the scroll effect appropriately triggers on relevant state changes.
241-268: Overlay implementation is well-crafted.The pointer-events layering, safe-area inset handling, and auto-open-on-submit behavior form a cohesive fullscreen chat experience.
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (3)
183-189: TheeffectiveDisplayModelogic elegantly resolves widget-specific mode.The computation correctly ensures that only the widget matching
fullscreenWidgetIdorpipWidgetIdrenders in that mode; others revert to inline. This prevents multiple widgets from simultaneously claiming fullscreen/pip status.
354-373: Debug store synchronization correctly trackseffectiveDisplayMode.Both widget debug info initialization and globals update effects properly depend on
effectiveDisplayMode, ensuring accurate state reflection.Also applies to: 376-396
855-856: Rendering decisions correctly derive fromeffectiveDisplayMode.The
isPipandisFullscreenflags now reflect the resolved mode, ensuring container styling and close button behavior align with the widget's actual display state.client/src/components/chat-v2/thread/message-view.tsx (1)
21-21: Prop threading throughMessageViewis consistent.The
fullscreenWidgetIdflows uniformly to both user and assistant message renderings, maintaining the established pattern.Also applies to: 36-36
client/src/components/ChatTabV2.tsx (1)
514-516: LGTM! Clean prop forwarding to enable fullscreen overlay.The props integrate seamlessly with existing state (
placeholder,inputDisabled) and align with the PR's objective to enable fullscreen chat.client/src/components/ui-playground/PlaygroundMain.tsx (6)
72-72: LGTM!
257-257: LGTM!
450-452: Verify UX flow when opening overlay on submit.The overlay opens before sending the message. Since the overlay has separate input state (line 615), the submitted text won't appear in the overlay's input field—only in the message thread. If the intent is to show the conversation history in the overlay after submission, this is correct. Otherwise, consider opening the overlay after the message is sent.
521-529: LGTM! Overlay visibility logic is sound.The computed
showFullscreenChatOverlaycorrectly restricts the overlay to desktop fullscreen mode when the widget isn't taking over the container. The effect properly closes the overlay when conditions change.
533-533: LGTM! Overlay rendering and input suppression are correctly implemented.The
relativepositioning enables the overlay, and the conditional rendering ensures only one input is active at a time. The overlay shares the main input state, which is appropriate since both are mutually exclusive.Also applies to: 596-596, 610-629
1322-1322: LGTM!client/src/components/chat-v2/thread/chatgpt-app-renderer.tsx (5)
111-111: LGTM! Prop addition follows the established pattern.Also applies to: 449-449
483-499: LGTM! Display mode resolution is correctly implemented.The
effectiveDisplayModeensures that in controlled scenarios, only the widget matchingfullscreenWidgetIdorpipWidgetIdreceives the corresponding mode, while others default to inline. Dependencies are complete.
585-586: LGTM! Display mode usage correctly updated.All computations now reference
effectiveDisplayMode, ensuring consistent behavior with the controlled mode logic.Also applies to: 648-662
686-686: LGTM! Effects and callbacks correctly referenceeffectiveDisplayMode.All side effects and debug info now use the resolved mode, ensuring widgets receive accurate display state in both controlled and uncontrolled scenarios.
Also applies to: 701-701, 712-721, 730-730, 742-744, 749-753
1159-1159: LGTM! Remaining usages are correct, and z-index adjustment aligns with overlay layering.The z-index reduction from
z-50toz-40for fullscreen widgets ensures the fullscreen chat overlay (atz-50) appears above widgets.Also applies to: 1174-1174, 1255-1255, 1293-1293, 1380-1380
client/src/components/chat-v2/thread.tsx (5)
1-1: LGTM! New imports are correctly added.Also applies to: 9-9
22-24: LGTM! Props and defaults are sensible.The feature is opt-in (default
false) with reasonable fallback values.Also applies to: 38-40
46-47: LGTM! State management and computed values are well-structured.The effect correctly resets overlay state when conditions change, preventing stale UI.
Also applies to: 71-82
101-101: LGTM! Prop threading is correct.
113-132: LGTM! Overlay rendering and onSend handler are correctly implemented.The
onSendhandler appropriately opens the overlay on submission, ensuring users see the conversation update. Input is captured and cleared before sending.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Andrew Khadder <54488379+khandrew1@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (2)
97-142: Message list and auto-scroll behavior are well-implemented.The filtering logic correctly excludes internal state messages, and auto-scroll triggers appropriately on message changes and thinking state updates.
For smoother UX, consider adding scroll behavior:
useEffect(() => { if (!open) return; - bottomRef.current?.scrollIntoView({ block: "end" }); + bottomRef.current?.scrollIntoView({ behavior: "smooth", block: "end" }); }, [open, visibleMessages.length, isThinking]);
144-215: Composer handles input and submission correctly.The keyboard handling properly distinguishes Enter from Shift+Enter and respects IME composition state, which is crucial for international input support.
Minor note: Line 189 sets
readOnly={disabled}, which is redundant since thedisabledattribute already prevents input. Either is sufficient:placeholder={placeholder} disabled={disabled} - readOnly={disabled} minRows={1}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Browser
console.*methods are acceptable for client-side debugging in client code
Files:
client/src/components/chat-v2/fullscreen-chat-overlay.tsx
🧬 Code graph analysis (1)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (2)
client/src/components/ui/textarea-autosize.tsx (1)
TextareaAutosize(22-22)client/src/components/ui/button.tsx (1)
Button(59-59)
🔇 Additional comments (5)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (5)
1-8: Imports are clean and purposeful.All dependencies are utilized; the previously flagged unused import has been addressed.
28-45: Well-structured bubble component.Properly handles user/assistant distinction and preserves whitespace for multi-line messages.
47-66: Thinking indicator is functional and accessible.The staggered dot animation provides clear feedback during response generation.
68-95: Toggle button follows accessibility best practices.Proper button type, dynamic aria-label, and clear visual feedback enhance usability.
217-268: Overlay layout strategy is well-crafted.The pointer-events pattern and safe-area handling demonstrate thoughtful mobile-first design. The component composition is clean and maintainable.
Verify that the submission flow (lines 259-262) intentionally opens the overlay before dispatching the send action:
onSubmit={() => { onOpenChange(true); // Opens overlay first onSend(); // Then sends }}This ensures the message appears immediately in the fullscreen view. Confirm this ordering aligns with the expected UX flow in fullscreen mode.
| function getMessagePreviewText(message: UIMessage): string { | ||
| const parts = Array.isArray(message?.parts) ? message.parts : []; | ||
| const texts = parts | ||
| .map((part: any) => { | ||
| if (!part || typeof part !== "object") return ""; | ||
| if (part.type === "text" && typeof part.text === "string") | ||
| return part.text; | ||
| if ("text" in part && typeof part.text === "string") return part.text; | ||
| return ""; | ||
| }) | ||
| .filter(Boolean); | ||
|
|
||
| if (texts.length > 0) return texts.join("\n").trim(); | ||
| if (typeof (message as any)?.content === "string") | ||
| return ((message as any).content as string).trim(); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any with proper types for compile-time safety.
The function uses any for part iteration (line 13) and message casting (line 23), which sacrifices TypeScript's type checking. Consider type guards or specific assertions based on the UIMessage structure from @ai-sdk/react.
Example refactor using type guards:
function getMessagePreviewText(message: UIMessage): string {
const parts = Array.isArray(message?.parts) ? message.parts : [];
const texts = parts
- .map((part: any) => {
+ .map((part) => {
if (!part || typeof part !== "object") return "";
if (part.type === "text" && typeof part.text === "string")
return part.text;
if ("text" in part && typeof part.text === "string") return part.text;
return "";
})
.filter(Boolean);
if (texts.length > 0) return texts.join("\n").trim();
- if (typeof (message as any)?.content === "string")
- return ((message as any).content as string).trim();
+ if ("content" in message && typeof message.content === "string")
+ return message.content.trim();
return "";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getMessagePreviewText(message: UIMessage): string { | |
| const parts = Array.isArray(message?.parts) ? message.parts : []; | |
| const texts = parts | |
| .map((part: any) => { | |
| if (!part || typeof part !== "object") return ""; | |
| if (part.type === "text" && typeof part.text === "string") | |
| return part.text; | |
| if ("text" in part && typeof part.text === "string") return part.text; | |
| return ""; | |
| }) | |
| .filter(Boolean); | |
| if (texts.length > 0) return texts.join("\n").trim(); | |
| if (typeof (message as any)?.content === "string") | |
| return ((message as any).content as string).trim(); | |
| return ""; | |
| } | |
| function getMessagePreviewText(message: UIMessage): string { | |
| const parts = Array.isArray(message?.parts) ? message.parts : []; | |
| const texts = parts | |
| .map((part) => { | |
| if (!part || typeof part !== "object") return ""; | |
| if (part.type === "text" && typeof part.text === "string") | |
| return part.text; | |
| if ("text" in part && typeof part.text === "string") return part.text; | |
| return ""; | |
| }) | |
| .filter(Boolean); | |
| if (texts.length > 0) return texts.join("\n").trim(); | |
| if ("content" in message && typeof message.content === "string") | |
| return message.content.trim(); | |
| return ""; | |
| } |
🤖 Prompt for AI Agents
In client/src/components/chat-v2/fullscreen-chat-overlay.tsx around lines 10 to
26, the function getMessagePreviewText uses `any` for message parts and a cast
for message content which loses compile-time safety; replace `any` by importing
or declaring the correct part/message types (e.g., a UIMessagePart or similar
from @ai-sdk/react or a local union type for { type: "text"; text: string } |
other part shapes), add a type guard function like isTextPart(part): part is
TextPart to narrow parts when mapping, use Array.isArray(message?.parts) as
Part[] after narrowing, and check message.content with a safe type guard (e.g.,
typeof (message as unknown)?.content === "string") or by extending UIMessage
type so you can access content without casting; update the filter/map to use
those narrowed types so no `any` or unsafe casts remain.
…or into khandrew1/fullscreen-chat
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/src/components/ui-playground/PlaygroundMain.tsx (1)
525-533: Redundant condition:!isWidgetFullTakeoverwhendeviceType === "desktop"is already checked.Given
isWidgetFullTakeoveris only true for mobile/tablet (lines 517-523), the!isWidgetFullTakeovercheck is always true whendeviceType === "desktop". Removing it would clarify intent.🔎 Suggested simplification:
const showFullscreenChatOverlay = displayMode === "fullscreen" && isWidgetFullscreen && - deviceType === "desktop" && - !isWidgetFullTakeover; + deviceType === "desktop";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/components/chat-v2/thread/chatgpt-app-renderer.tsx(15 hunks)client/src/components/ui-playground/PlaygroundMain.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/components/chat-v2/thread/chatgpt-app-renderer.tsx
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Browser
console.*methods are acceptable for client-side debugging in client code
Files:
client/src/components/ui-playground/PlaygroundMain.tsx
🧬 Code graph analysis (1)
client/src/components/ui-playground/PlaygroundMain.tsx (1)
client/src/components/chat-v2/fullscreen-chat-overlay.tsx (1)
FullscreenChatOverlay(217-268)
🔍 Remote MCP
Let me try a web search to find relevant information about fullscreen chat overlay patterns and best practices:Based on my research, I've gathered relevant context about React patterns and practices that apply to this PR. Here's my summary:
Summary of Relevant Context for PR #1103 Review
Fullscreen Chat Overlay Pattern Considerations
React fullscreen overlays can be efficiently implemented using React portals, which render components anywhere in the DOM tree while maintaining performance. This is relevant to the new FullscreenChatOverlay component being introduced.
Prop Threading Architecture
The PR implements extensive prop threading of fullscreenWidgetId through multiple component layers (Thread → MessageView → PartSwitch → renderers). This approach aligns with React composition patterns, though deep prop passing through component trees is called "prop drilling," which can become problematic as applications grow.
Key consideration: The PR threads the same control identifier (fullscreenWidgetId and pipWidgetId) through 7+ component levels. If this pattern needs to scale further, the React Context API provides an alternative solution for sharing data across component trees without prop drilling.
Display Mode Resolution Pattern
The introduction of effectiveDisplayMode computed via useMemo in ChatGPTAppRenderer and MCPAppsRenderer is a sound pattern. The stable dependency principle suggests that component APIs should encapsulate things likely to change (like mode resolution logic) to protect consumers from volatility, which this approach achieves by centralizing mode determination.
State Management & Effects
The PR introduces several state variables (isFullscreenChatOpen, fullscreenChatInput) and corresponding effects to reset state when the overlay is disabled. This is consistent with React best practices for managing dependent state.
Composition & Subcomponent Organization
The FullscreenChatOverlay component encapsulates internal subcomponents (MessageBubble, ThinkingRow, MessageList, Composer). The Compound Components Pattern is a React design pattern useful for breaking down complex parent components into smaller, manageable child components, enabling developers to create sophisticated UI components that are readily customizable and extended while maintaining clear code structure.
🔇 Additional comments (5)
client/src/components/ui-playground/PlaygroundMain.tsx (5)
72-72: Import aligns with subsequent usage.
258-258: State declaration is well-placed alongside related UI state.
454-456: Overlay activation on submit is correctly gated.The conditions ensure the overlay opens only when appropriate—fullscreen mode with an active fullscreen widget.
537-537: Positioning context established for child elements.
1326-1326: Positioning context for the device frame is appropriate.
| onSend={() => { | ||
| sendMessage({ text: input }); | ||
| setInput(""); | ||
| setMcpPromptResults([]); | ||
| }} |
There was a problem hiding this comment.
Missing analytics tracking for messages sent via fullscreen overlay.
The main onSubmit handler (lines 457-464) includes posthog.capture("app_builder_send_message", ...), but this onSend callback bypasses that tracking. Messages sent through the overlay won't appear in analytics.
🔎 Suggested fix:
onSend={() => {
+ posthog.capture("app_builder_send_message", {
+ location: "app_builder_tab",
+ platform: detectPlatform(),
+ environment: detectEnvironment(),
+ model_id: selectedModel?.id ?? null,
+ model_name: selectedModel?.name ?? null,
+ model_provider: selectedModel?.provider ?? null,
+ });
sendMessage({ text: input });
setInput("");
setMcpPromptResults([]);
}}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In client/src/components/ui-playground/PlaygroundMain.tsx around lines 627 to
631, the onSend handler sends the message but skips analytics; add the same
posthog.capture("app_builder_send_message", ...) call used in the main onSubmit
(lines ~457-464) with the identical properties (e.g., workspace id, builder id,
message text, and any context flags) before or immediately after sendMessage so
messages sent via the fullscreen overlay are tracked; keep the existing input
reset and setMcpPromptResults([]) behavior.
Uh oh!
There was an error while loading. Please reload this page.