mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): tool panel tab ✕ can close and re-open
The logs (and terminal) tab ✕ dismissed the pane from the layout but never synced the owning store — so the ⌘K toggle was stale and its open listener called setPaneCollapsed, a no-op when the pane isn't in the tree. The tab was gone with no way back short of a layout reset. Route the tab ✕ through closeCollapsePane (dismiss + store sync) so the toggle stays truthful, and make bindPaneCollapse's open listener call revealTreePane (un-dismiss + re-adopt) instead of setPaneCollapsed.
This commit is contained in:
parent
8f8bee94f7
commit
dd48d9a816
4 changed files with 159 additions and 10 deletions
|
|
@ -492,6 +492,11 @@ function bindPaneVisibility(
|
|||
// the zone to a persistent rail (tab stays) instead of hiding it — the
|
||||
// IntelliJ/VS-Code tool-window model. Restore routes back through `open` (rail
|
||||
// click / chevron) so ⌃`/the button stay truthful; the tab's ✕ removes it.
|
||||
//
|
||||
// OPEN uses revealTreePane (not setPaneCollapsed) so a pane dismissed via its
|
||||
// tab ✕ (closeCollapsePane) is un-dismissed + re-adopted — setPaneCollapsed
|
||||
// is a no-op when the pane isn't in the tree, so the toggle couldn't bring it
|
||||
// back after ✕ closed it.
|
||||
function bindPaneCollapse(
|
||||
paneId: string,
|
||||
$open: { get(): boolean; listen(fn: (open: boolean) => void): void },
|
||||
|
|
@ -500,7 +505,13 @@ function bindPaneCollapse(
|
|||
) {
|
||||
markCollapsePane(paneId)
|
||||
setPaneCollapsed(paneId, !$open.get())
|
||||
$open.listen(isOpen => setPaneCollapsed(paneId, !isOpen))
|
||||
$open.listen(isOpen => {
|
||||
if (isOpen) {
|
||||
revealTreePane(paneId)
|
||||
} else {
|
||||
setPaneCollapsed(paneId, true)
|
||||
}
|
||||
})
|
||||
registerPaneCloser(paneId, close)
|
||||
registerPaneOpener(paneId, open)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,117 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { atom } from 'nanostores'
|
||||
|
||||
import { allPaneIds } from './model'
|
||||
|
||||
// Repro for "the logs tab ✕ kills the pane and the toggle can't bring it
|
||||
// back": a tool panel (terminal/logs) bound via bindPaneCollapse had its tab ✕
|
||||
// routed through dismissTreePane, which removed the pane from the layout but
|
||||
// never synced the owning store — so the ⌘K "Toggle logs" toggle was stale
|
||||
// (nanostores don't fire on a same-value .set), and its open listener called
|
||||
// setPaneCollapsed, which is a no-op when the pane isn't in the tree. The tab
|
||||
// was gone with no way back short of a layout reset.
|
||||
//
|
||||
// The fix routes the tab ✕ through closeCollapsePane (dismiss + store sync),
|
||||
// and bindPaneCollapse's open listener calls revealTreePane (un-dismiss +
|
||||
// re-adopt) instead of setPaneCollapsed.
|
||||
|
||||
describe('collapse pane tab close + toggle recovery', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear()
|
||||
vi.resetModules()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.resetModules()
|
||||
})
|
||||
|
||||
async function setup() {
|
||||
const tree = await import('@/components/pane-shell/tree/store')
|
||||
const model = await import('@/components/pane-shell/tree/model')
|
||||
const { registry } = await import('@/contrib/registry')
|
||||
|
||||
// A tool panel (logs): placement 'bottom', not uncloseable.
|
||||
registry.register({
|
||||
id: 'logs',
|
||||
area: 'panes',
|
||||
title: 'logs',
|
||||
data: { placement: 'bottom' },
|
||||
render: () => null
|
||||
})
|
||||
// The main workspace, so adoption has an anchor.
|
||||
registry.register({
|
||||
id: 'workspace',
|
||||
area: 'panes',
|
||||
title: 'workspace',
|
||||
data: { placement: 'main', uncloseable: true },
|
||||
render: () => null
|
||||
})
|
||||
|
||||
tree.declareDefaultTree(
|
||||
model.split('column', [
|
||||
model.group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
model.group(['logs'], { active: 'logs', id: 'grp-logs' })
|
||||
])
|
||||
)
|
||||
|
||||
// Mirror controller.tsx bindPaneCollapse: the owning store drives
|
||||
// visibility, the closer syncs it on ✕, the opener reveals.
|
||||
const $logsOpen = atom(true)
|
||||
|
||||
// bindPaneCollapse inline (can't import the controller's private fn).
|
||||
tree.markCollapsePane('logs')
|
||||
tree.setPaneCollapsed('logs', !$logsOpen.get())
|
||||
$logsOpen.subscribe(isOpen => {
|
||||
if (isOpen) {
|
||||
tree.revealTreePane('logs')
|
||||
} else {
|
||||
tree.setPaneCollapsed('logs', true)
|
||||
}
|
||||
})
|
||||
tree.registerPaneCloser('logs', () => $logsOpen.set(false))
|
||||
tree.registerPaneOpener('logs', () => $logsOpen.set(true))
|
||||
|
||||
return { tree, $logsOpen }
|
||||
}
|
||||
|
||||
it('tab ✕ dismisses the pane AND syncs the owning store', async () => {
|
||||
const { tree, $logsOpen } = await setup()
|
||||
|
||||
expect($logsOpen.get()).toBe(true)
|
||||
expect(allPaneIds(tree.$layoutTree.get()!)).toContain('logs')
|
||||
|
||||
// The tab ✕ on a collapse pane.
|
||||
tree.closeCollapsePane('logs')
|
||||
|
||||
// Pane is gone from the layout…
|
||||
expect(allPaneIds(tree.$layoutTree.get()!)).not.toContain('logs')
|
||||
// …and the store was synced to false.
|
||||
expect($logsOpen.get()).toBe(false)
|
||||
})
|
||||
|
||||
it('the toggle brings the pane back after the tab ✕ closed it', async () => {
|
||||
const { tree, $logsOpen } = await setup()
|
||||
|
||||
// Close via tab ✕.
|
||||
tree.closeCollapsePane('logs')
|
||||
expect($logsOpen.get()).toBe(false)
|
||||
expect(allPaneIds(tree.$layoutTree.get()!)).not.toContain('logs')
|
||||
|
||||
// Re-open via the toggle (the ⌘K row / opener).
|
||||
$logsOpen.set(true)
|
||||
|
||||
// The pane is back in the layout.
|
||||
expect(allPaneIds(tree.$layoutTree.get()!)).toContain('logs')
|
||||
expect($logsOpen.get()).toBe(true)
|
||||
})
|
||||
|
||||
it('closeTabPane routes collapse panes through closeCollapsePane', async () => {
|
||||
const { tree, $logsOpen } = await setup()
|
||||
|
||||
tree.closeTabPane('logs')
|
||||
|
||||
expect(allPaneIds(tree.$layoutTree.get()!)).not.toContain('logs')
|
||||
expect($logsOpen.get()).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -36,10 +36,10 @@ import {
|
|||
activateTreePane,
|
||||
closeAllTreeTabs,
|
||||
closeOtherTreeTabs,
|
||||
closeTabPane,
|
||||
closeTreePane,
|
||||
closeTreeTabsToRight,
|
||||
collapseTreePane,
|
||||
dismissTreePane,
|
||||
isCollapsePane,
|
||||
isSessionStripPane,
|
||||
noteActiveTreeGroup,
|
||||
|
|
@ -101,7 +101,7 @@ function ZoneMenu({
|
|||
renderActionItem(kit, {
|
||||
icon: 'close',
|
||||
label: t.common.close,
|
||||
onSelect: () => closeTreePane(paneId)
|
||||
onSelect: () => closeTabPane(paneId)
|
||||
})}
|
||||
{renderActionItem(kit, {
|
||||
disabled: !targets.others,
|
||||
|
|
@ -287,10 +287,11 @@ export function TreeGroup({
|
|||
// MAIN strands the whole app behind a strip.
|
||||
const minimizable = !shown.some(id => paneChrome(paneFor(id)).uncloseable)
|
||||
|
||||
// Tab ✕: a tool panel (terminal/logs) is REMOVED from the layout (comes back
|
||||
// via its toggle); everything else routes through its Close (a session tile
|
||||
// closes the session, a store-bound pane collapses).
|
||||
const closeTab = (paneId: string) => (isCollapsePane(paneId) ? dismissTreePane(paneId) : closeTreePane(paneId))
|
||||
// Tab ✕: a tool panel (terminal/logs) is removed from the layout AND its
|
||||
// owning store is synced (closeTabPane → closeCollapsePane), so the toggle
|
||||
// stays truthful and can bring it back; everything else routes through its
|
||||
// Close (a session tile closes the session, a store-bound pane collapses).
|
||||
const closeTab = (paneId: string) => closeTabPane(paneId)
|
||||
|
||||
// A pane whose store owns Close keeps the gesture even when the pane itself
|
||||
// is uncloseable — the workspace tab empties to a fresh draft rather than
|
||||
|
|
|
|||
|
|
@ -420,12 +420,22 @@ export function treeTabCloseTargets(paneId: string): { all: number; others: numb
|
|||
return { all: others.length + (isUncloseablePane(paneId) ? 0 : 1), others: others.length, right: right.length }
|
||||
}
|
||||
|
||||
/** Close a pane, routing collapse panes (terminal/logs) through dismiss +
|
||||
* store sync so their toggle stays truthful. */
|
||||
export function closeTabPane(paneId: string) {
|
||||
if (isCollapsePane(paneId)) {
|
||||
closeCollapsePane(paneId)
|
||||
} else {
|
||||
closeTreePane(paneId)
|
||||
}
|
||||
}
|
||||
|
||||
export function closeOtherTreeTabs(paneId: string): void {
|
||||
closeableTreeSiblings(paneId).others.forEach(closeTreePane)
|
||||
closeableTreeSiblings(paneId).others.forEach(closeTabPane)
|
||||
}
|
||||
|
||||
export function closeTreeTabsToRight(paneId: string): void {
|
||||
closeableTreeSiblings(paneId).right.forEach(closeTreePane)
|
||||
closeableTreeSiblings(paneId).right.forEach(closeTabPane)
|
||||
}
|
||||
|
||||
/** Close every closeable tab in `paneId`'s group (the uncloseable workspace stays). */
|
||||
|
|
@ -433,7 +443,7 @@ export function closeAllTreeTabs(paneId: string): void {
|
|||
const tree = $layoutTree.get()
|
||||
const panes = (tree ? findGroupOfPane(tree, paneId) : null)?.panes ?? []
|
||||
|
||||
panes.filter(id => !isUncloseablePane(id)).forEach(closeTreePane)
|
||||
panes.filter(id => !isUncloseablePane(id)).forEach(closeTabPane)
|
||||
}
|
||||
|
||||
/** Pane ids in the tree under a `${prefix}:` namespace — lets a mirror prune
|
||||
|
|
@ -616,6 +626,16 @@ export function dismissTreePane(paneId: string) {
|
|||
}
|
||||
}
|
||||
|
||||
/** Tab ✕ on a tool panel (terminal/logs): dismiss from the layout AND sync
|
||||
* the owning store so the toggle stays truthful. The pane is removed first
|
||||
* so the store listener's setPaneCollapsed is a no-op (pane already gone),
|
||||
* not a minimize that strands a shared-zone sibling. The toggle's open
|
||||
* path (revealTreePane in bindPaneCollapse) un-dismisses + re-adopts. */
|
||||
export function closeCollapsePane(paneId: string) {
|
||||
dismissTreePane(paneId)
|
||||
paneClosers[paneId]?.()
|
||||
}
|
||||
|
||||
export function closeTreePane(paneId: string) {
|
||||
const closer = paneClosers[paneId]
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue