Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@AviPeltz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughReplaces the DropdownMenu with a split ButtonGroup (primary action + dropdown trigger); adds state to paginate/show more recent projects; computes current/other/recent projects from active workspace, recents, and homeDir; adds formatPath helper; and reorganizes dropdown items and actions (create workspace, browse). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (1)
53-71: Use functional state update forshowAllProjectstoggle and consider list shrinkageThe
showAllProjectsflag and derivedvisibleProjectslogic look good, but usingsetShowAllProjects(!showAllProjects)can be fragile under concurrent updates. A functional update avoids stale closures and is the idiomatic pattern in modern React.You can tighten this up as:
-const [showAllProjects, setShowAllProjects] = useState(false); +const [showAllProjects, setShowAllProjects] = useState(false); … - onClick={() => setShowAllProjects(!showAllProjects)} + onClick={() => setShowAllProjects((prev) => !prev)}Optionally (not required), if
otherProjects.lengthdrops belowINITIAL_PROJECTS_LIMIT, you may want to resetshowAllProjectstofalseto avoid holding a stale “expanded” state when there’s nothing to expand anymore.Also applies to: 200-217
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (5)
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
useCreateWorkspace(12-59)apps/desktop/src/renderer/react-query/projects/useOpenNew.ts (1)
useOpenNew(7-22)packages/ui/src/components/button-group.tsx (1)
ButtonGroup(17-17)packages/ui/src/components/tooltip.tsx (3)
Tooltip(74-74)TooltipTrigger(74-74)TooltipContent(74-74)packages/ui/src/components/dropdown-menu.tsx (3)
DropdownMenu(245-245)DropdownMenuTrigger(247-247)DropdownMenuContent(248-248)
⏰ 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 (2)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (2)
61-71: Data derivation and primary/dropdown actions look coherentThe way you derive
currentProject,otherProjects, and wire them intohandlePrimaryAction, the “Current project” and “Other/Recent projects” sections, plus the final “Browse for Project...” action is consistent and respects the pending states oncreateWorkspaceandopenNew. The reuse ofhandleCreateWorkspacekeeps toast handling centralized.If you want to tighten behavior further later, one thing to double‑check is the UX when
activeWorkspaceis available but its project is not present inrecentProjectsyet (or anymore): in that casecurrentProjectisundefinedand the primary action falls back to “Browse” instead of “New workspace in current project”. That might be acceptable, but it’s worth confirming matches the intended flow.Also applies to: 98-105, 152-199, 220-229
23-45: The edge case described cannot occur. Node.jsos.homedir()(the source ofhomeDirviatrpc.window.getHomeDir) never returns paths with trailing slashes. The current implementation is correct and handles the homeDir value as it will actually be provided. The suggested.replace(/\/$/, "")fix is unnecessary.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (2)
23-49: Consider extractingformatPathfor unit testing.The path formatting logic is non-trivial with regex operations and edge case handling. Extracting to a separate utility file (e.g.,
utils/formatPath.ts) would allow direct unit testing without rendering the component.
140-157: Nested Tooltip + DropdownMenuTrigger may cause interaction quirks.When both
TooltipandDropdownMenushare the same trigger, the tooltip may not dismiss properly when the dropdown opens, causing visual overlap. Consider removing the tooltip on the dropdown trigger or testing the interaction to ensure smooth UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
⏰ 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 (5)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (5)
1-19: LGTM!Imports are correctly structured: using path aliases as per
tsconfig.json, tRPC for IPC, and no Node.js modules in renderer process code.
55-74: LGTM!Proper use of local
useStatefor UI state, tRPC queries for IPC, and derived state calculations. The small array sizes make memoization unnecessary here.
76-115: LGTM!Clean async handling with appropriate toast feedback for both success and error cases. The
handleOpenChangecorrectly resets pagination state when the dropdown closes.
117-139: LGTM!The
classNamehandling correctly filters falsy values to prevent"undefined"from appearing in the DOM (addressing the previous review feedback). Accessibility is well-handled witharia-labeland proper disabled states.
158-244: LGTM!Dropdown content is well-structured with proper conditional rendering, pagination logic, and accessibility (
type="button"on all buttons, disabled states during mutations). The "Show more" count calculation is correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (1)
165-251: Consider using semantic menu items for keyboard navigation.The dropdown uses native
<button>elements instead ofDropdownMenuItemcomponents. While this enables custom styling, it loses built-in keyboard navigation (arrow keys to move between items). If accessibility is a priority, consider usingDropdownMenuItemwith custom styling or implementingrole="menuitem"with proper focus management.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (7)
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
useCreateWorkspace(12-59)apps/desktop/src/renderer/react-query/projects/useOpenNew.ts (1)
useOpenNew(7-22)packages/ui/src/components/sonner.tsx (1)
toast(30-30)packages/ui/src/components/button-group.tsx (1)
ButtonGroup(17-17)packages/ui/src/components/tooltip.tsx (3)
Tooltip(74-74)TooltipTrigger(74-74)TooltipContent(74-74)packages/ui/src/components/button.tsx (1)
Button(61-61)packages/ui/src/components/dropdown-menu.tsx (3)
DropdownMenu(245-245)DropdownMenuTrigger(247-247)DropdownMenuContent(248-248)
🔇 Additional comments (6)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceDropdown.tsx (6)
1-19: LGTM!Imports are well-organized, use proper path aliases as per
tsconfig.json, and correctly avoid Node.js modules in renderer process code. tRPC usage aligns with coding guidelines.
21-49: LGTM!The
formatPathhelper is well-documented, handles cross-platform path normalization (Windows backslashes), properly escapes special regex characters, and provides a sensible fallback for tilde replacement whenhomeDiris unavailable.
55-74: LGTM!State management and data fetching are well-structured. The derived state computations (currentProject, otherProjects, visibleProjects) are clear and handle the edge cases appropriately. For small lists of recent projects, inline derivation is acceptable.
76-122: LGTM!Handler functions are well-structured with proper error handling via
toast.promiseandtoast.error. ThecloseDropdownhelper correctly resets pagination state, andhandleOpenChangeensures clean state transitions.
124-146: LGTM!The
ButtonGroupclassName handling correctly addresses the previous review feedback. The primary button has proper accessibility (aria-label), disables during pending operations, and provides context-aware tooltip content.
147-164: Verify nested Tooltip + DropdownMenuTrigger behavior.The nested
Tooltip > TooltipTrigger > DropdownMenuTrigger > Buttonpattern is complex. Ensure the tooltip dismisses properly when the dropdown opens and that ref forwarding works correctly across all layers.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.