From 6cb1aa44ed18ce3c87d02a9f18b1d0a3225f286d Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 30 Jul 2026 04:16:34 -0500 Subject: [PATCH] fix(desktop): opening the diff pane no longer opens the file tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `revealTreePane` un-collapsed a pane's column by calling the side's bound store setter. On the right that setter IS `setFileBrowserOpen` — the file tree's own toggle — so anything sharing that column dragged the tree open with it: ⌘G on the diff, a session tile reveal, `focus_pane`. Un-collapse the column directly instead. The tree now opens only when its own toggle is pressed. `revealPreview` had a local workaround for exactly this, which the fix makes redundant. --- apps/desktop/src/app/contrib/controller.tsx | 14 ---- .../pane-shell/tree/reactive-unhide.test.ts | 71 +++++++++++-------- .../src/components/pane-shell/tree/store.ts | 20 +++--- 3 files changed, 51 insertions(+), 54 deletions(-) diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index b808b236583..a829d3d613d 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -27,8 +27,6 @@ import { revealTreePane, setPaneCollapsed, setTreePaneHidden, - setTreeSideCollapsed, - treeSideOfPane, watchContributedPanes } from '@/components/pane-shell/tree/store' import { SidebarProvider } from '@/components/ui/sidebar' @@ -629,18 +627,6 @@ registerPaneCloser('files', () => // the side, unhide, front — a NEW target while already visible still fronts. const revealPreview = () => { dockPaneBeside('preview', 'files') - - // The preview shares a collapsible column with the file tree, and - // revealTreePane un-collapses a column through its bound store — here ⌘J / - // $fileBrowserOpen, which IS the tree's toggle. Going through it would open - // the tree every time a preview opened. Un-collapse the column directly and - // leave the toggle alone, so a preview can appear on its own. - const side = treeSideOfPane('preview') - - if (side) { - setTreeSideCollapsed(side, false) - } - revealTreePane('preview') } diff --git a/apps/desktop/src/components/pane-shell/tree/reactive-unhide.test.ts b/apps/desktop/src/components/pane-shell/tree/reactive-unhide.test.ts index bb6153f50eb..34cab5cf9da 100644 --- a/apps/desktop/src/components/pane-shell/tree/reactive-unhide.test.ts +++ b/apps/desktop/src/components/pane-shell/tree/reactive-unhide.test.ts @@ -29,19 +29,21 @@ describe('reactive pane unhide', () => { const model = await import('@/components/pane-shell/tree/model') const { registry } = await import('@/contrib/registry') - // Register `files` like controller.tsx does — placement 'right' is what - // makes `treeSideOfPane('files')` return 'right' (and therefore what - // makes the buggy `revealTreePane` auto-expand the right column). - registry.register({ - id: 'files', - area: 'panes', - title: 'files', - data: { placement: 'right' }, - render: () => null - }) + // Register the right-column panes like controller.tsx does — placement + // 'right' is what makes `treeSideOfPane(id)` return 'right' (and therefore + // what makes the buggy `revealTreePane` auto-expand the right column). + for (const id of ['files', 'preview', 'review']) { + registry.register({ + id, + area: 'panes', + title: id, + data: { placement: 'right' }, + render: () => null + }) + } // Declare a minimal default tree mirroring the production DEFAULT_TREE's - // row shape (sessions | workspace | right-column-with-files). + // row shape (sessions | workspace | right-column-with-the-rail). tree.declareDefaultTree( model.split( 'row', @@ -51,7 +53,16 @@ describe('reactive pane unhide', () => { model.split( 'column', [ - model.split('row', [model.group(['files'], { id: 'grp-files' })], [1], 'spl-rail'), + model.split( + 'row', + [ + model.group(['review'], { id: 'grp-review' }), + model.group(['preview'], { id: 'grp-preview' }), + model.group(['files'], { id: 'grp-files' }) + ], + [1, 1, 1.2], + 'spl-rail' + ), model.group(['terminal'], { id: 'grp-terminal' }) ], [1.6, 1], @@ -127,30 +138,16 @@ describe('reactive pane unhide', () => { expect(tree.$collapsedTreeSides.get().has('right')).toBe(false) }) - // Opening a preview used to drag the file tree open with it: the preview - // shares ⌘J's column, and `revealTreePane` un-collapses a column through its - // bound store — which for the right side IS the file-browser toggle. The - // reveal now un-collapses the column directly and leaves the toggle alone. + // Opening a neighbour used to drag the file tree open with it: review and + // preview share ⌘J's column, and `revealTreePane` un-collapsed a column + // through its bound store — which for the right side IS the file-browser + // toggle. The reveal now un-collapses the column directly. it('revealing a preview opens its column without flipping the file-tree toggle', async () => { const { tree, layout } = await setupWithFiles() - const { registry } = await import('@/contrib/registry') - - registry.register({ - id: 'preview', - area: 'panes', - data: { placement: 'right' }, - render: () => null, - title: 'preview' - }) layout.setFileBrowserOpen(false) expect(tree.$collapsedTreeSides.get().has('right')).toBe(true) - // The revealPreview sequence from controller.tsx. - const side = tree.treeSideOfPane('files') - - expect(side).toBe('right') - tree.setTreeSideCollapsed(side!, false) tree.revealTreePane('preview') // The column is showing… @@ -159,6 +156,20 @@ describe('reactive pane unhide', () => { expect(layout.$fileBrowserOpen.get()).toBe(false) }) + it('opening the diff pane leaves the file tree closed', async () => { + const { tree, layout } = await setupWithFiles() + + layout.setFileBrowserOpen(false) + expect(tree.$collapsedTreeSides.get().has('right')).toBe(true) + + // ⌘G — `toggleReview` reveals the review pane, which lives in the same + // right column as the file tree. + tree.revealTreePane('review') + + expect(tree.$collapsedTreeSides.get().has('right')).toBe(false) + expect(layout.$fileBrowserOpen.get()).toBe(false) + }) + it('reactive unhide does not invoke the right side opener directly', async () => { const { tree, layout } = await setupWithFiles() diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index cf09dde33e0..b72ab5e03b4 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -646,8 +646,10 @@ export type TreeSide = 'left' | 'right' export const $collapsedTreeSides = atom>(new Set()) // Side visibility is DERIVED from an app store (the binding owns persistence -// + button state); reveals flow back through its setter so they never -// disagree with the flag. +// + button state). Reveals un-collapse the column directly instead of writing +// back through the setter — the right side's store IS the file tree's toggle, +// so a neighbour's reveal must not press it. Layout reset still reopens every +// side through its setter, because there the toggles SHOULD move. const sideOpeners: Partial void>> = {} export function setTreeSideCollapsed(side: TreeSide, collapsed: boolean) { @@ -772,14 +774,12 @@ export function revealTreePane(paneId: string) { const side = treeSideOfPane(paneId) if (side && $collapsedTreeSides.get().has(side)) { - const open = sideOpeners[side] - - // Through the bound store when there is one, so the toggle stays truthful. - if (open) { - open(true) - } else { - setTreeSideCollapsed(side, false) - } + // Un-collapse the COLUMN, never the side's bound store: on the right that + // store is ⌘J / $fileBrowserOpen, i.e. the file tree's own toggle. Routing + // a reveal through it dragged the tree open behind every neighbour that + // shares the column — open the diff (⌘G) and the file tree appeared too. + // The tree opens only when the user opens it. + setTreeSideCollapsed(side, false) } const hiddenNow = $hiddenTreePanes.get()