fix(desktop): collapse v2 sidebar count + actions into one slot#3842
Merged
Conversation
In the project row and section header, the workspace count sat next to the title and the action button (new-workspace "+" / section-actions ellipsis) lived as a separate end-aligned control. The actions were hidden until hover, but the count was always visible — so the right edge had two competing elements at different times. Put count and action in the same slot at the row's end: count visible at rest, action overlays it on hover/focus. Drops the parens around the count and shrinks the project-row count from text-xs to text-[10px] to match the section header. Builds on the v2 sidebar work in #3813 (cc @saddlepaddle).
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves workspace counts into fixed-size containers; makes "New workspace" and section header actions absolute overlay elements that reveal on hover/focus, stop event propagation, and include accessibility attributes; adds optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Greptile SummaryThis PR tightens the right edge of v2 sidebar rows by collapsing the workspace count and action button into a single overlay slot — count visible at rest, action fades in on hover/focus. The project row implementation is clean and consistent; the section header has one minor edge case where the count fades out on hover even when no Confidence Score: 5/5Safe to merge; the only finding is a minor visual glitch when All findings are P2 — a cosmetic UX inconsistency (count disappears on hover with nothing replacing it) that only manifests when DashboardSidebarSectionHeader.tsx — count fade classes should be conditioned on
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx | Consolidates workspace count and "+" button into a single 24 px overlay slot; adds aria-label to the button and correctly gates fade classes since the button is always present alongside the count. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx | Same overlay pattern applied to the section header, but the count fade classes are applied unconditionally even when actions is absent, causing the count to vanish on hover with nothing to replace it. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Sidebar Row Rendered] --> B{isRenaming?}
B -- yes --> C[Show RenameInput only]
B -- no --> D[Show name + overlay slot]
D --> E{Hover / focus-within?}
E -- no --> F[Count visible\nButton opacity-0]
E -- yes --> G{actions present?}
G -- yes --> H[Count opacity-0\nAction button fades in]
G -- no --> I[Count opacity-0\nEmpty slot ⚠️]
H --> J[User clicks action]
J --> K[stopPropagation\nfire callback]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
Line: 93
Comment:
**Count disappears with nothing to replace it when `actions` is absent**
The count `<span>` carries `group-hover:opacity-0 group-focus-within:opacity-0` unconditionally, so on hover the count fades out regardless of whether `actions` is truthy. When `actions` is `undefined`/`null` the overlay `<div>` is never rendered, so nothing takes the count's place — the slot becomes an empty 20 px ghost on every hover. Consider gating the fade classes on `!!actions`:
```suggestion
<span className={cn("text-[10px] font-normal tabular-nums transition-opacity", actions && "group-hover:opacity-0 group-focus-within:opacity-0")}>
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): collapse v2 sidebar count ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {actions} | ||
| {!isRenaming && ( | ||
| <div className="relative ml-1 flex size-5 shrink-0 items-center justify-center"> | ||
| <span className="text-[10px] font-normal tabular-nums transition-opacity group-hover:opacity-0 group-focus-within:opacity-0"> |
There was a problem hiding this comment.
actions is absent
The count <span> carries group-hover:opacity-0 group-focus-within:opacity-0 unconditionally, so on hover the count fades out regardless of whether actions is truthy. When actions is undefined/null the overlay <div> is never rendered, so nothing takes the count's place — the slot becomes an empty 20 px ghost on every hover. Consider gating the fade classes on !!actions:
Suggested change
| <span className="text-[10px] font-normal tabular-nums transition-opacity group-hover:opacity-0 group-focus-within:opacity-0"> | |
| <span className={cn("text-[10px] font-normal tabular-nums transition-opacity", actions && "group-hover:opacity-0 group-focus-within:opacity-0")}> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
Line: 93
Comment:
**Count disappears with nothing to replace it when `actions` is absent**
The count `<span>` carries `group-hover:opacity-0 group-focus-within:opacity-0` unconditionally, so on hover the count fades out regardless of whether `actions` is truthy. When `actions` is `undefined`/`null` the overlay `<div>` is never rendered, so nothing takes the count's place — the slot becomes an empty 20 px ghost on every hover. Consider gating the fade classes on `!!actions`:
```suggestion
<span className={cn("text-[10px] font-normal tabular-nums transition-opacity", actions && "group-hover:opacity-0 group-focus-within:opacity-0")}>
```
How can I resolve this? If you propose a fix, please make it concise.![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx`:
- Around line 108-117: The nested "New workspace" button in
DashboardSidebarProjectRow allows keyboard events to bubble up and trigger the
parent row's toggle handler; add a key-event stop to the button so Enter/Space
won't reach the parent. In the button element inside DashboardSidebarProjectRow
(the onClick/onContextMenu button), add an onKeyDown handler that calls
event.stopPropagation() (and optionally onKeyUp/onKeyPress if you want extra
safety) so keyboard activation still triggers the button’s own onClick/onKey
handlers (onNewWorkspace) but does not bubble to the row toggle.
🪄 Autofix (Beta)
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
Run ID: f141cd6d-f498-4c6a-9e90-a5fe427b6b37
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx:114">
P2: Missing `onKeyDown` stopPropagation on this button. Keyboard activation (Enter/Space) fires `keydown` before synthesizing `click`, so the parent row's `keydown` toggle handler will still fire even though `onClick` calls `stopPropagation()`. Add `onKeyDown={(event) => event.stopPropagation()}` to prevent the row from collapsing when the button is keyboard-activated.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx:116">
P2: The hidden "+" overlay remains clickable at rest because `opacity-0` does not disable pointer events, so clicking the count area can create a workspace unexpectedly.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx:93">
P2: The count fades out on hover unconditionally, but the overlay that replaces it only renders when `actions` is truthy. When `actions` is absent, hovering will hide the count with nothing taking its place. Gate the fade classes on whether `actions` exists.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Sections sat at pl-0.5 while workspaces under them were at pl-5 (from #3835). The section header now reads as if it's at the same level as the project row instead of grouping the workspaces beneath it. Bump the header to pl-5 so groups align with their workspaces under the project.
The count span sat in DOM order before the absolute action wrapper, so the wrapper rendered on top of it. Even with the action button at opacity-0, the wrapper could swallow hover/focus that should belong to the count. Render the action wrapper first, the count after with relative positioning so it's the top layer, and add pointer-events-none so the action's hover target stays clean. Also pin text-muted-foreground explicitly to match the project row.
Section was matching workspace at pl-5, which read as 'sibling' instead of 'parent'. Drop to pl-4 so the group sits one notch shallower than its workspaces.
Bring the section header to pl-3 (matches project row) and add mr-2 between the chevron/grip slot and the title so the icon-to-text gap mirrors the project row's gap-2.
Move section header from pl-3 to pl-5 so its chevron/grip aligns with the workspace icon column underneath it.
Workspaces inside a section share the same pl-5 indent as ungrouped workspaces, so the section nesting reads flat. Plumb the existing isInSection prop through to the row and bump grouped workspaces from pl-5 to pl-7.
Three valid points from PR review: - Project row "+" button: Enter/Space keydown bubbles to the row's collapse handler before the synthesized click fires. Add onKeyDown stopPropagation so keyboard activation doesn't toggle the row. - Project row "+" overlay: opacity-0 doesn't disable pointer events, so clicking the count area at rest can accidentally create a workspace. Add pointer-events-none at rest, re-enable on hover / focus-within / focus-visible. - Section header count: the fade-on-hover ran unconditionally, but the action overlay only renders when actions is truthy. When actions is absent the slot would go blank on hover. Gate the fade classes on actions.
Replaced the opacity layering (count opacity-0 + action opacity-100 on hover, with pointer-events-none gating to keep the count clickable) with a true display swap. Only one of the two is rendered at a time, so the count slot is never a transparent ghost over the button. For the section header, the dropdown trigger has data-state=open when its menu is open, which can persist after the row loses hover/focus. The action wrapper now stays visible via has-[[data-state=open]]:flex, and the count hides via peer-has-[[data-state=open]]:hidden so the open state still wins over the count.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx (1)
105-114:⚠️ Potential issue | 🟡 MinorStop
dblclickbubbling from the nested “New workspace” button.Double-clicking the
+can bubble adblclickto the row handler (Line 54) and trigger rename while trying to create workspaces quickly.🔧 Proposed fix
<button type="button" onClick={(event) => { event.stopPropagation(); onNewWorkspace(); }} onKeyDown={(event) => event.stopPropagation()} + onDoubleClick={(event) => event.stopPropagation()} onContextMenu={(event) => event.stopPropagation()} aria-label="New workspace" className="hidden size-full items-center justify-center rounded transition-colors hover:bg-muted group-hover:flex group-focus-within:flex focus-visible:flex focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx` around lines 105 - 114, The nested "New workspace" button in DashboardSidebarProjectRow is missing a dblclick stopper so double-clicks bubble up and trigger the row's rename handler; add an onDoubleClick={(event) => event.stopPropagation()} prop to the button (alongside the existing onClick/onKeyDown/onContextMenu handlers) in the DashboardSidebarProjectRow component to prevent dblclick events from propagating to the parent row handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsx`:
- Around line 105-114: The nested "New workspace" button in
DashboardSidebarProjectRow is missing a dblclick stopper so double-clicks bubble
up and trigger the row's rename handler; add an onDoubleClick={(event) =>
event.stopPropagation()} prop to the button (alongside the existing
onClick/onKeyDown/onContextMenu handlers) in the DashboardSidebarProjectRow
component to prevent dblclick events from propagating to the parent row handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82dbaea9-1fac-4d29-9f37-297bc4405a02
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectRow/DashboardSidebarProjectRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
…istency The thumbnail-at-rest / chevron-on-hover swap at the start of the project row was still using stacked opacities while the count/action slot at the row's end now uses display swap. Convert the icon slot to match — thumbnail is hidden on group-hover, chevron is hidden by default and shown on group-hover. One pattern across the row.
Project and section rows are role=button tabIndex=0, so clicking them (e.g., to collapse a section) gives the row itself :focus. group-focus-within matches that self-focus, which made the count hide and stay hidden — most visibly when collapsing a section that contains the active workspace: the user clicks the header, focus stays on it, and the count never returns until they click somewhere else. Switch to group-has-[:focus]: it only matches when a *descendant* has focus (e.g., the rename input or the dropdown trigger), not when the row itself is focused. Same swap on the action wrapper / + button so the action only appears when the user actually focuses into the row's interactive bits.
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx:96">
P2: Using `group-has-[:focus]` here breaks keyboard reveal when the header itself is focused; keep `group-focus-within` so actions become reachable via Tab.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {actions ? ( | ||
| // biome-ignore lint/a11y/noStaticElementInteractions: Nested action controls handle their own semantics; this wrapper only isolates events from the header toggle. | ||
| <div | ||
| className="peer hidden size-full items-center justify-center group-hover:flex group-has-[:focus]:flex has-[[data-state=open]]:flex" |
Contributor
There was a problem hiding this comment.
P2: Using group-has-[:focus] here breaks keyboard reveal when the header itself is focused; keep group-focus-within so actions become reachable via Tab.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx, line 96:
<comment>Using `group-has-[:focus]` here breaks keyboard reveal when the header itself is focused; keep `group-focus-within` so actions become reachable via Tab.</comment>
<file context>
@@ -93,7 +93,7 @@ export const DashboardSidebarSectionHeader = forwardRef<
// biome-ignore lint/a11y/noStaticElementInteractions: Nested action controls handle their own semantics; this wrapper only isolates events from the header toggle.
<div
- className="peer hidden size-full items-center justify-center group-hover:flex group-focus-within:flex has-[[data-state=open]]:flex"
+ className="peer hidden size-full items-center justify-center group-hover:flex group-has-[:focus]:flex has-[[data-state=open]]:flex"
onClick={(event) => event.stopPropagation()}
onKeyDown={(event) => event.stopPropagation()}
</file context>
Suggested change
| className="peer hidden size-full items-center justify-center group-hover:flex group-has-[:focus]:flex has-[[data-state=open]]:flex" | |
| className="peer hidden size-full items-center justify-center group-hover:flex group-focus-within:flex has-[[data-state=open]]:flex" |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
6 tasks
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
1 participant
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
Tightens the right edge of the v2 sidebar's project rows and section headers so the workspace count and the row's action button share one slot instead of competing for space.
text-xstotext-[10px]to match the section header. Parens dropped.Both swaps preserve the existing keyboard focus behavior — the action stays reachable via tab / focus-visible.
Builds on the v2 sidebar styling work in #3813 — credit to @saddlepaddle for the original pass; this PR is just polish on top.
Test plan
name … Nwith no "+" visiblefocus-visibleandgroup-focus-withintext-[10px])Summary by cubic
Collapses the v2 sidebar workspace count and actions into one end slot using a display swap, and tightens indents to clarify hierarchy. Keeps counts visible when rows are self-focused and keeps section actions visible while the menu is open; start icon swap now uses the same pattern.
text-[10px], no parens). “+” shows on hover or descendant focus via hidden/flex; has tooltip andaria-label; stops keydown bubbling. Start icon now swaps via display (thumbnail at rest, chevron on hover).pl-5withmr-2between icon and title. Actions show on hover or descendant focus and stay visible when open viahas-[[data-state=open]]; count hides only when actions are visible (no parens).isInSection; grouped rows indent topl-7(vspl-5ungrouped) for clearer nesting.Written for commit abc7700. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Style
Accessibility