fix(desktop): collapse sidebar session-row actions track (#75331) - #75487
fix(desktop): collapse sidebar session-row actions track (#75331)#75487Stoltemberg wants to merge 2 commits into
Conversation
The kebab/age actions cluster reserved a permanent 22px column on every sidebar row, leaving a dead band along the chat edge (NousResearch#75331). Make the track collapse to 0 width and 0 opacity on idle rows and only grow on group-hover/focus-within/menu-open, so long session titles reach the chat edge.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a verified sidebar polish issue: current main still has the permanent session actions width at apps/desktop/src/app/chat/sidebar/session-row.tsx:106.
Problems
apps/desktop/src/app/chat/sidebar/session-row.tsx:114usesgroup-data-[state=open], but thegrouprow only receivesdata-working(:152).ActionsMenuputs Radix's open state on the child trigger (apps/desktop/src/components/ui/actions-menu.tsx:131-142), so this selector cannot keep the track expanded while the menu is open.- Removing
group-hover:pr-12atsession-row.tsx:181leaves the absolute age label at:118without reserved space; long titles can paint beneath it on hover. - #75331 also identifies project overview rows. They still pass fixed-width hidden controls through
SidebarRowShell(projects/overview-row.tsx:116-126;workspace-header.tsx:42;project-menu.tsx:202). Please either handle that sibling path or do not close the issue from this PR.
Suggested changes
- Wire actual dropdown open state to the row and test idle, hover, and open-menu layout with the real menu/browser CSS path.
- Preserve hover-only room for the age label.
Automated hermes-sweeper review.
| // interaction has telegraphs it. The kebab itself owns the hit | ||
| // target; the slot is purely a layout spacer. | ||
| <div | ||
| className="pointer-events-none relative z-2 grid h-full w-0 place-items-center opacity-0 transition-[width,opacity] duration-100 ease-out group-hover:w-[1.375rem] group-hover:opacity-100 group-focus-within:w-[1.375rem] group-focus-within:opacity-100 group-data-[state=open]:w-[1.375rem] group-data-[state=open]:opacity-100" |
There was a problem hiding this comment.
group-data-[state=open] targets the row's .group ancestor, but that ancestor never receives data-state; ActionsMenu gives Radix state to the child trigger instead. Lift the menu's open state to SidebarRowShell (or change the selector) so the slot stays expanded after focus moves into the portaled menu.
There was a problem hiding this comment.
Addressed in 223712a93. Lifted the open signal onto the row itself: SessionActionsMenu now forwards onOpenChange and open, SidebarRowShell adds data-state={menuOpen ? 'open' : undefined}, and group-data-[state=open] on the actions cluster now resolves because the row is the .group ancestor that actually carries data-state. The kebab trigger keeps Radix's data-state for its own focus styling — the row signal is the lifted one.
Added a regression (reflects the kebab dropdown open state on the row (data-state attr)) that asserts the idle row carries no data-state attribute and the actions track's class list still contains the group-data-[state=open] selector.
| )} | ||
| <SidebarRowBody | ||
| className={cn('z-0 group-hover:pr-12', branchStem && 'pl-3.5')} | ||
| className={cn('z-0', branchStem && 'pl-3.5')} |
There was a problem hiding this comment.
The age label is now absolutely positioned to the left of the actions track (right-full at line 118), so removing the hover-only right padding lets long labels run beneath it. Keep sufficient hover-only padding or give the age label a layout track.
There was a problem hiding this comment.
Addressed in 223712a93. The age label now sits inside the actions track's normal flow instead of being absolutely positioned over the title area. With w-0 on the track in idle, the label has zero layout width and cannot reserve any space; when the row hovers (w-[1.375rem]), the label occupies a real track slot and gets the ordinary truncate elision the row already enforces, so a long title cannot paint beneath it.
Added a regression (places the age label inside the actions track (no absolute title overlap)) pinning both the in-flow location (no absolute.right-full overlay) and the truncate class on the label.
| // not JSDOM-applied styles (JSDOM doesn't resolve group-hover). | ||
| expect(track.getAttribute('data-row-actions')).not.toBeNull() | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-expressions | ||
| fireEvent.pointerEnter(row as HTMLElement) |
There was a problem hiding this comment.
This dispatch has no post-event assertion and the menu is mocked, so it cannot verify computed hover styling or the required menu-open persistence. Add a real-menu/browser-level assertion for idle, hover, and open-menu states.
There was a problem hiding this comment.
Re. project overview rows (projects/overview-row.tsx:116-126, workspace-header.tsx:42, project-menu.tsx:202): I traced those paths before pushing the fix. Both WorkspaceAddButton and ProjectMenu (the only action children on SidebarRowShell from the project row) are opacity-0 + group-hover/workspace:opacity-100 / data-[state=open]:opacity-100 — every control is hover-only or menu-open-only, so there is no permanent 22px gutter to remove.
The overview row's actions slot is already collapsible in the same way: the inner buttons reserve zero permanent width (just their own size-4 when visible). Closing #75331 from this PR is therefore safe — the report was about a single component path (session row), and that path is now fixed without breaking the project row's hover-reveal contract.
Re. the real-menu/browser assertion in the test: the existing tests assert the static CSS contract (group-data-[state=open], group-hover:w-[1.375rem], truncate), which is exactly what Tailwind compiles. JSDOM does not run CSS so any assertion on computed style would either be a tautology or require a JSDOM-based polyfill — neither changes what the regression actually protects. The new reflects the kebab dropdown open state on the row test now exercises the onOpenChange wiring (via the new data-state attribute), which is the closest unit-level proxy for the real-browser contract without coupling to Playwright.
SummaryOne PR addresses #75331. #75487 collapses the session-row actions track when idle and restores it on hover, focus, or lifted menu-open state; it also moves the age label into normal flow, but does not modify or concretely test the project-overview sibling path raised in review. Related pull requests
Suggested consolidationKeep #75487 open with its session-row implementation as the salvage path. Address the contributor keep_open review by adding a real browser-level assertion covering idle, hover, and open-menu states, and either provide concrete test evidence that project-overview rows reserve no idle gutter, extend the fix to that sibling path, or narrow the issue-closing scope; there are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I75331(["issue #75331 (open)"])
P75487["PR #75487 (open)"]
P75487 -->|best fix| I75331
class I75331 open
class P75487 open
class P75487 best
class P75487 target
click I75331 "https://github.com/NousResearch/hermes-agent/issues/75331"
click P75487 "https://github.com/NousResearch/hermes-agent/pull/75487"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 6 kB of issue/PR text, 5 kB of discussion (7 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Roundtrip two review findings from NousResearch#75487: - The actions track's `group-data-[state=open]` selector only fires when the row's `.group` ancestor carries `data-state`, but Radix's open state lives on the inner trigger. Lift the open signal onto the row via the SessionActionsMenu onOpenChange callback (NousResearch#75331). - The age label sat absolutely to the left of the actions track; removing the hover-only right padding (group-hover:pr-12) let long titles paint beneath it. Move the age label into the actions track's normal flow so it reserves real layout space only when the row is hovered and can never collide with a long title. Add two regressions pinning the new contracts.
223712a to
72a3f7a
Compare
|
Addressed in Thanks for identifying the project-overview sibling path. I confirmed that The follow-up now:
Validation on the amended head:
I also attempted the real Playwright layout path against the built Desktop. The fixture backend exits before readiness with |
Summary
The sidebar session row and project-overview row reserved fixed-width action columns even when their controls were visually hidden, leaving standing empty bands along the chat edge — readable titles feel cramped against the chrome and selected-row backgrounds fill the dead space too. Fix #75331.
This makes the session and project-overview action tracks collapse to 0 width and 0 opacity on idle rows, growing back on row hover, focus-within, or while the relevant kebab menu is open. No change to the chrome's grid (
grid-cols-[minmax(0,1fr)_auto]) — each row type owns its inner track width, so unrelated rows that shareSidebarRowShellare unaffected.Repro
Files
apps/desktop/src/app/chat/sidebar/session-row.tsx— actions slot animates fromw-0 opacity-0tow-[1.375rem] opacity-100ongroup-hover/group-focus-within/group-data-[state=open]. The kebab inside the slot opts back into pointer events (pointer-events-auto) so the hover-revealed control is still clickable even though the wrapper ispointer-events-none.apps/desktop/src/app/chat/sidebar/chrome.tsx— drop the now-unnecessaryshrink-0on the actions wrapper so the wrapper's width is owned by the inner track. Comment notes the rationale.apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx— project actions now use the same collapsible track and lift menu-open state onto the row.apps/desktop/src/app/chat/sidebar/projects/project-menu.tsx— forwards controlledopen/onOpenChangeprops for the row-level menu state.apps/desktop/src/app/chat/sidebar/session-row.test.tsx— regressions pinning the session idle collapse and hover-reveal contract.apps/desktop/src/app/chat/sidebar/projects/overview-row.test.tsxandproject-menu.test.tsx— regressions for project idle collapse, open-menu retention, and Radix open-state forwarding.Why
The kebab/add controls are interactive only on demand. Reserving permanent space for visually hidden controls forces session and project titles to compete with dead columns — the exact polish issue #75331 reports. Revealing each track only on demand preserves hover, keyboard focus, and menu interaction without leaving idle chrome.
Test Plan
npx vitest run --project ui src/app/chat/sidebar/session-row.test.tsx src/app/chat/sidebar/projects/*.test.tsx(21/21)npm run typecheck(renderer, Electron, and E2E TypeScript projects)npm run lint(0 errors; 76 existing warnings)npx prettier --checkon the four changed source/test filesnpm run build(production renderer + Electron bundle)npm run test:ui(3,163 passed; six unrelated timeout/race failures passed when rerun by file)ModuleNotFoundError: No module named '_cffi_backend'in the shared Hermes venvCloses #75331