fix(desktop): preserve multi-pane plugins when closing panes
Closing a pane contributed by a plugin used to disable the entire plugin, unloading every one of its contributions. For a plugin that owns several independent panes (e.g. Bot Mode's Cronjobs pane alongside its Bots roster and composer middleware), closing one pane silently killed the rest. Now: closing one pane of a multi-pane plugin dismisses only that pane; the plugin stays enabled and its other panes/commands/middleware keep working. Reset layout restores dismissed contributed panes. A single-pane plugin keeps the existing symmetric behavior (Close disables the plugin, with Settings -> Plugins as the recovery path). Adds regression coverage for both cases.
This commit is contained in:
committed by
Teknium
parent
86379c519a
commit
ad8365d533
@@ -0,0 +1,71 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { setPluginEnabled } from '@/contrib/plugins-store'
|
||||
import { registry } from '@/contrib/registry'
|
||||
|
||||
import { allPaneIds, group, split } from './model'
|
||||
import { $dismissedPanes, $layoutTree, closeTreePane } from './store'
|
||||
|
||||
vi.mock('@/contrib/plugins-store', () => ({ setPluginEnabled: vi.fn() }))
|
||||
vi.mock('@/store/notifications', () => ({ notify: vi.fn() }))
|
||||
|
||||
const disposers: (() => void)[] = []
|
||||
|
||||
function registerPluginPane(pluginId: string, paneId: string) {
|
||||
disposers.push(
|
||||
registry.register({
|
||||
area: 'panes',
|
||||
data: { placement: 'main' },
|
||||
id: paneId,
|
||||
render: () => null,
|
||||
source: `plugin:${pluginId}`,
|
||||
title: paneId
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear()
|
||||
$dismissedPanes.set(new Set())
|
||||
vi.mocked(setPluginEnabled).mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
disposers.splice(0).forEach(dispose => dispose())
|
||||
})
|
||||
|
||||
describe('closing plugin panes', () => {
|
||||
it('dismisses one pane without disabling a plugin that contributes multiple panes', () => {
|
||||
registerPluginPane('bots', 'bots:pane')
|
||||
registerPluginPane('bots', 'bots:routines')
|
||||
$layoutTree.set(
|
||||
split('row', [
|
||||
group(['workspace'], { active: 'workspace', id: 'g-main' }),
|
||||
group(['bots:pane'], { active: 'bots:pane', id: 'g-bots' }),
|
||||
group(['bots:routines'], { active: 'bots:routines', id: 'g-routines' })
|
||||
])
|
||||
)
|
||||
|
||||
closeTreePane('bots:routines')
|
||||
|
||||
expect(allPaneIds($layoutTree.get()!)).not.toContain('bots:routines')
|
||||
expect(allPaneIds($layoutTree.get()!)).toContain('bots:pane')
|
||||
expect($dismissedPanes.get()).toContain('bots:routines')
|
||||
expect(setPluginEnabled).not.toHaveBeenCalled()
|
||||
|
||||
// Any later registry mutation runs pane adoption. The dismissal must
|
||||
// survive that cycle instead of immediately resurrecting Cronjobs.
|
||||
registerPluginPane('other', 'other:pane')
|
||||
expect(allPaneIds($layoutTree.get()!)).not.toContain('bots:routines')
|
||||
})
|
||||
|
||||
it('keeps disabling a plugin whose only pane is closed', () => {
|
||||
registerPluginPane('single', 'single:pane')
|
||||
$layoutTree.set(group(['single:pane'], { active: 'single:pane', id: 'g-single' }))
|
||||
|
||||
closeTreePane('single:pane')
|
||||
|
||||
expect(setPluginEnabled).toHaveBeenCalledWith('single', false)
|
||||
expect(allPaneIds($layoutTree.get()!)).toContain('single:pane')
|
||||
})
|
||||
})
|
||||
@@ -172,9 +172,11 @@ function frontPaneInGroup(paneId: string) {
|
||||
* - a registered closer (core panes whose visibility an app store owns:
|
||||
* review/terminal/preview/sessions) closes through that store, so the
|
||||
* titlebar/statusbar toggles stay truthful;
|
||||
* - everything else (plugin panes, unbound core panes) is DISMISSED: removed
|
||||
* from the tree and remembered so adoption doesn't re-add it. Reveal
|
||||
* intent (a preview target, ⌘G) or a layout reset un-dismisses.
|
||||
* - unbound core panes and panes from multi-pane plugins are DISMISSED:
|
||||
* removed from the tree and remembered so adoption doesn't re-add them.
|
||||
* Reveal intent (a preview target, ⌘G) or a layout reset un-dismisses;
|
||||
* - closing the sole pane from a plugin disables that plugin, preserving the
|
||||
* discoverable Settings → Plugins recovery path for single-pane plugins.
|
||||
*/
|
||||
const DISMISSED_KEY = 'hermes.desktop.dismissedPanes.v1'
|
||||
|
||||
@@ -762,14 +764,24 @@ export function closeTreePane(paneId: string) {
|
||||
return
|
||||
}
|
||||
|
||||
// A plugin's pane: Close = DISABLE the plugin — the same switch as
|
||||
// Settings → Plugins, so recovery is discoverable and symmetric. The
|
||||
// contribution unregisters but the pane id STAYS in the tree, so
|
||||
// re-enabling restores it exactly where it was. (Dismissal + removal
|
||||
// would strand the pane with no way back short of a layout reset.)
|
||||
const source = registry.getArea('panes').find(c => c.id === paneId)?.source
|
||||
const panes = registry.getArea('panes')
|
||||
const source = panes.find(c => c.id === paneId)?.source
|
||||
|
||||
if (source?.startsWith('plugin:')) {
|
||||
// A plugin may own several independent panes. Closing one of them must not
|
||||
// unload every contribution from that plugin (for example, closing Bot
|
||||
// Mode's Cronjobs pane must leave its Bots roster and composer middleware
|
||||
// alive). Dismiss just that pane; Layout reset remains the explicit way to
|
||||
// restore dismissed contributed panes.
|
||||
if (panes.filter(c => c.source === source).length > 1) {
|
||||
dismissTreePane(paneId)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// A single-pane plugin keeps the existing symmetric behavior: Close uses
|
||||
// the same switch as Settings → Plugins. Its contribution unregisters but
|
||||
// the pane id stays in the tree, so re-enabling restores its exact place.
|
||||
const pluginId = source.slice('plugin:'.length)
|
||||
void setPluginEnabled(pluginId, false)
|
||||
notify({
|
||||
@@ -1111,15 +1123,6 @@ function adoptContributedPanes(): void {
|
||||
const mainId = panes.find(c => placementOf(c.id) === 'main')?.id
|
||||
const inTree = new Set(allPaneIds(tree))
|
||||
|
||||
// Plugin panes are never dismissed anymore (Close disables the plugin
|
||||
// instead) — drop stale entries so panes stranded by the old behavior
|
||||
// re-adopt on their own.
|
||||
for (const pane of panes) {
|
||||
if (pane.source?.startsWith('plugin:') && $dismissedPanes.get().has(pane.id)) {
|
||||
setDismissed(pane.id, false)
|
||||
}
|
||||
}
|
||||
|
||||
const dismissed = $dismissedPanes.get()
|
||||
|
||||
// `placement: 'floating'` opts OUT of the tree entirely — those panes render
|
||||
|
||||
@@ -246,6 +246,12 @@ data: {
|
||||
`'top' | 'bottom' | 'left' | 'right' | 'center'`. Declare a `width`/`height` so
|
||||
the pane doesn't claim half the zone.
|
||||
|
||||
Closing the only pane contributed by a plugin disables that plugin, which can
|
||||
be re-enabled from **Settings → Plugins**. When a plugin contributes multiple
|
||||
panes, closing one dismisses only that pane and leaves the plugin's other panes,
|
||||
commands, and middleware active. **Reset layout** restores dismissed contributed
|
||||
panes.
|
||||
|
||||
### Pages and sidebar nav
|
||||
|
||||
A route mounts a full page in the workspace pane, like any built-in view. Pair it
|
||||
|
||||
Reference in New Issue
Block a user