refactor (desktop): New workspace button#596
Conversation
📝 WalkthroughWalkthroughRemoves the original WorkspaceSidebarHeader file and replaces it with a new header component plus a dedicated NewWorkspaceButton that queries the active workspace and opens the new-workspace modal with an optional preselected projectId. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
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 |
AviPeltz
changed the title
![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
Fix all issues with AI Agents 🤖
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx:
- Around line 6-7: The getActive query result isn't handling loading/error
states so activeWorkspace (and derived projectId) may be undefined when used;
update the trpc.workspaces.getActive.useQuery call to destructure isLoading and
error alongside data, then: (1) disable the UI trigger by passing
disabled={isLoading} to the button that calls useOpenNewWorkspaceModal(), (2)
show or handle error (e.g., render an error state or toast) when error is
present, and (3) ensure openModal is invoked only with a defined projectId
(guard with if (activeWorkspace?.projectId) or pass undefined intentionally and
handle it inside useOpenNewWorkspaceModal) so clicks during load/failure don’t
pass an undefined projectId.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx (1)
24-33: Consider adding accessibility attributes to the button.The button implementation is clean, but could benefit from explicit accessibility attributes for screen readers.
🔎 Suggested accessibility improvements
<button type="button" onClick={handleNewWorkspace} + aria-label="Create new workspace" className="flex items-center gap-2 px-2 py-1.5 text-sm font-medium text-muted-foreground hover:text-foreground hover:bg-accent/50 rounded-md transition-colors" >If you add the
disabledprop (as suggested in the earlier comment), also add disabled state styling:- className="flex items-center gap-2 px-2 py-1.5 text-sm font-medium text-muted-foreground hover:text-foreground hover:bg-accent/50 rounded-md transition-colors" + className="flex items-center gap-2 px-2 py-1.5 text-sm font-medium text-muted-foreground hover:text-foreground hover:bg-accent/50 rounded-md transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx (1)
apps/desktop/src/renderer/stores/new-workspace-modal.ts (1)
useOpenNewWorkspaceModal(32-33)
🔇 Additional comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx (1)
1-3: LGTM! Imports follow guidelines.The imports correctly use path aliases and leverage Zustand for state management via the
useOpenNewWorkspaceModalhook, adhering to the project's architectural patterns.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx:
- Around line 5-15: NewWorkspaceButton is not handling trpc.workspaces.getActive
errors: destructure isError and error from trpc.workspaces.getActive.useQuery()
in the NewWorkspaceButton component, log the error with context (e.g., "Failed
to load active workspace" plus the error) when isError is true, and update the
button's disabled prop to disabled={isLoading || isError} so the button is
disabled on query failures; keep passing activeWorkspace?.projectId to openModal
when not errored.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
🧠 Learnings (3)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/**/[A-Z]*.tsx : Create component folder structure with one folder per component: ComponentName/ComponentName.tsx with barrel export index.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx (1)
apps/desktop/src/renderer/stores/new-workspace-modal.ts (1)
useOpenNewWorkspaceModal(32-33)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx (1)
NewWorkspaceButton(5-30)
⏰ 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: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx (2)
1-3: Imports follow best practices.The imports correctly use the renderer alias for tRPC and leverage the zustand store hook for modal management, aligning with the coding guidelines for this codebase.
17-29: Button implementation looks good.The JSX structure properly uses semantic HTML (
type="button"), handles the disabled state, and provides clear visual feedback with Tailwind classes. The icon and label combination enhances usability.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (1)
1-18: Clean presentational component - LGTM!The component follows the established folder structure pattern (ComponentName/ComponentName.tsx with barrel export) and provides a clear, semantic header for the workspace sidebar. The separation of the button logic into
NewWorkspaceButtonmaintains good component modularity.Based on learnings: The component folder structure correctly follows the pattern for components with PascalCase names.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/index.ts (1)
1-1: Appropriate barrel export for component folder structure.This minimal re-export follows the established pattern for component folders and doesn't constitute "barrel file abuse" as warned in the coding guidelines. It provides a clean public API for the WorkspaceSidebarHeader component.
Based on learnings: Correctly implements barrel export pattern for component folders (ComponentName/index.ts).
| export function NewWorkspaceButton() { | ||
| const openModal = useOpenNewWorkspaceModal(); | ||
| const { data: activeWorkspace, isLoading } = | ||
| trpc.workspaces.getActive.useQuery(); | ||
|
|
||
| const handleClick = () => { | ||
| // projectId may be undefined if no workspace is active or query failed | ||
| // openModal handles undefined by opening without a pre-selected project | ||
| const projectId = activeWorkspace?.projectId; | ||
| openModal(projectId); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read its content
cat -n "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx"Repository: superset-sh/superset
Length of output: 1337
🏁 Script executed:
# Search for similar tRPC query patterns in renderer components to understand error handling practices
rg "trpc\." apps/desktop/src/renderer --type tsx --type ts -A 3 -B 1 | head -100Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check how isError is used in other components
rg "isError" apps/desktop/src/renderer --type tsx --type ts -B 2 -A 2 | head -80Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for error handling patterns in renderer components
rg "isError" apps/desktop/src/renderer -A 2 -B 2Repository: superset-sh/superset
Length of output: 3490
🏁 Script executed:
# Check for trpc query error handling patterns
rg "useQuery.*error" apps/desktop/src/renderer -A 3 -B 1Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at the trpc configuration to understand what's available
cat -n "apps/desktop/src/renderer/lib/trpc.ts" 2>/dev/null || cat -n "src/lib/trpc.ts"Repository: superset-sh/superset
Length of output: 375
🏁 Script executed:
# Search for similar error logging patterns with context
rg "console\.error.*\[" apps/desktop/src/renderer | head -20Repository: superset-sh/superset
Length of output: 1422
Add error handling for tRPC query failures
The component destructures isLoading from the query result but ignores isError. When trpc.workspaces.getActive fails, the button remains enabled and passes undefined to openModal.
Per the coding guideline "Never swallow errors silently - at minimum log them with context", destructure isError and log the error:
export function NewWorkspaceButton() {
const openModal = useOpenNewWorkspaceModal();
- const { data: activeWorkspace, isLoading } =
+ const { data: activeWorkspace, isLoading, isError, error } =
trpc.workspaces.getActive.useQuery();
+ if (isError) {
+ console.error('[NewWorkspaceButton/getActive]', error);
+ }
+
const handleClick = () => {
// projectId may be undefined if no workspace is active or query failed
// openModal handles undefined by opening without a pre-selected project
const projectId = activeWorkspace?.projectId;
openModal(projectId);
};Alternatively or additionally, disable the button on error by changing the button's disabled attribute to disabled={isLoading || isError}.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function NewWorkspaceButton() { | |
| const openModal = useOpenNewWorkspaceModal(); | |
| const { data: activeWorkspace, isLoading } = | |
| trpc.workspaces.getActive.useQuery(); | |
| const handleClick = () => { | |
| // projectId may be undefined if no workspace is active or query failed | |
| // openModal handles undefined by opening without a pre-selected project | |
| const projectId = activeWorkspace?.projectId; | |
| openModal(projectId); | |
| }; | |
| export function NewWorkspaceButton() { | |
| const openModal = useOpenNewWorkspaceModal(); | |
| const { data: activeWorkspace, isLoading, isError, error } = | |
| trpc.workspaces.getActive.useQuery(); | |
| if (isError) { | |
| console.error('[NewWorkspaceButton/getActive]', error); | |
| } | |
| const handleClick = () => { | |
| // projectId may be undefined if no workspace is active or query failed | |
| // openModal handles undefined by opening without a pre-selected project | |
| const projectId = activeWorkspace?.projectId; | |
| openModal(projectId); | |
| }; |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx
around lines 5-15, NewWorkspaceButton is not handling trpc.workspaces.getActive
errors: destructure isError and error from trpc.workspaces.getActive.useQuery()
in the NewWorkspaceButton component, log the error with context (e.g., "Failed
to load active workspace" plus the error) when isError is true, and update the
button's disabled prop to disabled={isLoading || isError} so the button is
disabled on query failures; keep passing activeWorkspace?.projectId to openModal
when not errored.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.