Repository navigation
feat(desktop): make Pinned and Sessions sidebar sections collapsible like Projects - #6407
Merged
Merged
Conversation
…like Projects Extract the Projects header's collapse pattern into a shared DashboardSidebarSectionHeader (clickable strip, chevron that stays visible while collapsed) and generalize the persisted collapse store to cover pinned/sessions/workspaces, migrating the legacy sidebar-workspaces-collapse key to the new shape.
Contributor
📝 WalkthroughWalkthroughThe dashboard sidebar now uses one persisted Zustand store for pinned, sessions, and workspace collapse state. A shared accessible section header handles toggling. Section components hide their rows when collapsed while preserving actions and drop zones. ChangesSidebar collapse behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🟡 Moderate · up to The sidebar changes currently risk blocking keyboard and screen-reader access to section actions and can incorrectly hide Pinned and Sessions content, including the drag-to-pin target, when Projects is collapsed. These issues should be fixed before merging. Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardSidebarSectionHeader
participant useSidebarSectionsCollapseStore
participant SidebarSection
User->>DashboardSidebarSectionHeader: Click or press Enter/Space
DashboardSidebarSectionHeader->>useSidebarSectionsCollapseStore: Toggle section
useSidebarSectionsCollapseStore-->>SidebarSection: Updated collapsed state
SidebarSection-->>User: Show or hide section rows
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 31-54: The wrapper in DashboardSidebarSectionHeader must not
provide button semantics around the action children. Render the collapse
interaction as its own native button containing the label and chevron, add
aria-expanded reflecting isCollapsed, and keep children in a separate sibling
action area so DashboardSidebarSessionsSection and
DashboardSidebarWorkspacesHeader buttons remain independently accessible.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx`:
- Around line 136-137: Keep the sidebar section states independent in
DashboardSidebar: use workspacesListCollapsed only to control the Projects
sortable content, while always rendering the Pinned section when the sidebar is
expanded and determining Sessions row visibility from its own collapse state
rather than collapsed.workspaces. Preserve the empty Pinned drop zone during
workspace dragging.
In `@apps/desktop/src/renderer/stores/sidebar-sections-collapse.ts`:
- Around line 25-43: The persisted key configuration for sidebar-workspace
collapse must document its fixed-size bound, deletion owner or removal path, and
behavior when the feature is removed. Update the metadata around the
`sidebar-workspaces-collapse` entry accordingly, and move the key to `DEAD_KEYS`
when removing the feature.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fb5a62a-7dd8-4046-9b04-30450eea5663
📒 Files selected for processing (9)
apps/desktop/src/renderer/lib/persisted-keys/persisted-key-registry.test-data.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarPinnedSection/DashboardSidebarPinnedSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionHeader/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSessionsSection/DashboardSidebarSessionsSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspacesHeader/DashboardSidebarWorkspacesHeader.tsxapps/desktop/src/renderer/stores/sidebar-sections-collapse.tsapps/desktop/src/renderer/stores/sidebar-workspaces-collapse.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/stores/sidebar-workspaces-collapse.ts
Comment on lines
+31
to
+54
| // biome-ignore lint/a11y/useSemanticElements: can't be a native <button> — it may nest action buttons | ||
| <div | ||
| role="button" | ||
| tabIndex={0} | ||
| onClick={() => toggle(section)} | ||
| onKeyDown={(event) => { | ||
| if (event.key === "Enter" || event.key === " ") { | ||
| event.preventDefault(); | ||
| toggle(section); | ||
| } | ||
| }} | ||
| className="group flex min-h-8 w-full shrink-0 items-center gap-1.5 py-1.5 pl-4 pr-2 text-[10px] font-semibold uppercase tracking-[0.075em] text-muted-foreground transition-colors" | ||
| > | ||
| <span className="min-w-0 truncate text-left">{label}</span> | ||
| <HiChevronRight | ||
| className={cn( | ||
| "size-3 shrink-0 text-muted-foreground transition-[opacity,transform] duration-150", | ||
| isCollapsed | ||
| ? "opacity-100" | ||
| : "rotate-90 opacity-0 group-hover:opacity-100 group-focus-visible:opacity-100", | ||
| )} | ||
| /> | ||
| <div className="min-w-0 flex-1" /> | ||
| {children} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a separate collapse trigger.
Line 33 gives the wrapper button semantics. DashboardSidebarSessionsSection and DashboardSidebarWorkspacesHeader pass native buttons as children. Descendants of an ARIA button can become presentational to assistive technology, so users may not be able to access the section actions.
Render the collapse control as its own <button>. Keep the action area as a sibling. Set aria-expanded on the collapse button.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`
around lines 31 - 54, The wrapper in DashboardSidebarSectionHeader must not
provide button semantics around the action children. Render the collapse
interaction as its own native button containing the label and chevron, add
aria-expanded reflecting isCollapsed, and keep children in a separate sibling
action area so DashboardSidebarSessionsSection and
DashboardSidebarWorkspacesHeader buttons remain independently accessible.
Comment on lines
+136
to
+137
| const workspacesListCollapsed = useSidebarSectionsCollapseStore( | ||
| (s) => s.collapsed.workspaces, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the three section states independent.
After Line 136 changes this selector to the Projects-only flag, the conditions at Lines 257-267 still hide the Pinned section and Sessions rows when Projects is collapsed. This also removes the empty Pinned drop zone during a workspace drag.
Always render Pinned in the expanded sidebar. Do not derive Sessions row visibility from collapsed.workspaces. Use this flag only for the Projects sortable content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsx`
around lines 136 - 137, Keep the sidebar section states independent in
DashboardSidebar: use workspacesListCollapsed only to control the Projects
sortable content, while always rendering the Pinned section when the sidebar is
expanded and determining Sessions row visibility from its own collapse state
rather than collapsed.workspaces. Preserve the empty Pinned drop zone during
workspace dragging.
Comment on lines
+25
to
+43
| { | ||
| // Legacy key: v0 held only the workspaces-list flag as | ||
| // { isCollapsed }, before Pinned/Sessions became collapsible. | ||
| name: "sidebar-workspaces-collapse", | ||
| version: 1, | ||
| migrate: (persisted, version) => { | ||
| if (version === 0) { | ||
| const state = persisted as { isCollapsed?: boolean }; | ||
| return { | ||
| collapsed: { | ||
| pinned: false, | ||
| sessions: false, | ||
| workspaces: state.isCollapsed ?? false, | ||
| }, | ||
| }; | ||
| } | ||
| return persisted as SidebarSectionsCollapseState; | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the persisted key lifecycle.
This change expands sidebar-workspaces-collapse to store three section flags. Document its fixed-size bound, deletion owner or path, and the removal behavior. On feature removal, move the key to DEAD_KEYS.
As per coding guidelines: “For every renderer localStorage key, document its bound, deletion owner or path, and behavior when the feature is removed.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/stores/sidebar-sections-collapse.ts` around lines
25 - 43, The persisted key configuration for sidebar-workspace collapse must
document its fixed-size bound, deletion owner or removal path, and behavior when
the feature is removed. Update the metadata around the
`sidebar-workspaces-collapse` entry accordingly, and move the key to `DEAD_KEYS`
when removing the feature.
Source: Coding guidelines
Member
Author
|
Reviewed the CodeRabbit findings — proceeding without changes:
|
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Pinned and Sessions sidebar sections now collapse exactly like the Projects header: click the header strip to toggle, with the chevron cue that stays visible while collapsed (per the GH #6009 convention) so hidden rows read as hidden rather than missing. Collapse state persists per section.
Refactor
DashboardSidebarSectionHeaderrenders the top-level section header strip (label, chevron, right-aligned action slot);DashboardSidebarWorkspacesHeader(Projects),DashboardSidebarPinnedSection, andDashboardSidebarSessionsSectionall use it now.sidebar-workspaces-collapsestore generalized tosidebar-sections-collapseholdingcollapsed: { pinned, sessions, workspaces }. The legacy localStorage key is kept with a v0→v1persistmigration, so an already-collapsed projects list carries over; persisted-key registry updated.Behavior notes
Verification
Typecheck, lint, and persisted-key registry tests pass. Verified end-to-end over CDP against this worktree's dev app with trusted input: hover→chevron, collapse/expand each section, both-collapsed →
location.reload()→ still collapsed (store:{"collapsed":{"pinned":true,"sessions":true,"workspaces":false},"version":1}), console.error hooked throughout — zero entries. 27s screen recording captured.Summary by cubic
Makes Pinned and Sessions sidebar sections collapsible with the same UX as Projects. Previously only Projects collapsed; now each section toggles via its header, shows the chevron while collapsed, and remembers its own state.
Review notes
DashboardSidebarSectionHeaderand uses it for Projects, Pinned, and Sessions.useSidebarWorkspacesCollapseStorewithuseSidebarSectionsCollapseStoreconsumers.Persistence/migration
{ collapsed: { pinned, sessions, workspaces } }under the legacysidebar-workspaces-collapsekey with a v0→v1 migration that mapsisCollapsedtocollapsed.workspaces.Written for commit fd96fc3. Summary will update on new commits.
Summary by CodeRabbit