viewport -> containerDimensions #1148
Conversation
dosubot
bot
added
the
size:M
WalkthroughThis change refactors the device dimension handling in the MCP apps renderer component. The implementation transitions from viewport-based derivation to container dimensions, removing direct width and height computations. Refs are introduced to maintain state for playground activity and device type across closures without recalculation. Mobile detection logic during display mode requests now relies on these refs and playground state rather than viewport width. Terminology and payload structures are updated throughout to align with the containerDimensions approach for managing maximum height and width constraints. 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
🤖 Fix all issues with AI agents
In @client/src/components/chat-v2/thread/mcp-apps-renderer.tsx:
- Around line 731-735: The current logic sets isMobile to true when not in
playground, forcing PiP -> fullscreen in production; instead, update the
fallback to use the app's device/platform detection (e.g., call getDeviceType()
or use the existing platform variable) so production behavior matches other
renderers: replace the hardcoded true in the ternary with a runtime device check
(e.g., treat mobile/tablet as mobile) and keep isPlaygroundActiveRef.current
handling unchanged; reference isPlaygroundActiveRef, playgroundDeviceTypeRef and
the isMobile variable when making the change.
📜 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
Additional Context for PR Review
Project Context
The MCPJam/inspector project is related to the Model Context Protocol (MCP), an open standard that enables developers to build secure, two-way connections between their data sources and AI-powered tools. This provides context for understanding why the component in question (mcp-apps-renderer) is managing display dimensions.
Technical Terminology
The PR's refactoring from "viewport" to "containerDimensions" aligns with modern web design practices:
-
For years, responsive design was constrained by the viewport where a component had to adapt based on the overall screen size, not the space it was actually allocated. CSS Container Queries, now with broad browser support, solve this problem and enable a truly modular, component-driven approach.
-
Container queries allow components to respond to the size of their containing element rather than the viewport, offering a more granular and dynamic approach to styling.
Performance Implications
The PR's use of refs to avoid recalculation is a performance optimization pattern:
- Viewport units simplify the creation of responsive designs by reducing reliance on complex media queries or JavaScript-based resizing, and by tying dimensions to the viewport, developers can reduce the need for extensive CSS or JavaScript to handle responsiveness.
Review Focus Areas
Based on this context, key aspects to validate during PR review:
- State Synchronization: Verify that refs (isPlaygroundActive, playgroundDeviceType) are properly synchronized and don't introduce stale closure bugs
- Mobile Detection Logic: Confirm that the updated mobile detection properly handles both playground and non-playground scenarios
- Performance: Ensure the ref-based approach actually improves performance by avoiding unnecessary recalculations
- API Contract: Confirm that replacing the hostContext payload shape maintains compatibility with dependent components
[::web_search::]
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx (5)
220-220: LGTM—terminology update aligns with the containerDimensions refactor.
275-276: Behavioral change noted: maxHeight no longer clamps resize.The comment indicates resize clamping has been removed, though the value is still sent to guest apps. Confirm this doesn't impact existing MCP Apps that may have relied on clamping behavior.
319-320: Well-structured ref pattern for cross-closure state.The refs prevent stale closures in bridge handlers and avoid unnecessary recalculations, consistent with the performance optimization goals.
559-560: Ref synchronization properly configured.The useEffect correctly keeps refs in sync with their source values, ensuring runtime access in callbacks sees current state.
Also applies to: 571-572
509-512: containerDimensions correctly implements SEP-1865 protocol alignment.The
viewportproperty has been replaced withcontainerDimensionsas part of the SEP-1865 protocol specification. No code in this repository references the old property, and the implementation is sound. External MCP Apps must be compatible with this protocol version to function properly.
| // Use device type for mobile detection (defaults to mobile-like behavior when not in playground) | ||
| const isMobile = isPlaygroundActiveRef.current | ||
| ? playgroundDeviceTypeRef.current === "mobile" || | ||
| playgroundDeviceTypeRef.current === "tablet" | ||
| : true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for viewport or mobile detection patterns in the file
cd client/src/components/chat-v2/thread && cat -n mcp-apps-renderer.tsx | sed -n '700,770p'Repository: MCPJam/inspector
Length of output: 2978
🏁 Script executed:
#!/bin/bash
# Search for previous viewport/mobile detection in the entire codebase
rg -nP --type=ts --type=tsx 'viewportWidth|viewport.*width|isMobile' --max-count=20Repository: MCPJam/inspector
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for mobile detection patterns without file type restrictions
rg -n 'isMobile|viewport.*width|device.*type' client/src/components/chat-v2/thread/mcp-apps-renderer.tsx -A 3 -B 3Repository: MCPJam/inspector
Length of output: 2221
🏁 Script executed:
#!/bin/bash
# Search for previous implementations or related device detection
rg -n 'playgroundDeviceType|isPlaygroundActive' client/src/components/chat-v2/thread/ -A 2 -B 2Repository: MCPJam/inspector
Length of output: 14943
🏁 Script executed:
#!/bin/bash
# Look for viewport-based mobile detection anywhere in the codebase
rg -n 'window\.innerWidth|innerWidth.*mobile|viewport.*mobile' client/ --max-count=15Repository: MCPJam/inspector
Length of output: 460
🏁 Script executed:
#!/bin/bash
# Check git history for this specific section
git log -p --all -S 'isMobile = isPlaygroundActiveRef' -- client/src/components/chat-v2/thread/mcp-apps-renderer.tsx 2>/dev/null | head -100Repository: MCPJam/inspector
Length of output: 42
Clarify intent of mobile-default behavior in production.
Line 735 defaults isMobile to true outside the playground, converting all PiP requests to fullscreen in production. While the comment indicates this is intentional ("defaults to mobile-like behavior when not in playground"), this differs from viewport-based detection used elsewhere (e.g., chatgpt-app-renderer.tsx uses getDeviceType(), and the platform variable returns "web" in production). Confirm whether the hardcoded true is the desired behavior or if desktop device detection should be used instead.
🤖 Prompt for AI Agents
In @client/src/components/chat-v2/thread/mcp-apps-renderer.tsx around lines 731
- 735, The current logic sets isMobile to true when not in playground, forcing
PiP -> fullscreen in production; instead, update the fallback to use the app's
device/platform detection (e.g., call getDeviceType() or use the existing
platform variable) so production behavior matches other renderers: replace the
hardcoded true in the ternary with a runtime device check (e.g., treat
mobile/tablet as mobile) and keep isPlaygroundActiveRef.current handling
unchanged; reference isPlaygroundActiveRef, playgroundDeviceTypeRef and the
isMobile variable when making the change.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const isMobile = isPlaygroundActiveRef.current | ||
| ? playgroundDeviceTypeRef.current === "mobile" || | ||
| playgroundDeviceTypeRef.current === "tablet" | ||
| : true; |
There was a problem hiding this comment.
Custom viewport mobile detection ignores actual width
Medium Severity
The mobile detection logic for onrequestdisplaymode was changed from checking viewport width (viewportWidthRef.current < 768) to checking device type names. However, the "custom" device type case is not handled - when a user configures a custom viewport with a narrow width (e.g., 400px), it would previously have pip mode automatically converted to fullscreen (treating it as mobile), but now it won't since "custom" is not included in the device type check. This could cause pip mode to be used on inappropriately narrow custom viewports.
Update viewport type. Context PR