fix(mcp-apps): fix display mode height handling

[khandrew1]

Summary

• Preserve iframe height across display mode transitions (inline → PIP → fullscreen) by storing the last inline height in a ref
• Clamp PIP mode height to min(40vh, 600px) so tall apps don't overflow the viewport in the floating overlay

🤖 Generated with Claude Code

[chelojimenez]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

Walkthrough

The MCP apps renderer component has been enhanced to dynamically track iframe heights across different display modes. A new constant defines the maximum height for picture-in-picture mode, while a ref captures the height of inline iframes during resize events. The iframe height calculation now selects the appropriate value based on the active display mode: fullscreen uses 100%, picture-in-picture uses the defined maximum, and inline uses the tracked height. The transition timing has been updated to apply conditionally based on display mode transitions.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/mcp-apps-renderer.tsx (2)

396-407: Transition suppression via stale-ref comparison is correct but brittle.

The trick relies on effectiveDisplayModeRef.current lagging one render behind the prop (updated in the useEffect at Line 727). During a mode-switch render the two diverge, suppressing the CSS transition and avoiding a visible snap — precisely what's intended.

The coupling is implicit, though: anyone removing or reordering the ref sync in that useEffect would silently break the transition guard. A brief inline comment explaining the invariant would future-proof this nicely.

💡 Clarifying comment

     transition:
-      isFullscreen || effectiveDisplayModeRef.current !== effectiveDisplayMode
+      // Suppress height transition during the render that changes display mode.
+      // effectiveDisplayModeRef still holds the *previous* mode here (synced
+      // post-render in useEffect), so the mismatch signals a mode switch.
+      isFullscreen || effectiveDisplayModeRef.current !== effectiveDisplayMode
         ? undefined
         : "height 300ms ease-out",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/mcp-apps-renderer.tsx`
around lines 396 - 407, Add a short inline comment near the stale-ref comparison
that explains the invariant: effectiveDisplayModeRef.current is intentionally
allowed to lag one render behind the prop (it is synced in the separate
useEffect that updates effectiveDisplayModeRef) so the comparison can detect a
mode-switch render and suppress the CSS transition; mention that moving or
removing the ref-syncing useEffect will break this guard and that the ref must
remain updated only in that effect to preserve the behavior. Reference
effectiveDisplayModeRef.current and the useEffect that updates it in the comment
so future maintainers understand the coupling.

---

385-385: Sensible default; consider naming the magic value.

"400px" appears to be the legacy default height. A named constant (e.g., DEFAULT_INLINE_HEIGHT) would make the relationship self-documenting and easier to update in one place.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/mcp-apps-renderer.tsx`
at line 385, Introduce a named constant for the magic string and replace the
inline literal: define const DEFAULT_INLINE_HEIGHT = "400px" near the top of the
component file and update the useRef initializer to use DEFAULT_INLINE_HEIGHT
(i.e., const lastInlineHeightRef = useRef<string>(DEFAULT_INLINE_HEIGHT));
ensure any other occurrences of "400px" that represent the same default height
are replaced to keep the value centralized and self-documenting.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@mcpjam-inspector/client/src/components/chat-v2/thread/mcp-apps/mcp-apps-renderer.tsx`:
- Around line 396-407: Add a short inline comment near the stale-ref comparison
that explains the invariant: effectiveDisplayModeRef.current is intentionally
allowed to lag one render behind the prop (it is synced in the separate
useEffect that updates effectiveDisplayModeRef) so the comparison can detect a
mode-switch render and suppress the CSS transition; mention that moving or
removing the ref-syncing useEffect will break this guard and that the ref must
remain updated only in that effect to preserve the behavior. Reference
effectiveDisplayModeRef.current and the useEffect that updates it in the comment
so future maintainers understand the coupling.
- Line 385: Introduce a named constant for the magic string and replace the
inline literal: define const DEFAULT_INLINE_HEIGHT = "400px" near the top of the
component file and update the useRef initializer to use DEFAULT_INLINE_HEIGHT
(i.e., const lastInlineHeightRef = useRef<string>(DEFAULT_INLINE_HEIGHT));
ensure any other occurrences of "400px" that represent the same default height
are replaced to keep the value centralized and self-documenting.