feat(sidebar): refactor project sidebar into spaces with pinned sidebar and card redesign#2837
Open
feat(sidebar): refactor project sidebar into spaces with pinned sidebar and card redesign#2837
Conversation
Contributor
🧪 BenchmarkShould we run the Virtual MCP strategy benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Contributor
Release OptionsSuggested: Minor ( React with an emoji to override the release type:
Current version:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 9 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="apps/mesh/src/web/components/sidebar/agents-section.tsx">
<violation number="1" location="apps/mesh/src/web/components/sidebar/agents-section.tsx:57">
P2: Avoid nesting a `<button>` inside `SidebarMenuButton` (which already renders a button). This produces invalid interactive markup and can cause inconsistent interaction behavior.</violation>
</file>
<file name="apps/mesh/src/web/components/sidebar/projects-section.tsx">
<violation number="1" location="apps/mesh/src/web/components/sidebar/projects-section.tsx:41">
P2: Initialize/derive project row open state from the current pathname so active pinned views are visible.</violation>
<violation number="2" location="apps/mesh/src/web/components/sidebar/projects-section.tsx:83">
P2: Avoid nesting a native `<button>` inside `SidebarMenuButton` because it already renders a `<button>`.</violation>
</file>
<file name="apps/mesh/src/web/hooks/use-project-sidebar-items.tsx">
<violation number="1" location="apps/mesh/src/web/hooks/use-project-sidebar-items.tsx:56">
P2: `isActiveRoute` no longer matches project-scoped routes (`/org/projects/...`), so plugin and pinned-view items will never be marked active. Include the project base path in the active-route check to keep highlighting correct.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Move agents from nav item to expandable accordion section (matching projects pattern). Remove project-scoped sidebar branching to unify routing under the org level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace subtype discriminator with pinned boolean column. The sidebar now shows a single "Spaces" accordion with only pinned items and a popover picker to pin/unpin. New unified routes under /$org/spaces/$virtualMcpId. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
6 issues found across 20 files (changes from recent commits).
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="apps/mesh/src/web/index.tsx">
<violation number="1" location="apps/mesh/src/web/index.tsx:606">
P2: Spaces plugin routes are missing plugin-registered child routes, so nested plugin pages work under `/projects/...` but not under `/spaces/...`.</violation>
</file>
<file name="apps/mesh/migrations/052-spaces.ts">
<violation number="1" location="apps/mesh/migrations/052-spaces.ts:23">
P0: Wrong CHECK constraint name — the `up` migration will fail when dropping the `subtype` column.
Migration 048 created the constraint as `chk_connections_subtype`, but this migration tries to drop `connections_subtype_check`. The `IF EXISTS` silently no-ops, leaving the real constraint in place, which blocks `DROP COLUMN subtype`.</violation>
<violation number="2" location="apps/mesh/migrations/052-spaces.ts:60">
P2: Down migration re-creates the CHECK constraint with the wrong name `connections_subtype_check` instead of the original `chk_connections_subtype`. This breaks round-trip consistency — rolling back and re-applying migrations would leave a misnamed constraint.</violation>
</file>
<file name="apps/mesh/src/web/components/sidebar/spaces-section.tsx">
<violation number="1" location="apps/mesh/src/web/components/sidebar/spaces-section.tsx:88">
P2: Avoid nesting a native `<button>` inside `SidebarMenuButton` (which already renders as `<button>`). This creates invalid interactive markup and can break keyboard/screen-reader behavior.</violation>
</file>
<file name="apps/mesh/src/web/hooks/use-create-virtual-mcp.ts">
<violation number="1" location="apps/mesh/src/web/hooks/use-create-virtual-mcp.ts:57">
P1: New-agent creation navigates to a non-existent route (`/$org/spaces/$virtualMcpId`) instead of the defined settings route.</violation>
</file>
<file name="apps/mesh/src/web/components/import-from-deco-dialog.tsx">
<violation number="1" location="apps/mesh/src/web/components/import-from-deco-dialog.tsx:139">
P1: The import create payload dropped `subtype: "project"`, so imported items can be excluded from project queries that filter by subtype.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .execute(); | ||
|
|
||
| // Drop the CHECK constraint on subtype | ||
| await sql`ALTER TABLE connections DROP CONSTRAINT IF EXISTS connections_subtype_check`.execute( |
Contributor
There was a problem hiding this comment.
P0: Wrong CHECK constraint name — the up migration will fail when dropping the subtype column.
Migration 048 created the constraint as chk_connections_subtype, but this migration tries to drop connections_subtype_check. The IF EXISTS silently no-ops, leaving the real constraint in place, which blocks DROP COLUMN subtype.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/migrations/052-spaces.ts, line 23:
<comment>Wrong CHECK constraint name — the `up` migration will fail when dropping the `subtype` column.
Migration 048 created the constraint as `chk_connections_subtype`, but this migration tries to drop `connections_subtype_check`. The `IF EXISTS` silently no-ops, leaving the real constraint in place, which blocks `DROP COLUMN subtype`.</comment>
<file context>
@@ -0,0 +1,73 @@
+ .execute();
+
+ // Drop the CHECK constraint on subtype
+ await sql`ALTER TABLE connections DROP CONSTRAINT IF EXISTS connections_subtype_check`.execute(
+ db,
+ );
</file context>
Suggested change
| await sql`ALTER TABLE connections DROP CONSTRAINT IF EXISTS connections_subtype_check`.execute( | |
| await sql`ALTER TABLE connections DROP CONSTRAINT IF EXISTS chk_connections_subtype`.execute( |
| if (navigateOnCreate) { | ||
| navigate({ | ||
| to: "/$org/agents/$agentId", | ||
| to: "/$org/spaces/$virtualMcpId", |
Contributor
There was a problem hiding this comment.
P1: New-agent creation navigates to a non-existent route (/$org/spaces/$virtualMcpId) instead of the defined settings route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/hooks/use-create-virtual-mcp.ts, line 57:
<comment>New-agent creation navigates to a non-existent route (`/$org/spaces/$virtualMcpId`) instead of the defined settings route.</comment>
<file context>
@@ -49,16 +49,15 @@ export function useCreateVirtualMCP(
if (navigateOnCreate) {
navigate({
- to: "/$org/agents/$agentId",
+ to: "/$org/spaces/$virtualMcpId",
params: {
org: org.slug,
</file context>
Suggested change
| to: "/$org/spaces/$virtualMcpId", | |
| to: "/$org/spaces/$virtualMcpId/settings", |
tlgimenes
force-pushed
the
tlgimenes/project-sidebar-refactor
branch
from
c060769 to
9c28929
Compare
…ed navigation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tlgimenes
force-pushed
the
tlgimenes/project-sidebar-refactor
branch
from
9c28929 to
d5e723d
Compare
- Move project icon into the colored banner row (vertically centered, left-aligned) - Move edited timestamp to footer row alongside user avatar - Replace org badge with last-edited-by user avatar and tooltip - Remove unused ConnectionIcon component - Restore AgentsList on home page (matching main branch) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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="apps/mesh/src/web/components/home/agents-list.tsx">
<violation number="1">
P2: useEffect is banned in this codebase; avoid adding new effects (even with lint suppression). Move the focus behavior to the popover’s open-change handler or a render‑phase microtask pattern instead.
(Based on your team's feedback about avoiding useEffect in this codebase.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -18,8 +18,11 @@ import { | |||
| } from "@deco/ui/components/popover.tsx"; | |||
Contributor
There was a problem hiding this comment.
P2: useEffect is banned in this codebase; avoid adding new effects (even with lint suppression). Move the focus behavior to the popover’s open-change handler or a render‑phase microtask pattern instead.
(Based on your team's feedback about avoiding useEffect in this codebase.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/components/home/agents-list.tsx, line 102:
<comment>useEffect is banned in this codebase; avoid adding new effects (even with lint suppression). Move the focus behavior to the popover’s open-change handler or a render‑phase microtask pattern instead.
(Based on your team's feedback about avoiding useEffect in this codebase.) </comment>
<file context>
@@ -0,0 +1,297 @@
+
+ // Focus search input when popover opens (skip on mobile to avoid keyboard popup)
+ // oxlint-disable-next-line ban-use-effect/ban-use-effect
+ useEffect(() => {
+ if (open && !isMobile) {
+ setTimeout(() => {
</file context>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tlgimenes
changed the title
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="apps/mesh/src/web/components/sidebar/spaces-section.tsx">
<violation number="1" location="apps/mesh/src/web/components/sidebar/spaces-section.tsx:63">
P1: This adds a nested `<button>` inside `SidebarMenuButton` (which already renders a `<button>`), creating invalid interactive markup. Use a non-button wrapper for the row or move the settings action outside the button element.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ee all - Show agent name, description, and larger icon in pin popover items - Add "Create new" and "See all" action buttons in popover footer Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="apps/mesh/src/web/components/chat/tasks-panel.tsx">
<violation number="1" location="apps/mesh/src/web/components/chat/tasks-panel.tsx:510">
P2: Scoping `availableAgents` to the current space can strand a stale `agentFilter` and produce an empty task list with no visible way to clear the filter.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| const availableAgents = [ | ||
| ...new Set(visible.flatMap((t) => t.agent_ids ?? [])), | ||
| ...new Set(spaceFiltered.flatMap((t) => t.agent_ids ?? [])), |
Contributor
There was a problem hiding this comment.
P2: Scoping availableAgents to the current space can strand a stale agentFilter and produce an empty task list with no visible way to clear the filter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/components/chat/tasks-panel.tsx, line 510:
<comment>Scoping `availableAgents` to the current space can strand a stale `agentFilter` and produce an empty task list with no visible way to clear the filter.</comment>
<file context>
@@ -500,15 +500,21 @@ export function TaskListContent({ onTaskSelect }: TaskListContentProps) {
+
const availableAgents = [
- ...new Set(visible.flatMap((t) => t.agent_ids ?? [])),
+ ...new Set(spaceFiltered.flatMap((t) => t.agent_ids ?? [])),
];
</file context>
Widen the collapsed sidebar to 6rem and scale all icons/avatars 2x. Constrain the inner container width when collapsed and center items so they align in the middle of the visible area. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move CollapsibleContent outside SidebarMenu to avoid invalid ul > div > li nesting. Make SidebarMenuItem w-full so it fills the parent ul. Change SidebarMenuButton collapsed variant from fixed size-12 to h-12 w-full with justify-center so icons are properly centered in the sidebar. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use size-12 instead of h-12 w-full so buttons are square in collapsed mode, centered within the full-width li. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Width fills the parent, height matches via aspect-square, making buttons square and responsive to the sidebar width. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Override AgentAvatar to w-full h-full when collapsed so the colored icon background fills the square button. Inner SVGs scale to 50% of the container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Streamline sidebar by removing unused collapse-related code, simplifying account switcher, and extracting settings into a dedicated layout with nested routes. Reduces ~500 lines of code across 43 files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Renumber 052-spaces migration to 053 to accommodate 052-thread-agent-ids from main - Accept deletion of monitoring dashboard files from main - Keep settings layout refactor, remove orphaned dashboard routes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make the pin-to-sidebar action also navigate to the space. Reduce collapsed sidebar width from 48px to 40px. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reduce collapsed sidebar from 96px to 56px, use SidebarMenuButton for all sidebar buttons (org switcher, settings, inbox, user menu) to match the spaces section style, and update border-radius to rounded-lg. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New spaces are now pinned to the sidebar on creation. Increased gap between space items in the sidebar from 2px to 8px. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add settings gear icon to the tasks panel header that navigates to the current space's settings page. Tasks panel now reads virtualMcpId from the URL route params instead of the chat store. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix extra closing brace in tasks panel gear icon onClick handler. Remove extra padding from connections tab content in space settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What is this contribution about?
Refactors the sidebar and spaces UI with several improvements:
/$org/spaces(list) and/$org/spaces/$virtualMcpId(home, settings, apps, workflows)pinnedcolumn to connections, replacedsubtypewith pinned-based filteringScreenshots/Demonstration
N/A
How to Test
/spaces— space cards should show icon in the colored banner, user avatar in footer with "Last edited by" tooltip, and timestampMigration Notes
pinnedboolean toconnectionstablesubtypecolumn and replaces index with(organization_id, connection_type, pinned)Review Checklist
bun run check,bun run lint,bun run fmtall pass