From 1c0ccdddcd9dee6260c7311f6028a91cae21e768 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 17:08:52 -0400 Subject: [PATCH] fix(ui): hide the terminal dock while Settings is open (#114737) * fix(ui): hide docked terminal and browser panels during settings takeover * test(ui): align browser panel casts with core test typecheck * test(ui): split dock suppression coverage --- ui/src/app/app-host.dock-suppression.test.ts | 115 ++++++++++++++++ ui/src/app/app-host.ts | 2 + .../components/browser/browser-panel.test.ts | 90 +++++++++++++ ui/src/components/browser/browser-panel.ts | 8 ++ ui/src/components/dock-layout-controller.ts | 31 ++++- .../terminal-panel-suppression.test.ts | 127 ++++++++++++++++++ .../terminal/terminal-panel.test-support.ts | 70 ++++++++++ .../terminal/terminal-panel.test.ts | 78 ++--------- ui/src/components/terminal/terminal-panel.ts | 10 ++ 9 files changed, 463 insertions(+), 68 deletions(-) create mode 100644 ui/src/app/app-host.dock-suppression.test.ts create mode 100644 ui/src/components/terminal/terminal-panel-suppression.test.ts create mode 100644 ui/src/components/terminal/terminal-panel.test-support.ts diff --git a/ui/src/app/app-host.dock-suppression.test.ts b/ui/src/app/app-host.dock-suppression.test.ts new file mode 100644 index 000000000000..21603fc2df4b --- /dev/null +++ b/ui/src/app/app-host.dock-suppression.test.ts @@ -0,0 +1,115 @@ +/* @vitest-environment jsdom */ + +import { render as renderLit, type TemplateResult } from "lit"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { GatewayBrowserClient } from "../api/gateway.ts"; +import type { RouteId } from "../app-routes.ts"; +import { createStorageMock } from "../test-helpers/storage.ts"; +import { resetAppHostTestGlobals } from "./app-host.test-support.ts"; +import "./app-host.ts"; +import type { ApplicationRuntime } from "./bootstrap.ts"; +import type { ApplicationContext } from "./context.ts"; + +type ShellRenderState = { + runtime: ApplicationRuntime; + routeState: { routeId: RouteId }; + render: () => TemplateResult; +}; + +afterEach(() => { + resetAppHostTestGlobals(); +}); + +describe("OpenClaw shell dock suppression", () => { + it("suppresses docked panels only while a settings route owns the viewport", () => { + vi.stubGlobal("localStorage", createStorageMock()); + vi.stubGlobal( + "matchMedia", + vi.fn(() => ({ matches: false })), + ); + const client = {} as GatewayBrowserClient; + const context = { + basePath: "", + gateway: { + snapshot: { + phase: "connected", + client, + sessionKey: "agent:main:main", + assistantAgentId: "main", + hello: { + auth: { role: "operator", scopes: ["operator.admin"] }, + features: { methods: ["terminal.open", "browser.request"] }, + }, + lastError: null, + offlineStable: false, + selfUser: null, + }, + connection: { gatewayUrl: "ws://gateway.test", token: "", password: "" }, + connect: vi.fn(), + }, + agents: { state: { agentsList: null } }, + agentSelection: { state: { selectedId: "main" } }, + config: { + current: { terminalEnabled: true, serverVersion: null, devGitBranch: null }, + }, + runtimeConfig: { + state: { configSchema: null, configForm: null, configSnapshot: null, configUiHints: null }, + }, + sessions: { state: { result: null } }, + navigation: { + snapshot: { + navCollapsed: false, + navWidth: 280, + sidebarEntries: [], + pinnedAgentIds: [], + }, + update: vi.fn(), + }, + overlays: { + snapshot: { + updateAvailable: null, + updateRunning: false, + updateStatusBanner: null, + controlUiRefreshRequired: false, + approvalQueue: [], + approvalBusy: false, + approvalErrors: new Map(), + approvalNowMs: 0, + devicePairSetupOpen: false, + devicePairSetupLoading: false, + devicePairSetupError: null, + devicePairSetup: null, + devicePairSetupAccess: "full", + devicePairPendingCount: 0, + deviceAuthMigration: { error: null }, + }, + runUpdate: vi.fn(), + }, + theme: { mode: "dark" }, + preload: vi.fn(), + } as unknown as ApplicationContext; + const shell = document.createElement("openclaw-app-shell") as unknown as ShellRenderState; + shell.runtime = { context, router: {} } as unknown as ApplicationRuntime; + const container = document.createElement("div"); + + shell.routeState = { routeId: "config" }; + renderLit(shell.render(), container); + expect( + ( + container.querySelector("openclaw-terminal-panel") as HTMLElement & { + suppressed: boolean; + } + ).suppressed, + ).toBe(true); + + shell.routeState = { routeId: "chat" }; + renderLit(shell.render(), container); + expect( + ( + container.querySelector("openclaw-terminal-panel") as HTMLElement & { + suppressed: boolean; + } + ).suppressed, + ).toBe(false); + }); +}); diff --git a/ui/src/app/app-host.ts b/ui/src/app/app-host.ts index 64225e1cc53c..995ba45d2d14 100644 --- a/ui/src/app/app-host.ts +++ b/ui/src/app/app-host.ts @@ -2015,11 +2015,13 @@ class OpenClawShell extends OpenClawLightDomElement { { expect(panel.browserPanelIsOpen()).toBe(true); }); + it("suppresses an open dock without overwriting its persisted preference", async () => { + localStorage.setItem( + "openclaw.browser.panel.v1", + JSON.stringify({ open: true, dock: "right", height: 420, width: 560 }), + ); + const panel = document.createElement("openclaw-browser-panel") as unknown as HTMLElement & { + available: boolean; + suppressed: boolean; + renderRoot: ShadowRoot; + updateComplete: Promise; + }; + panel.available = true; + document.body.append(panel); + await panel.updateComplete; + + expect(panel.renderRoot.querySelector(".bp")).not.toBeNull(); + panel.suppressed = true; + await waitForFast(() => expect(panel.renderRoot.querySelector(".bp")).toBeNull()); + + expect(document.documentElement.style.getPropertyValue("--oc-browser-reserve-right")).toBe( + "0px", + ); + expect(JSON.parse(localStorage.getItem("openclaw.browser.panel.v1") ?? "{}")).toMatchObject({ + open: true, + }); + + panel.suppressed = false; + await waitForFast(() => expect(panel.renderRoot.querySelector(".bp")).not.toBeNull()); + + expect(panel.renderRoot.querySelector(".bp")).not.toBeNull(); + }); + + it("waits for availability before restoring after suppression", async () => { + localStorage.setItem( + "openclaw.browser.panel.v1", + JSON.stringify({ open: true, dock: "right", height: 420, width: 560 }), + ); + const panel = document.createElement("openclaw-browser-panel") as unknown as HTMLElement & { + available: boolean; + suppressed: boolean; + browserPanelIsOpen(): boolean; + updateComplete: Promise; + }; + panel.suppressed = true; + document.body.append(panel); + await panel.updateComplete; + + panel.suppressed = false; + await panel.updateComplete; + expect(panel.browserPanelIsOpen()).toBe(false); + + panel.available = true; + await waitForFast(() => expect(panel.browserPanelIsOpen()).toBe(true)); + }); + + it("mounts closed inside a takeover instead of refreshing a hidden dock", async () => { + localStorage.setItem( + "openclaw.browser.panel.v1", + JSON.stringify({ open: true, dock: "right", height: 420, width: 560 }), + ); + const requests: string[] = []; + const client = { + request: async (method: string) => { + requests.push(method); + return {} as T; + }, + } as GatewayBrowserClient; + const panel = document.createElement("openclaw-browser-panel") as unknown as HTMLElement & { + client: GatewayBrowserClient | null; + available: boolean; + suppressed: boolean; + browserPanelIsOpen(): boolean; + updateComplete: Promise; + }; + panel.client = client; + panel.available = true; + panel.suppressed = true; + document.body.append(panel); + await panel.updateComplete; + await Promise.resolve(); + + expect(panel.browserPanelIsOpen()).toBe(false); + expect(requests).toEqual([]); + + panel.suppressed = false; + await waitForFast(() => expect(panel.browserPanelIsOpen()).toBe(true)); + }); + it("keeps an already closed panel closed for an explicit close request", () => { const panel = document.createElement("openclaw-browser-panel") as unknown as HTMLElement & { available: boolean; diff --git a/ui/src/components/browser/browser-panel.ts b/ui/src/components/browser/browser-panel.ts index eb40d43b5335..4dafb1d0da71 100644 --- a/ui/src/components/browser/browser-panel.ts +++ b/ui/src/components/browser/browser-panel.ts @@ -42,6 +42,8 @@ class OpenClawBrowserPanel extends OpenClawLitElement implements BrowserPanelCon @property({ attribute: false }) client: GatewayBrowserClient | null = null; /** Whether the connected gateway advertises browser.request to this operator. */ @property({ type: Boolean }) available = false; + /** Full-page route takeovers (settings) own the viewport; the dock hides while one renders. */ + @property({ type: Boolean }) suppressed = false; /** Control UI base path, used for the authenticated media fetch. */ @property({ attribute: false }) basePath = ""; /** Bearer credential for the assistant-media screenshot fetch. */ @@ -60,6 +62,9 @@ class OpenClawBrowserPanel extends OpenClawLitElement implements BrowserPanelCon override connectedCallback(): void { super.connectedCallback(); window.addEventListener(BROWSER_PANEL_TOGGLE_EVENT, this.onToggleRequest); + // A settings takeover can already own the viewport when the panel mounts. + // Suppress before the restored open state refreshes a dock nobody can see. + this.dockLayout.setSuppressed(this.suppressed); if (this.dockLayout.open) { void this.browserPanelController.refreshAll(); } @@ -71,6 +76,9 @@ class OpenClawBrowserPanel extends OpenClawLitElement implements BrowserPanelCon } override updated(changed: Map): void { + if (changed.has("suppressed") && this.dockLayout.setSuppressed(this.suppressed)) { + void this.browserPanelController.refreshAll(); + } this.browserPanelController.synchronizeHostProperties(changed); if (changed.has("client") || changed.has("available")) { if (!this.available && this.dockLayout.open) { diff --git a/ui/src/components/dock-layout-controller.ts b/ui/src/components/dock-layout-controller.ts index c93ce899295f..57795a06771c 100644 --- a/ui/src/components/dock-layout-controller.ts +++ b/ui/src/components/dock-layout-controller.ts @@ -24,6 +24,7 @@ export class DockLayoutController implements Reacti height: number; width: number; + private suppressed = false; private resizeCleanup: (() => void) | null = null; private readonly onViewportResize = () => { const height = Math.min(this.height, this.options.layout.maxHeight()); @@ -80,8 +81,36 @@ export class DockLayoutController implements Reacti this.setOpen(false, false); } + /** + * Full-page route takeovers (settings) own the viewport, so docks hide while + * one renders. Hiding never persists — the user's open preference must survive + * the visit — and suppression also blocks `restoreOpenState()` so a reconnect + * mid-takeover cannot pop the panel back over settings. Returns true when the + * caller must resume its surface after the takeover ends. + * + * Only automatic restores are blocked. An explicit open (Ctrl+`, toolbar, + * `ui.command`) still wins and shows the dock over the takeover: swallowing a + * requested terminal would be a worse papercut than the one this fixes. + */ + setSuppressed(suppressed: boolean): boolean { + if (this.suppressed === suppressed) { + return false; + } + this.suppressed = suppressed; + if (suppressed) { + this.hideWithoutPersisting(); + return false; + } + return this.restoreOpenState(); + } + restoreOpenState(): boolean { - if (this.open || (!this.isFullscreen() && !this.options.layout.load().open)) { + if ( + this.suppressed || + !this.options.isAvailable() || + this.open || + (!this.isFullscreen() && !this.options.layout.load().open) + ) { return false; } this.open = true; diff --git a/ui/src/components/terminal/terminal-panel-suppression.test.ts b/ui/src/components/terminal/terminal-panel-suppression.test.ts new file mode 100644 index 000000000000..2633aea20297 --- /dev/null +++ b/ui/src/components/terminal/terminal-panel-suppression.test.ts @@ -0,0 +1,127 @@ +/* @vitest-environment jsdom */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { i18n } from "../../i18n/index.ts"; +import { createStorageMock } from "../../test-helpers/storage.ts"; +import { waitForFast } from "../../test-helpers/wait-for.ts"; +import type { TerminalGatewayClient } from "./terminal-connection.ts"; +import { + createTerminalController, + defineTestTerminalPanelElement, + terminalOpenResult, + type CreateGhosttyTerminalMock, +} from "./terminal-panel.test-support.ts"; +import { OpenClawTerminalPanel } from "./terminal-panel.ts"; + +const createGhosttyTerminalMock: CreateGhosttyTerminalMock = vi.fn(); +const TERMINAL_PANEL_ELEMENT_NAME = defineTestTerminalPanelElement(createGhosttyTerminalMock); + +describe("OpenClawTerminalPanel dock suppression", () => { + beforeEach(async () => { + vi.stubGlobal("localStorage", createStorageMock()); + vi.stubGlobal("sessionStorage", createStorageMock()); + await i18n.setLocale("en"); + }); + + afterEach(async () => { + document.body.replaceChildren(); + localStorage.clear(); + sessionStorage.clear(); + createGhosttyTerminalMock.mockReset(); + vi.unstubAllGlobals(); + await i18n.setLocale("en"); + }); + + it("suppresses an open dock without overwriting its persisted preference", async () => { + localStorage.setItem( + "openclaw.terminal.panel.v1", + JSON.stringify({ open: true, dock: "bottom", height: 320, width: 520 }), + ); + const panel = document.createElement(TERMINAL_PANEL_ELEMENT_NAME) as OpenClawTerminalPanel; + panel.available = true; + document.body.append(panel); + await panel.updateComplete; + + expect(panel.renderRoot.querySelector(".tp")).not.toBeNull(); + panel.suppressed = true; + await waitForFast(() => expect(panel.renderRoot.querySelector(".tp")).toBeNull()); + + expect(document.documentElement.style.getPropertyValue("--oc-terminal-reserve-bottom")).toBe( + "0px", + ); + expect(JSON.parse(localStorage.getItem("openclaw.terminal.panel.v1") ?? "{}")).toMatchObject({ + open: true, + }); + + panel.suppressed = false; + await waitForFast(() => expect(panel.renderRoot.querySelector(".tp")).not.toBeNull()); + + expect(panel.renderRoot.querySelector(".tp")).not.toBeNull(); + }); + + it("defers availability restore until suppression ends", async () => { + localStorage.setItem( + "openclaw.terminal.panel.v1", + JSON.stringify({ open: true, dock: "bottom", height: 320, width: 520 }), + ); + createGhosttyTerminalMock.mockResolvedValue(createTerminalController()); + const requests: string[] = []; + const client: TerminalGatewayClient = { + forceReconnect: () => {}, + request: async (method: string) => { + requests.push(method); + return (method === "terminal.open" ? terminalOpenResult("session-1") : {}) as T; + }, + addEventListener: () => () => {}, + }; + const panel = document.createElement(TERMINAL_PANEL_ELEMENT_NAME) as OpenClawTerminalPanel; + panel.client = client; + panel.suppressed = true; + document.body.append(panel); + await panel.updateComplete; + + panel.available = true; + await panel.updateComplete; + await Promise.resolve(); + + expect(panel.terminalPanelOpen).toBe(false); + expect(requests).not.toContain("terminal.open"); + + panel.suppressed = false; + await panel.updateComplete; + await waitForFast(() => expect(requests).toContain("terminal.open")); + + expect(panel.terminalPanelOpen).toBe(true); + }); + + it("mounts closed inside a takeover instead of booting a hidden session", async () => { + localStorage.setItem( + "openclaw.terminal.panel.v1", + JSON.stringify({ open: true, dock: "bottom", height: 320, width: 520 }), + ); + createGhosttyTerminalMock.mockResolvedValue(createTerminalController()); + const requests: string[] = []; + const client: TerminalGatewayClient = { + forceReconnect: () => {}, + request: async (method: string) => { + requests.push(method); + return (method === "terminal.open" ? terminalOpenResult("session-1") : {}) as T; + }, + addEventListener: () => () => {}, + }; + const panel = document.createElement(TERMINAL_PANEL_ELEMENT_NAME) as OpenClawTerminalPanel; + panel.client = client; + panel.available = true; + panel.suppressed = true; + document.body.append(panel); + await panel.updateComplete; + await Promise.resolve(); + + expect(panel.terminalPanelOpen).toBe(false); + expect(requests).not.toContain("terminal.open"); + + panel.suppressed = false; + await panel.updateComplete; + await waitForFast(() => expect(requests).toContain("terminal.open")); + }); +}); diff --git a/ui/src/components/terminal/terminal-panel.test-support.ts b/ui/src/components/terminal/terminal-panel.test-support.ts new file mode 100644 index 000000000000..d36b4c2a6360 --- /dev/null +++ b/ui/src/components/terminal/terminal-panel.test-support.ts @@ -0,0 +1,70 @@ +import { vi, type Mock } from "vitest"; +import { OpenClawTerminalPanel } from "./terminal-panel.ts"; + +export type CreateOptions = { + parent: HTMLElement; + terminalOptions?: { + fontFamily?: string; + theme?: { background?: string; foreground?: string }; + }; + onData?: (bytes: Uint8Array) => void; + onResize?: (size: { columns: number; rows: number }) => void; +}; + +export type CreateGhosttyTerminalMock = Mock< + (options: CreateOptions) => Promise> +>; + +export function createTerminalController(dispose: () => void = vi.fn()) { + const wasmTerm = {}; + const renderer = { + setTheme: vi.fn(), + render: vi.fn(), + }; + return { + readOnly: false, + terminal: { + cols: 100, + rows: 30, + viewportY: 0, + wasmTerm, + renderer, + write: vi.fn(), + focus: vi.fn(), + reset: vi.fn(), + paste: vi.fn(), + }, + write: vi.fn(), + fit: vi.fn(), + resize: vi.fn(), + setReadOnly: vi.fn(), + attach: vi.fn(), + dispose, + }; +} + +export function terminalOpenResult(sessionId: string) { + return { + sessionId, + agentId: "ops", + shell: "/bin/zsh", + cwd: "/work/ops", + confined: false, + }; +} + +export function defineTestTerminalPanelElement( + createGhosttyTerminalMock: CreateGhosttyTerminalMock, + tagName = `test-openclaw-terminal-panel-${crypto.randomUUID()}`, +): string { + type TerminalFactory = typeof import("./terminal-runtime.ts").createIsolatedGhosttyTerminal; + + // The full non-isolated UI suite can import the production panel before this + // test. Override its factory instead of relying on a module mock import order. + class TestTerminalPanel extends OpenClawTerminalPanel { + override createTerminalController = createGhosttyTerminalMock as unknown as TerminalFactory; + } + + customElements.define(tagName, TestTerminalPanel); + return tagName; +} diff --git a/ui/src/components/terminal/terminal-panel.test.ts b/ui/src/components/terminal/terminal-panel.test.ts index e23d52e1a607..eb37ef00aa65 100644 --- a/ui/src/components/terminal/terminal-panel.test.ts +++ b/ui/src/components/terminal/terminal-panel.test.ts @@ -1,66 +1,21 @@ /* @vitest-environment jsdom */ -import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { i18n } from "../../i18n/index.ts"; import { createStorageMock } from "../../test-helpers/storage.ts"; import { waitForFast } from "../../test-helpers/wait-for.ts"; import type { TerminalGatewayClient } from "./terminal-connection.ts"; - -type CreateOptions = { - parent: HTMLElement; - terminalOptions?: { - fontFamily?: string; - theme?: { background?: string; foreground?: string }; - }; - onData?: (bytes: Uint8Array) => void; - onResize?: (size: { columns: number; rows: number }) => void; -}; - -type CreateGhosttyTerminalMock = Mock< - (options: CreateOptions) => Promise> ->; -type TerminalFactory = typeof import("./terminal-runtime.ts").createIsolatedGhosttyTerminal; +import { + createTerminalController, + defineTestTerminalPanelElement, + terminalOpenResult, + type CreateGhosttyTerminalMock, + type CreateOptions, +} from "./terminal-panel.test-support.ts"; +import { OpenClawTerminalPanel } from "./terminal-panel.ts"; const createGhosttyTerminalMock: CreateGhosttyTerminalMock = vi.fn(); -function createTerminalController(dispose: () => void = vi.fn()) { - const wasmTerm = {}; - const renderer = { - setTheme: vi.fn(), - render: vi.fn(), - }; - return { - readOnly: false, - terminal: { - cols: 100, - rows: 30, - viewportY: 0, - wasmTerm, - renderer, - write: vi.fn(), - focus: vi.fn(), - reset: vi.fn(), - paste: vi.fn(), - }, - write: vi.fn(), - fit: vi.fn(), - resize: vi.fn(), - setReadOnly: vi.fn(), - attach: vi.fn(), - dispose, - }; -} - -function terminalOpenResult(sessionId: string) { - return { - sessionId, - agentId: "ops", - shell: "/bin/zsh", - cwd: "/work/ops", - confined: false, - }; -} - function deferred() { let resolve!: (value: T) => void; let reject!: (reason?: unknown) => void; @@ -71,17 +26,7 @@ function deferred() { return { promise, resolve, reject }; } -import { OpenClawTerminalPanel } from "./terminal-panel.ts"; - -const TERMINAL_PANEL_ELEMENT_NAME = `test-openclaw-terminal-panel-${crypto.randomUUID()}`; - -// The full non-isolated UI suite can import the production panel before this -// test. Override its factory instead of relying on a module mock import order. -class TestTerminalPanel extends OpenClawTerminalPanel { - override createTerminalController = createGhosttyTerminalMock as unknown as TerminalFactory; -} - -customElements.define(TERMINAL_PANEL_ELEMENT_NAME, TestTerminalPanel); +const TERMINAL_PANEL_ELEMENT_NAME = defineTestTerminalPanelElement(createGhosttyTerminalMock); async function startPanelWithPendingOpen() { let createOptions: CreateOptions | undefined; @@ -136,8 +81,7 @@ describe("OpenClawTerminalPanel", () => { element.available = true; document.body.append(element); - class LazyUpgradeTerminalPanel extends TestTerminalPanel {} - customElements.define(tagName, LazyUpgradeTerminalPanel); + defineTestTerminalPanelElement(createGhosttyTerminalMock, tagName); const panel = element as unknown as OpenClawTerminalPanel; await panel.updateComplete; await waitForFast(() => expect(panel.terminalPanelOpen).toBe(true)); diff --git a/ui/src/components/terminal/terminal-panel.ts b/ui/src/components/terminal/terminal-panel.ts index 4eb3643017b7..648446b6be13 100644 --- a/ui/src/components/terminal/terminal-panel.ts +++ b/ui/src/components/terminal/terminal-panel.ts @@ -58,6 +58,8 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { @property({ attribute: false }) agentId: string | null = null; /** Whether the connected gateway advertises the terminal surface. */ @property({ type: Boolean }) available = false; + /** Full-page route takeovers (settings) own the viewport; the dock hides while one renders. */ + @property({ type: Boolean }) suppressed = false; /** Active Control UI color mode, mirrored into the terminal theme. */ @property({ attribute: false }) themeMode: "dark" | "light" = "dark"; /** @@ -104,6 +106,9 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { override connectedCallback(): void { super.connectedCallback(); this.terminalSessions.connectHost(); + // A settings takeover can already own the viewport when the panel mounts. + // Suppress before the restored open state boots a session nobody can see. + this.dockLayout.setSuppressed(this.suppressed); if (!this.fullscreen) { window.addEventListener("keydown", this.onGlobalKeyDown); window.addEventListener(TERMINAL_PANEL_TOGGLE_EVENT, this.onToggleRequest); @@ -121,6 +126,11 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { } override updated(changed: Map): void { + if (changed.has("suppressed") && this.dockLayout.setSuppressed(this.suppressed)) { + // Restoring after a takeover: a reconnect during settings disposed the tabs + // without restoring them, so re-run the normal open path. + void this.terminalSessions.restoreSessions(); + } if (changed.has("client") || changed.has("available")) { this.terminalSessions.scheduleLifecycleSync(); }