Skip to content

fix(desktop): flip zone collapse chevron to action direction - #74007

Closed
chillerno1 wants to merge 2 commits into
NousResearch:mainfrom
chillerno1:fix/desktop-terminal-collapse-chevron
Closed

fix(desktop): flip zone collapse chevron to action direction#74007
chillerno1 wants to merge 2 commits into
NousResearch:mainfrom
chillerno1:fix/desktop-terminal-collapse-chevron

Conversation

@chillerno1

Copy link
Copy Markdown
Contributor

Summary

  • Point zone minimize/restore chevrons in the action direction: down when expanded, up when collapsed.
  • Align floating-pane collapse chevrons with the same convention (and with master-detail detail headers).
  • Extend the floating-panes collapse test to assert the chevron classes.

Fixes #74006

Test plan

  • NODE_OPTIONS='--localstorage-file=/tmp/hermes-vitest-ls.json' npm run test:ui -- src/components/pane-shell/tree/renderer/floating-panes.test.tsx (8 passed)
  • Manual desktop sandbox: terminal collapse shows an up chevron; restore shows a down chevron
  • Manual spot check confirmed by the user

Collapsed tool zones (terminal/logs) kept a down chevron after minimize,
so the restore affordance looked like another collapse. Point the icon in
the action direction — down when expanded, up when collapsed — matching
master-detail collapsible detail headers. Same fix for floating panes.
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Reviewed against #74006 — this correctly resolves it. The flip collapsed ? 'chevron-up' : 'chevron-down' gives the intended semantics: down = collapse (expanded), up = restore (collapsed), matching the issue's expected/actual.

Nice that it fixes both surfaces in one pass — the docked/tool-zone strip in tree-group.tsx (node.minimized, the path the issue actually reports for the bottom-docked terminal) and the sibling floating-pane header in floating-panes.tsx — so the two chevrons stay consistent.

One minor coverage note (non-blocking): the added assertions live in floating-panes.test.tsx, so the tree-group.tsx flip — which is the exact reported terminal-zone case — has no direct test. It's a one-line, symmetric change so risk is low, but a node.minimized chevron-direction assertion in the tree-group tests would lock the reported path in against future regressions.

Otherwise LGTM.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 29, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Static review confirms the action direction is correct: tree-group.tsx:328 restores when node.minimized and collapses otherwise, so the changed icon conditional at tree-group.tsx:547 now matches the invoked action. floating-panes.tsx:142-147 has the analogous toggle, and the added assertions exercise both floating states.

Suggested changes

  • Consider adding a direct regression test for the reported docked-zone route in apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx. The new assertions in apps/desktop/src/components/pane-shell/tree/renderer/floating-panes.test.tsx:171-186 cover only the sibling floating implementation; no TreeGroup renderer test was found under apps/desktop/src/components/pane-shell/tree/.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
@chillerno1

Copy link
Copy Markdown
Contributor Author

@teknium1 have added the direct regression test as suggested

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for this fix! It was salvaged into #86771 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing since the work has landed.

@teknium1 teknium1 closed this Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop zone collapse chevron stays down after minimizing terminal

4 participants