feat(ui): unified sidebar with collapsible sections and enhanced UX #659
Conversation
Add new unified-sidebar component for layout improvements. - Export UnifiedSidebar from layout components - Update root route to use new sidebar structure
Merge the unified-sidebar implementation into the standard sidebar folder structure. The unified sidebar becomes the canonical sidebar with improved features including collapsible sections, scroll indicators, and enhanced mobile support. - Delete old sidebar.tsx - Move unified-sidebar components to sidebar/components - Rename UnifiedSidebar to Sidebar - Update all imports in __root.tsx - Remove redundant unified-sidebar folder
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors sidebar UI: adds collapsible sections, project dropdowns and keyboard switching, context menu/edit dialogs, scroll tracking, expanded footer with docs/feedback, minor positioning tweak, test selector updates, and related type/exports adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant SidebarHeader
participant Sidebar (state)
participant Router
participant ContentArea
User->>SidebarHeader: Click project in dropdown / press numeric key
SidebarHeader->>Sidebar (state): request switchProject(projectId)
Sidebar (state)->>Router: navigate("/board", { projectId })
Router->>ContentArea: render board for projectId
Sidebar (state)->>SidebarHeader: update currentProject state
SidebarHeader-->>User: close dropdown / show selected project
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
stefandevo
added
the
Testers-Requested
Summary of ChangesHello @stefandevo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significantly revamped user interface for the application's sidebar, transforming it into a unified and highly functional navigation hub. The changes focus on enhancing user experience through collapsible sections, improved project management workflows, and better responsiveness across different screen sizes. This refactor streamlines the navigation, making it more intuitive and efficient for users to interact with projects and tools, while also improving the underlying code structure. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx`:
- Around line 53-56: The call to getElectronAPI() inside handleFeedbackClick can
throw in SSR or non-Electron environments; update handleFeedbackClick to guard
against that by wrapping the call in a try/catch or checking existence before
using it, e.g., attempt to get the API and if it's falsy or throws, gracefully
fallback (no-op or window.open) instead of calling api.openExternalLink;
reference the handleFeedbackClick function, getElectronAPI(), and
api.openExternalLink when locating and changing the code.
In `@apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx`:
- Around line 202-204: The hotkey hint in the SidebarHeader component is using a
hardcoded "⌘" which is not OS-aware; replace it by importing and using
formatShortcut (from `@/store/app-store`) to render the modifier symbol for
hotkeyLabel (e.g., call formatShortcut('Cmd' or appropriate modifier) and render
that result instead of the literal "⌘{hotkeyLabel}"). Update both occurrences
referenced (the span rendering hotkeyLabel and the similar rendering at the
other location) so they call formatShortcut with the correct modifier and
concatenate the returned symbol with hotkeyLabel.
🧹 Nitpick comments (4)
apps/ui/src/components/layout/sidebar/hooks/use-navigation.ts (1)
177-202: Consider adding a keyboard shortcut for the Dashboard item.The Dashboard navigation item lacks a
shortcutproperty, while other navigation items have shortcuts defined. This means users cannot navigate to the Dashboard using keyboard shortcuts.If this is intentional (e.g., Dashboard is always accessible via the sidebar), this is fine. Otherwise, consider adding a shortcut for consistency with other navigation items.
💡 Suggested change
// Dashboard - standalone at top { label: '', items: [ { id: 'dashboard', label: 'Dashboard', icon: Home, + shortcut: shortcuts.dashboard, // Add to UseNavigationProps.shortcuts }, ], },apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsx (2)
102-105: Redundant ternary expression.Both branches of the ternary produce the same value
'mt-1', making the conditional pointless.Proposed fix
className={cn( 'flex-1 overflow-y-auto scrollbar-hide px-3 pb-2', - sidebarOpen ? 'mt-1' : 'mt-1' + 'mt-1' )}
299-319: Consider using the Tooltip component for consistency.This custom span-based tooltip differs from the
Tooltipcomponent used elsewhere in the sidebar (e.g., in the collapsed dropdown section above and insidebar-footer.tsx). Using the same Tooltip component would ensure consistent behavior, animations, and accessibility.apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx (1)
51-56: Minor: Unnecessary optional chaining.The function signature indicates
project: Projectis always defined, but line 52 usesproject?.icon. SinceProjectis not nullable here, the optional chaining is unnecessary.Proposed fix
const getIconComponent = (project: Project): LucideIcon => { - if (project?.icon && project.icon in LucideIcons) { + if (project.icon && project.icon in LucideIcons) { return (LucideIcons as unknown as Record<string, LucideIcon>)[project.icon]; } return Folder; };
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a unified sidebar with collapsible sections, enhanced project management features, and improved mobile responsiveness. The changes are extensive, refactoring several components to support the new design and functionality. Key improvements include dynamic project switching, integrated project creation/opening, and a scroll indicator for navigation. Overall, the changes significantly enhance the user experience and modularity of the sidebar.
Style Guide Adherence Summary
No custom style guide was provided, so the review adheres to commonly accepted best practices for TypeScript/React projects. Several minor inconsistencies in UI component usage and styling were identified, which could be improved for better maintainability and consistency.
| <div className="flex flex-col items-center py-2 px-2 gap-1"> | ||
| {/* Running Agents */} | ||
| {!hideRunningAgents && ( | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={() => navigate({ to: '/running-agents' })} | ||
| className={cn( | ||
| 'relative flex items-center justify-center w-10 h-10 rounded-xl', | ||
| 'transition-all duration-200 ease-out titlebar-no-drag', | ||
| isActiveRoute('running-agents') | ||
| ? [ | ||
| 'bg-gradient-to-r from-brand-500/20 via-brand-500/15 to-brand-600/10', | ||
| 'text-foreground border border-brand-500/30', | ||
| 'shadow-md shadow-brand-500/10', | ||
| ] | ||
| : [ | ||
| 'text-muted-foreground hover:text-foreground', | ||
| 'hover:bg-accent/50 border border-transparent hover:border-border/40', | ||
| ] | ||
| )} | ||
| data-testid="running-agents-link" | ||
| > | ||
| <Activity | ||
| className={cn( | ||
| 'w-[18px] h-[18px]', | ||
| isActiveRoute('running-agents') && 'text-brand-500' | ||
| )} | ||
| /> | ||
| {runningAgentsCount > 0 && ( | ||
| <span | ||
| className={cn( | ||
| 'absolute -top-1 -right-1 flex items-center justify-center', | ||
| 'min-w-4 h-4 px-1 text-[9px] font-bold rounded-full', | ||
| 'bg-brand-500 text-white shadow-sm' | ||
| )} | ||
| > | ||
| {runningAgentsCount > 99 ? '99' : runningAgentsCount} | ||
| </span> | ||
| )} | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| Running Agents | ||
| {runningAgentsCount > 0 && ( | ||
| <span className="ml-2 px-1.5 py-0.5 bg-brand-500 text-white rounded-full text-[10px]"> | ||
| {runningAgentsCount} | ||
| </span> | ||
| )} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} | ||
|
|
||
| {/* Settings */} | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={() => navigate({ to: '/settings' })} | ||
| className={cn( | ||
| 'flex items-center justify-center w-10 h-10 rounded-xl', | ||
| 'transition-all duration-200 ease-out titlebar-no-drag', | ||
| isActiveRoute('settings') | ||
| ? [ | ||
| 'bg-gradient-to-r from-brand-500/20 via-brand-500/15 to-brand-600/10', | ||
| 'text-foreground border border-brand-500/30', | ||
| 'shadow-md shadow-brand-500/10', | ||
| ] | ||
| : [ | ||
| 'text-muted-foreground hover:text-foreground', | ||
| 'hover:bg-accent/50 border border-transparent hover:border-border/40', | ||
| ] | ||
| )} | ||
| data-testid="settings-button" | ||
| > | ||
| <Settings | ||
| className={cn( | ||
| 'w-[18px] h-[18px]', | ||
| isActiveRoute('settings') && 'text-brand-500' | ||
| )} | ||
| /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| Global Settings | ||
| <span className="ml-2 px-1.5 py-0.5 bg-muted rounded text-[10px] font-mono text-muted-foreground"> | ||
| {formatShortcut(shortcuts.settings, true)} | ||
| </span> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
|
|
||
| {/* Documentation */} | ||
| {!hideWiki && ( | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={handleWikiClick} | ||
| className={cn( | ||
| 'flex items-center justify-center w-10 h-10 rounded-xl', | ||
| 'text-muted-foreground hover:text-foreground', | ||
| 'hover:bg-accent/50 border border-transparent hover:border-border/40', | ||
| 'transition-all duration-200 ease-out titlebar-no-drag' | ||
| )} | ||
| data-testid="documentation-button" | ||
| > | ||
| <BookOpen className="w-[18px] h-[18px]" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| Documentation | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} | ||
|
|
||
| {/* Feedback */} | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={handleFeedbackClick} | ||
| className={cn( | ||
| 'flex items-center justify-center w-10 h-10 rounded-xl', | ||
| 'text-muted-foreground hover:text-foreground', | ||
| 'hover:bg-accent/50 border border-transparent hover:border-border/40', | ||
| 'transition-all duration-200 ease-out titlebar-no-drag' | ||
| )} | ||
| data-testid="feedback-button" | ||
| > | ||
| <MessageSquare className="w-[18px] h-[18px]" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| Feedback | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
In the collapsed state, each button (Running Agents, Settings, Documentation, Feedback) is wrapped in its own TooltipProvider. This is redundant and can impact performance slightly due to unnecessary context creation. A single TooltipProvider can wrap the entire div containing these buttons.
Consider wrapping the entire div (lines 67-209) with one TooltipProvider to optimize performance and simplify the component structure.
<div className="flex flex-col items-center py-2 px-2 gap-1">
<TooltipProvider delayDuration={0}>
{/* Running Agents */}
{!hideRunningAgents && (
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={() => navigate({ to: '/running-agents' })}
className={cn(
'relative flex items-center justify-center w-10 h-10 rounded-xl',
'transition-all duration-200 ease-out titlebar-no-drag',
isActiveRoute('running-agents')
? [
'bg-gradient-to-r from-brand-500/20 via-brand-500/15 to-brand-600/10',
'text-foreground border border-brand-500/30',
'shadow-md shadow-brand-500/10',
]
: [
'text-muted-foreground hover:text-foreground',
'hover:bg-accent/50 border border-transparent hover:border-border/40',
]
)}
data-testid="running-agents-link"
>
<Activity
className={cn(
'w-[18px] h-[18px]',
isActiveRoute('running-agents') && 'text-brand-500'
)}
/>
{runningAgentsCount > 0 && (
<span
className={cn(
'absolute -top-1 -right-1 flex items-center justify-center',
'min-w-4 h-4 px-1 text-[9px] font-bold rounded-full',
'bg-brand-500 text-white shadow-sm'
)}
>
{runningAgentsCount > 99 ? '99' : runningAgentsCount}
</span>
)}
</button>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
Running Agents
{runningAgentsCount > 0 && (
<span className="ml-2 px-1.5 py-0.5 bg-brand-500 text-white rounded-full text-[10px]">
{runningAgentsCount}
</span>
)}
</TooltipContent>
</Tooltip>
)}
{/* Settings */}
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={() => navigate({ to: '/settings' })}
className={cn(
'flex items-center justify-center w-10 h-10 rounded-xl',
'transition-all duration-200 ease-out titlebar-no-drag',
isActiveRoute('settings')
? [
'bg-gradient-to-r from-brand-500/20 via-brand-500/15 to-brand-600/10',
'text-foreground border border-brand-500/30',
'shadow-md shadow-brand-500/10',
]
: [
'text-muted-foreground hover:text-foreground',
'hover:bg-accent/50 border border-transparent hover:border-border/40',
]
)}
data-testid="settings-button"
>
<Settings
className={cn(
'w-[18px] h-[18px]',
isActiveRoute('settings') && 'text-brand-500'
)}
/>
</button>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
Global Settings
<span className="ml-2 px-1.5 py-0.5 bg-muted rounded text-[10px] font-mono text-muted-foreground">
{formatShortcut(shortcuts.settings, true)}
</span>
</TooltipContent>
</Tooltip>
{/* Documentation */}
{!hideWiki && (
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={handleWikiClick}
className={cn(
'flex items-center justify-center w-10 h-10 rounded-xl',
'text-muted-foreground hover:text-foreground',
'hover:bg-accent/50 border border-transparent hover:border-border/40',
'transition-all duration-200 ease-out titlebar-no-drag'
)}
data-testid="documentation-button"
>
<BookOpen className="w-[18px] h-[18px]" />
</button>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
Documentation
</TooltipContent>
</Tooltip>
)}
{/* Feedback */}
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={handleFeedbackClick}
className={cn(
'flex items-center justify-center w-10 h-10 rounded-xl',
'text-muted-foreground hover:text-foreground',
'hover:bg-accent/50 border border-transparent hover:border-border/40',
'transition-all duration-200 ease-out titlebar-no-drag'
)}
data-testid="feedback-button"
>
<MessageSquare className="w-[18px] h-[18px]" />
</button>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
Feedback
</TooltipContent>
</Tooltip>
</TooltipProvider>
</div>
| {!hideWiki && ( | ||
| <div className="px-3 py-0.5"> | ||
| <button | ||
| onClick={handleWikiClick} | ||
| className={cn( | ||
| 'group flex items-center w-full px-3 py-1.5 rounded-md titlebar-no-drag', | ||
| 'text-muted-foreground/70 hover:text-foreground', | ||
| 'hover:bg-accent/30', | ||
| 'transition-all duration-200 ease-out' | ||
| )} | ||
| data-testid="documentation-button" | ||
| > | ||
| <BookOpen className="w-4 h-4 shrink-0" /> | ||
| <span className="ml-2.5 text-xs">Documentation</span> | ||
| </button> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Feedback Link */} | ||
| <div className="px-3 pt-0.5"> | ||
| <button | ||
| onClick={handleFeedbackClick} | ||
| className={cn( | ||
| 'group flex items-center w-full px-3 py-1.5 rounded-md titlebar-no-drag', | ||
| 'text-muted-foreground/70 hover:text-foreground', | ||
| 'hover:bg-accent/30', | ||
| 'transition-all duration-200 ease-out' | ||
| )} | ||
| data-testid="feedback-button" | ||
| > | ||
| <MessageSquare className="w-4 h-4 shrink-0" /> | ||
| <span className="ml-2.5 text-xs">Feedback</span> | ||
| <ExternalLink className="w-3 h-3 ml-auto text-muted-foreground/50" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
The padding for the "Documentation Link" (py-0.5) and "Feedback Link" (pt-0.5) in the expanded state is inconsistent. For better visual alignment and consistency, consider using the same vertical padding for both, e.g., py-0.5 for both or py-1.5 for both if that's the desired spacing.
{/* Documentation Link */}
{!hideWiki && (
<div className="px-3 py-0.5">
<button
onClick={handleWikiClick}
className={cn(
'group flex items-center w-full px-3 py-1.5 rounded-md titlebar-no-drag',
'text-muted-foreground/70 hover:text-foreground',
'hover:bg-accent/30',
'transition-all duration-200 ease-out'
)}
data-testid="documentation-button"
>
<BookOpen className="w-4 h-4 shrink-0" />
<span className="ml-2.5 text-xs">Documentation</span>
</button>
</div>
)}
{/* Feedback Link */}
<div className="px-3 py-1.5">
<button
onClick={handleFeedbackClick}
className={cn(
'group flex items-center w-full px-3 py-1.5 rounded-md titlebar-no-drag',
'text-muted-foreground/70 hover:text-foreground',
'hover:bg-accent/30',
'transition-all duration-200 ease-out'
)}
data-testid="feedback-button"
>
<MessageSquare className="w-4 h-4 shrink-0" />
<span className="ml-2.5 text-xs">Feedback</span>
<ExternalLink className="w-3 h-3 ml-auto text-muted-foreground/50" />
</button>
</div>
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={handleLogoClick} | ||
| className="group flex flex-col items-center" | ||
| data-testid="logo-button" | ||
| > | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 256 256" | ||
| role="img" | ||
| aria-label="Automaker Logo" | ||
| className="size-8 group-hover:rotate-12 transition-transform duration-300 ease-out" | ||
| > | ||
| <defs> | ||
| <linearGradient | ||
| id="bg-collapsed" | ||
| x1="0" | ||
| y1="0" | ||
| x2="256" | ||
| y2="256" | ||
| gradientUnits="userSpaceOnUse" | ||
| > | ||
| <stop offset="0%" style={{ stopColor: 'var(--brand-400)' }} /> | ||
| <stop offset="100%" style={{ stopColor: 'var(--brand-600)' }} /> | ||
| </linearGradient> | ||
| </defs> | ||
| <rect x="16" y="16" width="224" height="224" rx="56" fill="url(#bg-collapsed)" /> | ||
| <g | ||
| fill="none" | ||
| stroke="#FFFFFF" | ||
| strokeWidth="20" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| > | ||
| <path d="M92 92 L52 128 L92 164" /> | ||
| <path d="M144 72 L116 184" /> | ||
| <path d="M164 92 L204 128 L164 164" /> | ||
| </g> | ||
| </svg> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| Go to Dashboard | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
|
|
||
| {/* Collapsed project icon with dropdown */} | ||
| {currentProject && ( | ||
| <> | ||
| <div className="w-full h-px bg-border/40 my-2" /> | ||
| <DropdownMenu open={dropdownOpen} onOpenChange={setDropdownOpen}> | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <DropdownMenuTrigger asChild> | ||
| <button | ||
| onContextMenu={(e) => onProjectContextMenu(currentProject, e)} | ||
| className="p-1 rounded-lg hover:bg-accent/50 transition-colors" | ||
| data-testid="collapsed-project-button" | ||
| > | ||
| {renderProjectIcon(currentProject)} | ||
| </button> | ||
| </DropdownMenuTrigger> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| {currentProject.name} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| <DropdownMenuContent | ||
| align="start" | ||
| side="right" | ||
| sideOffset={8} | ||
| className="w-64" | ||
| data-testid="collapsed-project-dropdown-content" | ||
| > | ||
| <div className="px-2 py-1.5"> | ||
| <span className="text-xs font-medium text-muted-foreground">Projects</span> | ||
| </div> | ||
| {projects.map((project, index) => { | ||
| const isActive = currentProject?.id === project.id; | ||
| const hotkeyLabel = index < 9 ? `${index + 1}` : index === 9 ? '0' : undefined; | ||
|
|
||
| return ( | ||
| <DropdownMenuItem | ||
| key={project.id} | ||
| onClick={() => handleProjectSelect(project)} | ||
| onContextMenu={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| setDropdownOpen(false); | ||
| onProjectContextMenu(project, e); | ||
| }} | ||
| className="flex items-center gap-3 cursor-pointer" | ||
| data-testid={`collapsed-project-item-${project.id}`} | ||
| > | ||
| {renderProjectIcon(project, 'sm')} | ||
| <span | ||
| className={cn( | ||
| 'flex-1 truncate', | ||
| isActive && 'font-semibold text-foreground' | ||
| )} | ||
| > | ||
| {project.name} | ||
| </span> | ||
| {hotkeyLabel && ( | ||
| <span className="text-xs text-muted-foreground">⌘{hotkeyLabel}</span> | ||
| )} | ||
| </DropdownMenuItem> | ||
| ); | ||
| })} | ||
| <DropdownMenuSeparator /> | ||
| <DropdownMenuItem | ||
| onClick={() => { | ||
| setDropdownOpen(false); | ||
| onNewProject(); | ||
| }} | ||
| className="cursor-pointer" | ||
| data-testid="collapsed-new-project-dropdown-item" | ||
| > | ||
| <Plus className="w-4 h-4 mr-2" /> | ||
| <span>New Project</span> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem | ||
| onClick={() => { | ||
| setDropdownOpen(false); | ||
| onOpenFolder(); | ||
| }} | ||
| className="cursor-pointer" | ||
| data-testid="collapsed-open-project-dropdown-item" | ||
| > | ||
| <FolderOpen className="w-4 h-4 mr-2" /> | ||
| <span>Open Project</span> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Similar to the sidebar footer, the TooltipProvider is unnecessarily duplicated around the Tooltip components within the collapsed state's project section. A single TooltipProvider can wrap the entire div containing the DropdownMenu for better performance and cleaner code.
<TooltipProvider delayDuration={0}>
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={handleLogoClick}
className="group flex flex-col items-center"
data-testid="logo-button"
>
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 256 256"
role="img"
aria-label="Automaker Logo"
className="size-8 group-hover:rotate-12 transition-transform duration-300 ease-out"
>
<defs>
<linearGradient
id="bg-collapsed"
x1="0"
y1="0"
x2="256"
y2="256"
gradientUnits="userSpaceOnUse"
>
<stop offset="0%" style={{ stopColor: 'var(--brand-400)' }} />
<stop offset="100%" style={{ stopColor: 'var(--brand-600)' }} />
</linearGradient>
</defs>
<rect x="16" y="16" width="224" height="224" rx="56" fill="url(#bg-collapsed)" />
<g
fill="none"
stroke="#FFFFFF"
strokeWidth="20"
strokeLinecap="round"
strokeLinejoin="round"
>
<path d="M92 92 L52 128 L92 164" />
<path d="M144 72 L116 184" />
<path d="M164 92 L204 128 L164 164" />
</g>
</svg>
</button>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
Go to Dashboard
</TooltipContent>
</Tooltip>
{/* Collapsed project icon with dropdown */}
{currentProject && (
<>
<div className="w-full h-px bg-border/40 my-2" />
<DropdownMenu open={dropdownOpen} onOpenChange={setDropdownOpen}>
<Tooltip>
<TooltipTrigger asChild>
<DropdownMenuTrigger asChild>
<button
onContextMenu={(e) => onProjectContextMenu(currentProject, e)}
className="p-1 rounded-lg hover:bg-accent/50 transition-colors"
data-testid="collapsed-project-button"
>
{renderProjectIcon(currentProject)}
</button>
</DropdownMenuTrigger>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
{currentProject.name}
</TooltipContent>
</Tooltip>
<DropdownMenuContent
align="start"
side="right"
sideOffset={8}
className="w-64"
data-testid="collapsed-project-dropdown-content"
>
<div className="px-2 py-1.5">
<span className="text-xs font-medium text-muted-foreground">Projects</span>
</div>
{projects.map((project, index) => {
const isActive = currentProject?.id === project.id;
const hotkeyLabel = index < 9 ? `${index + 1}` : index === 9 ? '0' : undefined;
return (
<DropdownMenuItem
key={project.id}
onClick={() => handleProjectSelect(project)}
onContextMenu={(e) => {
e.preventDefault();
e.stopPropagation();
setDropdownOpen(false);
onProjectContextMenu(project, e);
}}
className="flex items-center gap-3 cursor-pointer"
data-testid={`collapsed-project-item-${project.id}`}
>
{renderProjectIcon(project, 'sm')}
<span
className={cn(
'flex-1 truncate',
isActive && 'font-semibold text-foreground'
)}
>
{project.name}
</span>
{hotkeyLabel && (
<span className="text-xs text-muted-foreground">⌘{hotkeyLabel}</span>
)}
</DropdownMenuItem>
);
})}
<DropdownMenuSeparator />
<DropdownMenuItem
onClick={() => {
setDropdownOpen(false);
onNewProject();
}}
className="cursor-pointer"
data-testid="collapsed-new-project-dropdown-item"
>
<Plus className="w-4 h-4 mr-2" />
<span>New Project</span>
</DropdownMenuItem>
<DropdownMenuItem
onClick={() => {
setDropdownOpen(false);
onOpenFolder();
}}
className="cursor-pointer"
data-testid="collapsed-open-project-dropdown-item"
>
<FolderOpen className="w-4 h-4 mr-2" />
<span>Open Project</span>
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
</>
)}
</TooltipProvider>
| {section.label && !sidebarOpen && SectionIcon && section.collapsible && isCollapsed && ( | ||
| <DropdownMenu> | ||
| <TooltipProvider delayDuration={0}> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <DropdownMenuTrigger asChild> | ||
| <button | ||
| className={cn( | ||
| 'group flex items-center justify-center w-full py-2 rounded-lg', | ||
| 'text-muted-foreground hover:text-foreground', | ||
| 'hover:bg-accent/50 border border-transparent hover:border-border/40', | ||
| 'transition-all duration-200 ease-out' | ||
| )} | ||
| > | ||
| <SectionIcon className="w-[18px] h-[18px]" /> | ||
| </button> | ||
| </DropdownMenuTrigger> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right" sideOffset={8}> | ||
| {section.label} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| <DropdownMenuContent side="right" align="start" sideOffset={8} className="w-48"> | ||
| {section.items.map((item) => { | ||
| const ItemIcon = item.icon; | ||
| return ( | ||
| <DropdownMenuItem | ||
| key={item.id} | ||
| onClick={() => navigate({ to: `/${item.id}` as unknown as '/' })} | ||
| className="flex items-center gap-2 cursor-pointer" | ||
| > | ||
| <ItemIcon className="w-4 h-4" /> | ||
| <span>{item.label}</span> | ||
| {item.shortcut && ( | ||
| <span className="ml-auto text-[10px] font-mono text-muted-foreground"> | ||
| {formatShortcut(item.shortcut, true)} | ||
| </span> | ||
| )} | ||
| </DropdownMenuItem> | ||
| ); | ||
| })} | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| )} |
There was a problem hiding this comment.
In the collapsed state, the DropdownMenu for section icons is wrapped in its own TooltipProvider. For better performance and cleaner code, a single TooltipProvider can wrap the entire DropdownMenu component.
{/* Section icon with dropdown (collapsed sidebar) */}
{section.label && !sidebarOpen && SectionIcon && section.collapsible && isCollapsed && (
<TooltipProvider delayDuration={0}>
<DropdownMenu>
<Tooltip>
<TooltipTrigger asChild>
<DropdownMenuTrigger asChild>
<button
className={cn(
'group flex items-center justify-center w-full py-2 rounded-lg',
'text-muted-foreground hover:text-foreground',
'hover:bg-accent/50 border border-transparent hover:border-border/40',
'transition-all duration-200 ease-out'
)}
>
<SectionIcon className="w-[18px] h-[18px]" />
</button>
</DropdownMenuTrigger>
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
{section.label}
</TooltipContent>
</Tooltip>
<DropdownMenuContent side="right" align="start" sideOffset={8} className="w-48">
{section.items.map((item) => {
const ItemIcon = item.icon;
return (
<DropdownMenuItem
key={item.id}
onClick={() => navigate({ to: `/${item.id}` as unknown as '/' })}
className="flex items-center gap-2 cursor-pointer"
>
<ItemIcon className="w-4 h-4" />
<span>{item.label}</span>
{item.shortcut && (
<span className="ml-auto text-[10px] font-mono text-muted-foreground">
{formatShortcut(item.shortcut, true)}
</span>
)}
</DropdownMenuItem>
);
})}
</DropdownMenuContent>
</DropdownMenu>
</TooltipProvider>
)}
| 'min-w-4 h-4 px-0.5 text-[9px] font-bold rounded-full', | ||
| 'bg-primary text-primary-foreground shadow-sm', | ||
| 'animate-in fade-in zoom-in duration-200' | ||
| )} | ||
| > | ||
| {item.count > 99 ? '99' : item.count} | ||
| </span> |
There was a problem hiding this comment.
The px-0.5 padding for the collapsed count badge is inconsistent with the px-1 used in sidebar-footer.tsx for a similar badge. For better visual consistency across the sidebar, consider unifying this padding to px-1.
<span
className={cn(
'absolute -top-1.5 -right-1.5 flex items-center justify-center',
'min-w-4 h-4 px-1 text-[9px] font-bold rounded-full',
'bg-primary text-primary-foreground shadow-sm',
'animate-in fade-in zoom-in duration-200'
)}
>
{item.count > 99 ? '99' : item.count}
</span>
| // Auto-collapse sidebar on small screens | ||
| useSidebarAutoCollapse({ sidebarOpen, toggleSidebar }); |
There was a problem hiding this comment.
The comment for useSidebarAutoCollapse states "Auto-collapse sidebar on small screens", but the hook itself (in use-sidebar-auto-collapse.ts) still includes logic to "Update Electron window minWidth". The comment in sidebar.tsx should accurately reflect all functionalities of the hook for clarity.
| // Auto-collapse sidebar on small screens | |
| useSidebarAutoCollapse({ sidebarOpen, toggleSidebar }); | |
| // Auto-collapse sidebar on small screens and update Electron window minWidth | |
| useSidebarAutoCollapse({ sidebarOpen, toggleSidebar }); |
…ebar - Add try/catch for getElectronAPI() in sidebar-footer with window.open fallback - Use formatShortcut() for OS-aware hotkey display in sidebar-header - Remove unnecessary optional chaining on project.icon - Remove redundant ternary in sidebar-navigation className - Update E2E tests to use new project-dropdown-trigger data-testid Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Features
Test plan
Summary by CodeRabbit
New Features
UI/UX Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.