-
Notifications
You must be signed in to change notification settings - Fork 168
full screen styling for MCP apps #1132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
khandrew1
commented
Dec 29, 2025
- Adds the same styling for fullscreen/pip for ChatGPT Apps to MCP Apps
WalkthroughThe change refactors display-mode handling in the MCP apps renderer by introducing 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (1)
1-1: Run Prettier to resolve formatting issues.The CI pipeline flagged code style violations. Execute
npm run prettier-fixas noted in the project's development practices.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/components/chat-v2/thread/mcp-apps-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/chat-v2/thread/mcp-apps-renderer.tsx
🪛 GitHub Actions: Prettier and Build Check
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
🔍 Remote MCP
Summary of Relevant Context for PR Review
Based on my search, here is additional context relevant to reviewing PR #1132:
Project Context
MCPJam Inspector provides an Apps Builder, a local emulator to quickly view and iterate on MCP apps and ChatGPT apps widgets. The PR aligns with the platform's dual-app support philosophy—standardizing styling across both app types.
MCP Apps and Styling Standards
The MCP Apps Extension (SEP-1865) standardizes support for interactive user interfaces in the Model Context Protocol, building on proven work from MCP-UI and OpenAI Apps SDK, and introduces a standardized pattern for declaring UI resources and enabling communication between embedded interfaces and the host application.
For display modes, the requestDisplayMode API supports fullscreen mode, with the note that on mobile, PiP may be coerced to fullscreen.
Key Components Referenced in Changes
The widget emulator supports custom UI components and interactive visualizations, and the Apps Builder allows testing across different emulator device types (Desktop, Tablet, Mobile views) including testing for locale change, CSP permissions, light/dark mode, hover & touch, and safe area insets.
Development Practices
Code style is maintained using Prettier for formatting, with contributors expected to run npm run prettier-fix before committing to ensure consistent codebase style.
🔇 Additional comments (6)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (6)
890-896: Well-crafted display mode predicates.The distinction between
isMobilePlaygroundModeandisContainedFullscreenModeelegantly captures the viewport containment semantics. Clean derivation from existing store state.
897-917: Clean conditional class composition.The IIFE elegantly encapsulates the branching logic, yielding readable code without sprawling ternaries. The array-join pattern for multi-line classes at lines 909–913 is idiomatic and maintainable.
921-936: Touch-friendly close control with proper exit handling.The conditional rendering logic is precise, and the inline comment clarifying
onExitFullscreendelegation is appreciated. The 32×32px touch target suits mobile ergonomics.
938-957: Purposeful header bar for desktop fullscreen.The empty
<div />spacer paired withjustify-betweencenters the tool name elegantly. The defensivepipWidgetIdcheck before invokingonExitPipprevents spurious callbacks.
959-971: Distinct PiP control for desktop contexts.The smaller icon and bordered styling differentiate this from the mobile variant. Both
aria-labelandtitleensure accessibility and discoverability.
981-985: Adaptive iframe styling.The conditional class composition correctly applies
flex-1for fullscreen expansion while preserving border aesthetics in inline mode. The template literal keeps the logic concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (1)
980-987: Iframe styling adapts correctly to display mode.The conditional className and height logic properly remove decorative borders and enable fill behavior in fullscreen while maintaining contained dimensions inline. Note that
flex-1withheight: "100%"is slightly redundant but harmless.Optional: simplify fullscreen height
In fullscreen mode,
flex-1already instructs the iframe to fill available space, so the explicitheight: "100%"could be omitted:style={{ - height: isFullscreen ? "100%" : "400px", + height: isFullscreen ? undefined : "400px", }}This is a minor refinement and the current implementation is functional.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/components/chat-v2/thread/mcp-apps-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/chat-v2/thread/mcp-apps-renderer.tsx
🔍 Remote MCP Context7
Resolved library for deeper context
- Selected library ID: /modelcontextprotocol/inspector
- Why chosen: exact name match to the MCPJam Inspector repository, high source reputation and substantial code snippets (most relevant authoritative source for PR #1132 review).
Concise relevant facts for reviewing PR #1132
- Repository: MCP Inspector (inspector) — React-based web UI + Node proxy; this PR targets the UI renderer for MCP apps, so repository-level behavior and styling conventions are relevant.
- Library metadata: 85 code snippets, Source Reputation: High, Benchmark Score: 79.3 — indicates ample examples and an authoritative source to cross-check UI patterns and existing fullscreen/PiP implementations.
🔇 Additional comments (5)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (5)
890-895: Clean abstraction for device-aware display modes.The introduction of
isMobilePlaygroundModeandisContainedFullscreenModeclarifies the conditional rendering logic below and properly distinguishes mobile-only behavior from mobile+tablet containment scenarios.
896-916: Well-structured container class computation.The refactored IIFE cleanly maps display modes to appropriate positioning and styling. The z-index layering (z-40 for fixed overlays, z-10 for contained modes) ensures correct stacking behavior across contexts.
920-935: Unified close button handles contained modes correctly.The conditional rendering for contained fullscreen and mobile PiP is sound, and the onClick handler appropriately dispatches to either PiP or fullscreen exit flows. The inline comment clarifies the implicit fullscreen exit behavior.
958-970: PiP close control for non-mobile contexts is correctly implemented.The conditional rendering complements the unified close button (line 921) by handling desktop/tablet PiP scenarios. The onClick logic is sound, and accessibility attributes are appropriately provided.
937-956: Reconsider PiP check in fullscreen exit handler.Line 946–948 checks
pipWidgetId === toolCallIdwithin a fullscreen-only rendering branch (isFullscreen && !isContainedFullscreenMode). SinceeffectiveDisplayModeenforces mutual exclusivity between fullscreen and PiP states,pipWidgetIdshould never equaltoolCallIdin this branch. Either this check is defensive code for handling parent state inconsistencies, or it's dead code. If intentional, add a comment explaining the edge case; otherwise, remove it.