feat(desktop): move sidebar toggle to topbar with hotkey tooltip#1036
Conversation
Move the sidebar toggle button from WorkspaceSidebarHeader to the TopBar, positioning it after the traffic lights on Mac. The tooltip now displays the user's configured hotkey using HotkeyTooltipContent.
📝 WalkthroughWalkthroughThis PR refactors the sidebar toggle UI by extracting it from the WorkspaceSidebarHeader component into a new SidebarToggle component located in the TopBar, while reorganizing component imports and establishing barrel exports for better module accessibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 |
Reorganize TopBar to follow project structure guidelines: - Extract SidebarToggle to its own subcomponent folder - Move OpenInMenuButton, OrganizationDropdown, WindowControls to subfolders - Add barrel exports (index.ts) for each component - Rename index.tsx to TopBar.tsx with proper barrel export
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/TopBar/SidebarToggle/SidebarToggle.tsx`:
- Around line 28-37: The icon-only toggle button lacks accessible labeling and
state. Update the button element (the one using onClick={toggleCollapsed} and
rendering getToggleIcon(...)) to include an aria-label that changes based on the
collapsed state (e.g., "Collapse sidebar" vs "Expand sidebar") and set
aria-pressed (or aria-expanded) to reflect the current collapsed value; derive
the label/state from the component's collapsed/isCollapsed prop or state and
keep toggleCollapsed as the click handler so screen readers get both a
meaningful name and the current pressed/expanded state.
| <button | ||
| type="button" | ||
| onClick={toggleCollapsed} | ||
| className="no-drag group flex items-center justify-center size-8 rounded-md text-muted-foreground hover:text-foreground hover:bg-accent/50 transition-colors" | ||
| > | ||
| <span className="group-hover:hidden">{getToggleIcon(false)}</span> | ||
| <span className="hidden group-hover:block"> | ||
| {getToggleIcon(true)} | ||
| </span> | ||
| </button> |
There was a problem hiding this comment.
Add accessible labeling for the icon-only toggle.
Screen readers won’t have a usable name without an aria-label/pressed state.
✅ Suggested fix
<button
type="button"
onClick={toggleCollapsed}
+ aria-label="Toggle sidebar"
+ aria-pressed={collapsed}
className="no-drag group flex items-center justify-center size-8 rounded-md text-muted-foreground hover:text-foreground hover:bg-accent/50 transition-colors"
>🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/TopBar/SidebarToggle/SidebarToggle.tsx`
around lines 28 - 37, The icon-only toggle button lacks accessible labeling and
state. Update the button element (the one using onClick={toggleCollapsed} and
rendering getToggleIcon(...)) to include an aria-label that changes based on the
collapsed state (e.g., "Collapse sidebar" vs "Expand sidebar") and set
aria-pressed (or aria-expanded) to reflect the current collapsed value; derive
the label/state from the component's collapsed/isCollapsed prop or state and
keep toggleCollapsed as the click handler so screen readers get both a
meaningful name and the current pressed/expanded state.
Move TopBar from screens/main/components to routes/_authenticated/_dashboard/components to properly co-locate it next to the layout that uses it. Update import paths accordingly.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.