feat(hosts): move the client selector into the Connect canvas#3269
feat(hosts): move the client selector into the Connect canvas#3269olartgabo wants to merge 1 commit into
Conversation
Follow-up to the PR #3144 review round: instead of growing the small header pager, the client selector now lives INSIDE the host canvas, pinned top-center in a fixed position, sized up to 44px controls. - New HostCanvasSelector: one labeled dropdown trigger (logo + name + 'n / total' position chip) plus an explicit 'Add client' action behind a divider - no adjacent icon-only buttons. - The client menu opens to the LEFT of the card so it floats over empty canvas instead of covering the client card below. - Menu keeps parity with the header pager: per-client delete (guarded on the last client) and quick-add template shortcuts. - The header GlobalHostBar hides itself while the canvas is open so the control isn't duplicated; it still renders on every other tab (and keeps seeding the default client). - Analytics: same connect_host_overlay_* events with location 'host_canvas'; registers connect_host_overlay_add_clicked. - 6 new tests; hosts suites (337 tests) and typecheck:client pass.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dosubot
Bot
added
size:L
✅ 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. |
Internal previewPreview URL: https://mcp-inspector-pr-3269.up.railway.app |
WalkthroughAdds a route-aware header overlay suppression rule and introduces a pinned host selector within the redesigned host canvas. The selector supports switching, creating, deleting, and quickly adding hosts, with navigation, preview state updates, logos, toasts, and analytics tracking. A new analytics event is registered, and tests cover rendering, creation, menu placement, switching, navigation, and deletion constraints. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx`:
- Around line 115-140: Update the track calls in switchTo,
openCreateWithTemplate, and the additionally referenced handlers to run after
the corresponding selector action and be isolated in try/catch blocks, matching
handleDelete. Ensure telemetry exceptions cannot prevent navigation, dialog
opening, or selecting the newly created client.
🪄 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: fb5a66d9-0ba3-4c31-9e7b-bbcb1a95898f
📒 Files selected for processing (5)
mcpjam-inspector/client/src/components/hosts/GlobalHostBar.tsxmcpjam-inspector/client/src/components/hosts/redesigned/HostBuilderViewRedesigned.tsxmcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsxmcpjam-inspector/client/src/components/hosts/redesigned/__tests__/HostCanvasSelector.test.tsxmcpjam-inspector/shared/analytics-events.ts
| const switchTo = (hostId: string) => { | ||
| if (hostId === activeHostId) return; | ||
| track("connect_host_overlay_swapped", { | ||
| location: ANALYTICS_LOCATION, | ||
| from: activeHostId, | ||
| to: hostId, | ||
| host_count: hosts.length, | ||
| }); | ||
| setPreviewedHostId(hostId); | ||
| // The URL is the source of truth for which host the canvas renders | ||
| // (see App's hosts-route sync) — replace so switching doesn't pile | ||
| // history entries. | ||
| navigate(buildHostsPath(hostId), { replace: true }); | ||
| }; | ||
|
|
||
| const openCreateWithTemplate = (templateId?: string) => { | ||
| setCreateTemplateId(templateId); | ||
| setShowCreate(true); | ||
| setMenuOpen(false); | ||
| if (templateId) { | ||
| track("connect_host_overlay_quick_added", { | ||
| location: ANALYTICS_LOCATION, | ||
| template_id: templateId, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep analytics failures from blocking selector actions.
These track() calls are not isolated, so a PostHog exception can prevent navigation, opening the create dialog, or selecting the newly created client. Perform the action first and wrap telemetry, as already done in handleDelete.
Proposed pattern
- track("connect_host_overlay_swapped", {
- location: ANALYTICS_LOCATION,
- from: activeHostId,
- to: hostId,
- host_count: hosts.length,
- });
setPreviewedHostId(hostId);
navigate(buildHostsPath(hostId), { replace: true });
+ try {
+ track("connect_host_overlay_swapped", {
+ location: ANALYTICS_LOCATION,
+ from: activeHostId,
+ to: hostId,
+ host_count: hosts.length,
+ });
+ } catch {
+ // Analytics must not block host switching.
+ }Also applies to: 318-327, 346-352
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx`
around lines 115 - 140, Update the track calls in switchTo,
openCreateWithTemplate, and the additionally referenced handlers to run after
the corresponding selector action and be isolated in try/catch blocks, matching
handleDelete. Ensure telemetry exceptions cannot prevent navigation, dialog
opening, or selecting the newly created client.
prathmeshpatel
left a comment
prathmeshpatel
left a comment
There was a problem hiding this comment.
The client menu opens to the left
Would love to see a full screen recording of what behavior looks like now!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx:79">
P3: Opening Connect’s canvas no longer records `connect_host_overlay_opened` for the new `host_canvas` surface, so analytics loses the entry event while all later selector actions remain attributed. Add the one-time mount tracking used by `HostOverlayBar`, with `location: "host_canvas"`.</violation>
<violation number="2" location="mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx:269">
P2: Quick-add template clicks can fall through the surrounding menu item and also run the generic Add client handler, which risks duplicate create-dialog opens and double analytics for the same action.</violation>
</file>
<file name="mcpjam-inspector/client/src/components/hosts/GlobalHostBar.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/hosts/GlobalHostBar.tsx:27">
P2: The header host selector is being hidden on the hosts browse view as soon as a previewed host exists, not just when the canvas is open. That removes the control from `/hosts` even though the page is still in browse mode.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ))} | ||
| </DropdownMenuRadioGroup> | ||
| <DropdownMenuSeparator /> | ||
| <DropdownMenuItem |
There was a problem hiding this comment.
P2: Quick-add template clicks can fall through the surrounding menu item and also run the generic Add client handler, which risks duplicate create-dialog opens and double analytics for the same action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx, line 269:
<comment>Quick-add template clicks can fall through the surrounding menu item and also run the generic Add client handler, which risks duplicate create-dialog opens and double analytics for the same action.</comment>
<file context>
@@ -0,0 +1,356 @@
+ ))}
+ </DropdownMenuRadioGroup>
+ <DropdownMenuSeparator />
+ <DropdownMenuItem
+ data-testid="host-canvas-menu-add"
+ onSelect={() => openCreateWithTemplate(undefined)}
</file context>
| if (onHostsRoute && (urlHostId || previewedHostId)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
P2: The header host selector is being hidden on the hosts browse view as soon as a previewed host exists, not just when the canvas is open. That removes the control from /hosts even though the page is still in browse mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/hosts/GlobalHostBar.tsx, line 27:
<comment>The header host selector is being hidden on the hosts browse view as soon as a previewed host exists, not just when the canvas is open. That removes the control from `/hosts` even though the page is still in browse mode.</comment>
<file context>
@@ -1,13 +1,33 @@
+ .slice(`${routePaths.hosts}/`.length)
+ .split("/")[0] || null)
+ : null;
+ if (onHostsRoute && (urlHostId || previewedHostId)) {
+ return null;
+ }
</file context>
| if (onHostsRoute && (urlHostId || previewedHostId)) { | |
| return null; | |
| } | |
| if (urlHostId) { | |
| return null; | |
| } |
| @@ -0,0 +1,356 @@ | |||
| import { useMemo, useState } from "react"; | |||
There was a problem hiding this comment.
P3: Opening Connect’s canvas no longer records connect_host_overlay_opened for the new host_canvas surface, so analytics loses the entry event while all later selector actions remain attributed. Add the one-time mount tracking used by HostOverlayBar, with location: "host_canvas".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/hosts/redesigned/HostCanvasSelector.tsx, line 79:
<comment>Opening Connect’s canvas no longer records `connect_host_overlay_opened` for the new `host_canvas` surface, so analytics loses the entry event while all later selector actions remain attributed. Add the one-time mount tracking used by `HostOverlayBar`, with `location: "host_canvas"`.</comment>
<file context>
@@ -0,0 +1,356 @@
+ activeHostId: string;
+}
+
+export function HostCanvasSelector({
+ projectId,
+ activeHostId,
</file context>
3 participants
UX: client selector moves into the canvas
Follow-up to the #3144 review round ("this needs to be bigger… not a fan of two buttons next to each other… bringing the selector into the canvas in a fixed position and make it larger is a good call for now"). This supersedes the header
+approach from #3144 with the design the team picked from the exploration screenshots.Change
HostCanvasSelector, pinned top-center over the Connect host canvas in a fixed position (it does not pan/zoom with the flow). One large labeled trigger — client logo + name +n / totalposition chip — plus an explicit Add client action behind a divider. No adjacent icon-only buttons, and the add path is fully labeled.side="left") so it floats over empty canvas instead of covering the client card underneath.GlobalHostBarhides itself while the canvas is open so the control isn't duplicated; every other tab keeps the header pager unchanged (including its default-client seeding).connect_host_overlay_*events fire withlocation: "host_canvas"; registersconnect_host_overlay_add_clicked.Screenshots
Testing
HostCanvasSelector.test.tsx(labeled add button + tracking, menudata-side="left", switch navigates + updates preview pointer, delete guard on last client).typecheck:clientpass.2026-07-16.17-45-48.mp4
Summary by cubic
Moves the client selector into the Connect host canvas with a top‑center overlay, replacing the header pager while the canvas is open. Uses a single labeled switcher and an explicit Add client action for clearer controls.
GlobalHostBarhides on the hosts route when a host is active; unchanged elsewhere.connect_host_overlay_*events includelocation: "host_canvas"; newconnect_host_overlay_add_clickedevent added.Written for commit e6bc817. Summary will update on new commits.