-
Notifications
You must be signed in to change notification settings - Fork 30
Introduce Home of project and auto-pinning for created resources #1860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a ProjectHome UI and routes "home://self" to it, restructures the sidebar (HOME and MCPs buttons), introduces optional auto-pinning when opening/creating resources, adds copy-to-clipboard and create-aware auto-open logic in tool messages, and migrates old default pinned tabs to an empty default. Changes
Sequence Diagram(s)sequenceDiagram
participant Tool as Tool UI (tool-message)
participant Utils as openResourceTab
participant Tabs as Tabs Store (addTab / setActiveTab)
participant Pins as pinTab (usePinnedTabs)
Note over Tool: User or auto flow triggers resource open
Tool->>Utils: openResourceTab(resourceUri, setActiveTab, pinTab?, shouldPin?)
alt shouldPin == true and pinTab provided
Utils->>Tabs: addTab({resourceUri, title, type})
Tabs-->>Utils: tabCreated
Utils->>Pins: pinTab({resourceUri, title, type:"detail", icon?})
Pins-->>Tabs: pinned
else no pin
Utils->>Tabs: addTab({resourceUri, title, type})
end
Utils->>Tabs: setActiveTab(tabId)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to review closely:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧬 Code graph analysis (3)apps/web/src/components/chat/tool-message.tsx (3)
apps/web/src/components/sidebar/index.tsx (2)
apps/web/src/components/canvas/canvas-tab-content.tsx (1)
🪛 GitHub Actions: Testsapps/web/src/components/sidebar/index.tsx[warning] 255-255: ESLint: 'searchParams' is declared but never used. Unused variables should start with a '_'. [warning] 255-255: ESLint: 'setSearchParams' is declared but never used. Unused variables should start with a '_'. 🔇 Additional comments (18)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/web/src/utils/resource-tabs.ts (1)
92-92: Potential undefined title fallback.The
.pop()method can returnundefinedif the array is empty. While this is unlikely for a resource URI, the fallback to "Resource" only happens when the result is falsy, not specifically undefined.Consider making the fallback more explicit:
- const title = resourceUri.split("/").pop() || "Resource"; + const title = resourceUri.split("/").pop() ?? "Resource";Using the nullish coalescing operator (
??) makes it clearer that you're handlingundefined.apps/web/src/components/project/home.tsx (1)
91-93: Consider handling more connection types.The current implementation only handles
connection.type === "HTTP"and falls back to "Unknown connection" for all other types. If other connection types (e.g., WebSocket, SSE) are valid and expected, consider handling them explicitly.If other types are expected, consider:
- {selfIntegration.connection?.type === "HTTP" - ? selfIntegration.connection.url - : "Unknown connection"} + {selfIntegration.connection?.type === "HTTP" && selfIntegration.connection.url + ? selfIntegration.connection.url + : selfIntegration.connection?.type === "STDIO" + ? "Standard I/O" + : "Unknown connection"}apps/web/src/components/sidebar/index.tsx (4)
488-507: Avoid opening duplicate Home tabs; reuse existing tab when possibleThe Home button always calls
addTabwithresourceUri: "home://self"without checking for an existing Home tab, unlikerenderPinnedTab, which reuses existing tabs byresourceUri. This can lead to multiple identical Home tabs.Consider reusing an existing Home tab before creating a new one, e.g.:
onClick={() => { const existingTab = tabs.find((t) => t.resourceUri === "home://self"); if (existingTab) { setActiveTab(existingTab.id); } else { addTab({ type: "detail", resourceUri: "home://self", title: "Home", icon: "home", }); } trackEvent("sidebar_navigation_click", { item: "Home" }); isMobile && toggleSidebar(); }}
510-534: Apply the same “reuse existing tab” pattern for MCPsSimilar to Home, the MCPs button always creates a new list tab for
buildAppsListUri(). Given thatbuildAppsListUrireturns a stable URI, repeated clicks may create duplicate MCPs tabs.Consider mirroring the pattern used in
renderPinnedTab(and the suggested Home change) by checkingtabsfor an existing MCPs tab byresourceUriand callingsetActiveTabinstead of alwaysaddTab.
536-567: Pinned section conditional rendering looks good; minor a11y enhancement possibleThe conditional
pinnedTabs.length > 0plus the drag‑mode toggle is a clean improvement—no empty “Pinned” section and a clear affordance for reordering.You might optionally add
aria-pressed={isDragMode}to the drag toggle<button>so assistive tech recognizes it as a stateful toggle, but the current implementation is otherwise solid.
599-611: Close sidebar on mobile after creating a new threadThe New thread button creates a thread and tracks the click but does not close the sidebar on mobile, unlike navigation actions such as Home and MCPs that call
isMobile && toggleSidebar().For consistency and a smoother mobile UX, consider:
onClick={() => { createThread(); trackEvent("sidebar_new_thread_click"); isMobile && toggleSidebar(); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/src/components/canvas/canvas-tab-content.tsx(2 hunks)apps/web/src/components/chat/tool-message.tsx(5 hunks)apps/web/src/components/decopilot/thread-provider.tsx(1 hunks)apps/web/src/components/home/project-home.tsx(1 hunks)apps/web/src/components/project/home.tsx(1 hunks)apps/web/src/components/resources-v2/list.tsx(6 hunks)apps/web/src/components/sidebar/index.tsx(3 hunks)apps/web/src/hooks/use-pinned-tabs.ts(1 hunks)apps/web/src/utils/resource-tabs.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/src/components/resources-v2/list.tsx (1)
apps/web/src/hooks/use-pinned-tabs.ts (1)
usePinnedTabs(50-156)
apps/web/src/components/project/home.tsx (4)
packages/sdk/src/mcp/schema.ts (1)
integrations(351-363)packages/sdk/src/hooks/mcp.ts (1)
useIntegrations(141-175)apps/web/src/components/integrations/common.tsx (1)
IntegrationIcon(19-35)apps/web/src/components/resources-v2/list.tsx (1)
ResourcesV2List(1695-1747)
apps/web/src/components/canvas/canvas-tab-content.tsx (2)
apps/web/src/components/project/home.tsx (1)
ProjectHome(46-117)packages/sdk/src/lazy.ts (1)
lazy(1-9)
apps/web/src/components/chat/tool-message.tsx (3)
packages/sdk/src/hooks/store.tsx (1)
useSDK(36-44)apps/web/src/hooks/use-pinned-tabs.ts (1)
usePinnedTabs(50-156)apps/web/src/utils/resource-tabs.ts (1)
openResourceTab(58-112)
apps/web/src/components/sidebar/index.tsx (3)
apps/web/src/hooks/analytics.ts (1)
trackEvent(48-61)apps/web/src/components/decopilot/thread-provider.tsx (1)
buildAppsListUri(1011-1013)packages/ai/src/agent.ts (1)
createThread(899-925)
⏰ 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: Cloudflare Pages
🔇 Additional comments (15)
apps/web/src/components/decopilot/thread-provider.tsx (1)
172-177: LGTM!The home URI parsing is correctly placed and follows the same pattern as other URI checks. The placement before the agents list check maintains proper precedence.
apps/web/src/components/canvas/canvas-tab-content.tsx (2)
19-19: LGTM!Lazy loading the ProjectHome component is appropriate and follows the established pattern for other components in this file.
305-307: LGTM!The routing branch for the home protocol is correctly implemented and follows the same pattern as other protocol handlers.
apps/web/src/components/home/project-home.tsx (1)
72-72: Added background styling to chat panel.The
bg-sidebarclass addition provides visual consistency for the chat panel. No functional concerns.apps/web/src/components/resources-v2/list.tsx (2)
873-881: Auto-pinning newly created documents.The implementation correctly pins newly created resources only after successful tab creation. The pattern is consistent and appropriate.
973-981: Consistent auto-pinning across all creation flows.The auto-pinning logic is consistently applied across document templates, AI PRD templates, and generic resource creation. Each flow correctly checks for successful tab creation before pinning.
Also applies to: 1044-1052, 1152-1160
apps/web/src/components/chat/tool-message.tsx (4)
773-776: CREATE operation detection logic.The detection of CREATE operations using
toolName?.includes("_CREATE")is straightforward and aligns with the naming convention used in the codebase.
786-805: Auto-pin on resource creation with proper dependencies.The effect correctly auto-opens and auto-pins newly created resources. The dependency array includes all necessary values, and the
hasOpenedTabRefensures the effect runs only once per tool call.
812-812: Intentional behavior: Manual clicks don't auto-pin.The comment correctly clarifies that manual clicks should not trigger auto-pinning, only automatic opens after creation. This is the right UX decision.
830-855: Copy-to-clipboard functionality.The copy button implementation is clean and uses proper tooltip feedback. The
useCopyhook handles the clipboard interaction and state management.apps/web/src/components/project/home.tsx (1)
46-117: Well-structured ProjectHome component.The component is cleanly organized with:
- Proper hook usage and memoization
- Clear separation of header and content sections
- Graceful handling of missing integration data
- Appropriate use of ResourcesV2List for resource management
apps/web/src/hooks/use-pinned-tabs.ts (3)
35-42: Migration constant for old default pins.The
OLD_DEFAULT_NATIVE_URISconstant properly documents the old default pinned tabs that need to be removed during migration.
54-65: Migration removes old default pins on first load.The migration logic correctly filters out old default native URIs while preserving user-created pins. This runs once when the hook initializes with existing data.
Note: This migration is permanent - once a user loads the updated code, the old defaults are removed from localStorage and won't return even if they revert to an older version. This is typically the desired behavior for this type of migration.
48-48: Design change: Empty default pinned tabs.The shift from auto-populated defaults to an empty array means users start with a clean slate and manually pin what they want. This aligns with the PR's goal of auto-pinning created resources instead.
apps/web/src/components/sidebar/index.tsx (1)
570-597: Threads section label & structure are consistent with new sidebar UXRenaming the header to “Threads” and keeping the thread selector action under it aligns well with the new structure and doesn’t introduce any behavioral changes.
96ae072 to
9543244
Compare
Deploying decocms-admin-frontend with
|
| Latest commit: |
9543244
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e94e03e0.admin-decocms-frontend.pages.dev |
| Branch Preview URL: | https://valls-pinned-resources.admin-decocms-frontend.pages.dev |
Direct access to all project resources through Home, while created resources automatically appear in the Pinned section for easy access. This is a step that introduces Projects as a scoped App until we get MCPs in our organization.
Summary by CodeRabbit
New Features
UI Updates
Chores / Migration