Open
Conversation
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
dosubot
bot
added
size:XL
Contributor
WalkthroughThis pull request introduces a new "learn more" feature consisting of three React components— 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsx (1)
5-16:⚠️ Potential issue | 🟡 MinorPlease migrate this test setup to repository mock presets.
This remains an ad-hoc inline mock setup; client tests are expected to use shared presets from
client/src/test/mocks/for consistency.As per coding guidelines:
mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets fromclient/src/test/mocks/includingmcpApiPresetsandstorePresetsin client tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsx` around lines 5 - 16, Replace the ad-hoc vi.mock block in nav-main.test.tsx that stubs SidebarGroup, SidebarGroupContent, SidebarMenu, SidebarMenuItem, SidebarMenuButton and useSidebar with the shared mock presets from client/src/test/mocks/; remove the inline vi.mock and instead import and apply the sidebar mock preset plus mcpApiPresets and storePresets (e.g., import { sidebarPresets, mcpApiPresets, storePresets } from 'client/src/test/mocks') and register or call them in the test setup/beforeEach so the test uses the centralized Sidebar mock and the mcpApiPresets and storePresets rather than the local mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsx`:
- Around line 78-170: The panel is visually modal but missing accessibility: add
programmatic modal semantics and focus management to LearnMoreExpandedPanel by
(1) adding role="dialog" and aria-modal="true" to the motion.div (key
"learn-more-panel") and ensuring the header/title is referenced via
aria-labelledby, (2) capture document.activeElement on mount and restore it in
the cleanup that runs when onClose is invoked, (3) set initial focus to the
close button (the button that renders the XIcon) on open (either via autoFocus
or by focusing it using panelRef.current.querySelector), (4) implement a simple
focus trap on the panelRef that intercepts Tab/Shift+Tab to cycle focus within
focusable elements inside the panel, and (5) add an Escape key listener to call
onClose; ensure background content is not reachable (e.g., toggle aria-hidden on
the app root or rely on aria-modal=true) and clean up all listeners on unmount.
---
Outside diff comments:
In `@mcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsx`:
- Around line 5-16: Replace the ad-hoc vi.mock block in nav-main.test.tsx that
stubs SidebarGroup, SidebarGroupContent, SidebarMenu, SidebarMenuItem,
SidebarMenuButton and useSidebar with the shared mock presets from
client/src/test/mocks/; remove the inline vi.mock and instead import and apply
the sidebar mock preset plus mcpApiPresets and storePresets (e.g., import {
sidebarPresets, mcpApiPresets, storePresets } from 'client/src/test/mocks') and
register or call them in the test setup/beforeEach so the test uses the
centralized Sidebar mock and the mcpApiPresets and storePresets rather than the
local mocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb6fd5c3-aba5-4bd1-8c0f-46a5c2da6193
⛔ Files ignored due to path filters (1)
mcpjam-inspector/client/public/tool-vid-march.mp4is excluded by!**/*.mp4
📒 Files selected for processing (9)
mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsxmcpjam-inspector/client/src/components/learn-more/LearnMoreHoverCard.tsxmcpjam-inspector/client/src/components/learn-more/LearnMoreModal.tsxmcpjam-inspector/client/src/components/mcp-sidebar.tsxmcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsxmcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsxmcpjam-inspector/client/src/components/sidebar/nav-main.tsxmcpjam-inspector/client/src/hooks/use-learn-more.tsmcpjam-inspector/client/src/lib/learn-more-content.ts
💤 Files with no reviewable changes (1)
- mcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsx
Comment on lines
+78
to
+170
| <motion.div | ||
| ref={panelRef} | ||
| key="learn-more-panel" | ||
| className="fixed z-50 bg-background rounded-lg border shadow-lg p-6 overflow-y-auto" | ||
| style={{ | ||
| top: "10vh", | ||
| left: "50%", | ||
| marginLeft: -(PANEL_WIDTH / 2), | ||
| width: PANEL_WIDTH, | ||
| maxWidth: "calc(100vw - 2rem)", | ||
| maxHeight: "80vh", | ||
| }} | ||
| initial={getInitialStyle()} | ||
| animate={{ | ||
| opacity: 1, | ||
| x: 0, | ||
| y: 0, | ||
| scale: 1, | ||
| transformOrigin: "top left", | ||
| }} | ||
| exit={{ | ||
| opacity: 0, | ||
| scale: 0.97, | ||
| transition: { duration: 0.15, ease: EASING } as any, | ||
| }} | ||
| transition={{ duration: 0.35, ease: EASING }} | ||
| > | ||
| {/* Close button */} | ||
| <button | ||
| onClick={onClose} | ||
| className="absolute top-4 right-4 rounded-xs opacity-70 transition-opacity hover:opacity-100 focus:outline-none" | ||
| > | ||
| <XIcon className="h-4 w-4" /> | ||
| <span className="sr-only">Close</span> | ||
| </button> | ||
|
|
||
| {/* Header */} | ||
| <div className="mb-4"> | ||
| <h2 className="text-lg font-semibold leading-none pb-1"> | ||
| {entry.title} | ||
| </h2> | ||
| <p className="text-muted-foreground text-sm"> | ||
| {entry.description} | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* Video */} | ||
| {entry.videoUrl ? ( | ||
| <div className="aspect-video w-full overflow-hidden rounded-md bg-muted mb-4"> | ||
| {entry.videoUrl.endsWith(".mp4") ? ( | ||
| <video | ||
| src={entry.videoUrl} | ||
| className="h-full w-full" | ||
| autoPlay | ||
| loop | ||
| muted | ||
| playsInline | ||
| preload="auto" | ||
| title={`${entry.title} video`} | ||
| /> | ||
| ) : ( | ||
| <iframe | ||
| src={entry.videoUrl} | ||
| className="h-full w-full" | ||
| allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" | ||
| allowFullScreen | ||
| title={`${entry.title} video`} | ||
| /> | ||
| )} | ||
| </div> | ||
| ) : ( | ||
| <div className="aspect-video w-full overflow-hidden rounded-md bg-muted flex items-center justify-center mb-4"> | ||
| <p className="text-muted-foreground text-sm"> | ||
| Video coming soon | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Docs link */} | ||
| <div className="flex justify-end"> | ||
| <Button variant="outline" size="sm" asChild> | ||
| <a | ||
| href={entry.docsUrl} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| Read the docs | ||
| <ExternalLink className="ml-1.5 h-3.5 w-3.5" /> | ||
| </a> | ||
| </Button> | ||
| </div> | ||
| </motion.div> | ||
| </> |
Contributor
There was a problem hiding this comment.
Expanded panel is visually modal but not programmatically modal.
The panel lacks modal semantics and focus containment/restoration, so keyboard users can still reach background UI while it is open.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsx`
around lines 78 - 170, The panel is visually modal but missing accessibility:
add programmatic modal semantics and focus management to LearnMoreExpandedPanel
by (1) adding role="dialog" and aria-modal="true" to the motion.div (key
"learn-more-panel") and ensuring the header/title is referenced via
aria-labelledby, (2) capture document.activeElement on mount and restore it in
the cleanup that runs when onClose is invoked, (3) set initial focus to the
close button (the button that renders the XIcon) on open (either via autoFocus
or by focusing it using panelRef.current.querySelector), (4) implement a simple
focus trap on the panelRef that intercepts Tab/Shift+Tab to cycle focus within
focusable elements inside the panel, and (5) add an Escape key listener to call
onClose; ensure background content is not reachable (e.g., toggle aria-hidden on
the app root or rely on aria-modal=true) and clean up all listeners on unmount.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The header bar had an "inline 3/3" popover for switching display modes,
duplicating the per-widget Inline/PiP/Fullscreen tabs already available
on each tool result. Removed the popover and cleaned up unused state,
callbacks, and imports.
Also fixed nav-main test: added missing useSidebar mock that was causing
test failures after useSidebar() was added to NavMain.