From e4b03a7ddad9e520d4703303c3f08a71927b3bcd Mon Sep 17 00:00:00 2001 From: Yaswanth Narvaneni Date: Fri, 16 Oct 2026 11:21:21 +0100 Subject: [PATCH 1/2] Close mobile layers with the phone's Back On phones, Back left Roamgate even with the tab sheet, the input dock or a mobile view such as Files open. A small hook, useSystemBack, now gives open layers one history entry: Back closes the topmost layer (the tab sheet above the dock and mobile views; equal levels newest first) and the entry returns while layers remain. When the last layer closes on screen, the entry is removed once, even if several closed in the same commit, so the next Back leaves the page in one press. A layer opened while that removal is pending keeps its entry, and unmounting leaves history alone. Desktop is unchanged. The initial JS budget rises from 802 to 804 KiB for the hook. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AWodzpMyqdBcX25EE9HhDG --- FEATURES.md | 3 + scripts/check-web-assets.mjs | 2 +- web/src/App.tsx | 9 + .../components/TerminalView.inputMode.test.ts | 1 + web/src/components/TerminalView.tsx | 6 + web/src/systemBack.test.ts | 183 ++++++++++++++++++ web/src/systemBack.ts | 121 ++++++++++++ 7 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 web/src/systemBack.test.ts create mode 100644 web/src/systemBack.ts diff --git a/FEATURES.md b/FEATURES.md index e4c440fc..cb7d32e2 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -184,6 +184,9 @@ Drafts stay in this browser and re-anchor to changed files on refresh. (keys go straight to the terminal). Its tap raises the keyboard in either mode; afterwards, sends, scrolling, and shortcut actions leave a dismissed keyboard closed. +- The phone's Back gesture or button closes the tab sheet, then the input dock + or a mobile view such as Workspaces, Files, Changes or Ranger, one per + press, before leaving the page. - The `>_` button in Composer offers slash commands for Claude Code, Codex, Pi, Kimi Code, Grok Build, and Antigravity CLI. - Install as a PWA for an app-style window; see diff --git a/scripts/check-web-assets.mjs b/scripts/check-web-assets.mjs index 0b51727d..e071ac55 100644 --- a/scripts/check-web-assets.mjs +++ b/scripts/check-web-assets.mjs @@ -6,7 +6,7 @@ import { fileURLToPath, URL } from "node:url"; const publicRoot = fileURLToPath(new URL("../server/public/", import.meta.url)); const maxFileCount = 160; const maxTotalBytes = 14 * 1024 * 1024; -const maxInitialJsBytes = 803 * 1024; +const maxInitialJsBytes = 804 * 1024; const maxInitialJsGzipBytes = 256 * 1024; const maxInitialCssBytes = 196 * 1024; diff --git a/web/src/App.tsx b/web/src/App.tsx index 6f0e11ad..635c5f9d 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -30,6 +30,7 @@ import { } from "./mobileControlsPlacement"; import { useMobileControlsDrag } from "./components/useMobileControlsDrag"; import { LAYOUT_CHANGE_EVENT, useLayoutPreferences } from "./layoutPreferences"; +import { useSystemBack } from "./systemBack"; import { shortcutMatches, shortcutTitle, @@ -1675,6 +1676,14 @@ export default function App() { }); } }, [mobile, updateInspectorState]); + // The phone's Back closes the tab sheet, then returns from Workspaces, + // Files, Changes, Assistant or Annotations to the session. + useSystemBack( + mobileTabSheetOpen, + () => setMobileTabSheetOpen(false), + "popup", + ); + useSystemBack(mobileView !== "session", activateTerminalSurface, "panel"); const toggleSidebar = useCallback(() => { setMobileView("session"); setSidebarHidden((value) => !value); diff --git a/web/src/components/TerminalView.inputMode.test.ts b/web/src/components/TerminalView.inputMode.test.ts index 26882deb..0a743cfc 100644 --- a/web/src/components/TerminalView.inputMode.test.ts +++ b/web/src/components/TerminalView.inputMode.test.ts @@ -161,6 +161,7 @@ async function registerDomTests() { })); mock.module("../layoutPreferences", () => ({ isMobileLayout: () => true, + useLayoutPreferences: () => ({ mobile: true }), LAYOUT_CHANGE_EVENT: "test-layout-change", })); mock.module("./CreateWorkspaceDialog", () => ({ diff --git a/web/src/components/TerminalView.tsx b/web/src/components/TerminalView.tsx index 791c2aad..f5f8a2b2 100644 --- a/web/src/components/TerminalView.tsx +++ b/web/src/components/TerminalView.tsx @@ -17,6 +17,7 @@ import { isMobileLayout, LAYOUT_CHANGE_EVENT } from "../layoutPreferences"; import { terminalFontOptions } from "../appearance"; import { detectShortcutPlatform } from "../shortcutBindings"; import { PaneDragHandle, PaneMoveMenu } from "./PaneMoveMenu"; +import { useSystemBack } from "../systemBack"; import { getShortcutSnapshot, shortcutMatches, @@ -600,6 +601,11 @@ export function TerminalView({ }, [controlledComposerOpen, onComposerOpenChange], ); + useSystemBack( + composerOpen && isActivePane, + () => setComposerOpen(false), + "panel", + ); const agentHistoryOpen = controlledAgentHistoryOpen ?? localAgentHistoryOpen; const setAgentHistoryOpen = useCallback( (open: boolean) => { diff --git a/web/src/systemBack.test.ts b/web/src/systemBack.test.ts new file mode 100644 index 00000000..21c46630 --- /dev/null +++ b/web/src/systemBack.test.ts @@ -0,0 +1,183 @@ +import { afterAll, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import type { ReactNode } from "react"; + +// History and popstate are global, and the hook installs a module-level +// popstate listener; keep this DOM out of the shared test runner. +if (process.env.ROAMGATE_SYSTEM_BACK_DOM_TEST !== "1") { + test("system Back regressions in an isolated runtime", async () => { + const child = Bun.spawn([process.execPath, "test", import.meta.path], { + env: { ...process.env, ROAMGATE_SYSTEM_BACK_DOM_TEST: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + if (code !== 0) throw new Error(`${stdout}\n${stderr}`); + expect(code).toBe(0); + }, 15_000); +} else { + await registerDomTests(); +} + +async function registerDomTests() { + const browser = new Window({ url: "http://localhost/" }); + for (const [key, value] of Object.entries({ + window: browser, + document: browser.document, + navigator: browser.navigator, + HTMLElement: browser.HTMLElement, + Element: browser.Element, + Node: browser.Node, + Event: browser.Event, + CustomEvent: browser.CustomEvent, + localStorage: browser.localStorage, + sessionStorage: browser.sessionStorage, + IS_REACT_ACT_ENVIRONMENT: true, + })) + Object.defineProperty(globalThis, key, { + value, + writable: true, + configurable: true, + }); + afterAll(() => browser.happyDOM.abort()); + + const { act, createElement, Fragment, StrictMode } = await import("react"); + const { createRoot } = await import("react-dom/client"); + const { initializeLayoutPreferences, updateLayoutPreferences } = await import( + "./layoutPreferences" + ); + const { useSystemBack } = await import("./systemBack"); + + type Name = "sheet" | "composer" | "files"; + const levels = { sheet: "popup", composer: "panel", files: "panel" } as const; + let open = new Set(); + let closed: Name[] = []; + const container = document.createElement("div"); + document.body.append(container); + let root = createRoot(container); + + // Composer is a child of the sheet so React runs its effects first when + // both activate in one commit; stacking must not depend on that order. + function Layer({ name, children }: { name: Name; children?: ReactNode }) { + useSystemBack( + open.has(name), + () => { + closed.push(name); + show([...open].filter((candidate) => candidate !== name)); + }, + levels[name], + ); + return createElement(Fragment, null, children); + } + function render() { + root.render( + createElement( + StrictMode, + null, + createElement( + Layer, + { name: "sheet" }, + createElement(Layer, { name: "composer" }), + ), + createElement(Layer, { name: "files" }), + ), + ); + } + function show(names: Name[]) { + open = new Set(names); + render(); + } + const settle = () => + act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + async function set(names: Name[]) { + await act(async () => show(names)); + await settle(); + } + async function userBack() { + browser.history.back(); + await settle(); + } + const guarded = () => + typeof browser.history.state === "object" && + browser.history.state !== null && + "roamgateBack" in browser.history.state; + const at = () => browser.location.pathname; + // Every case starts on /b, reached from /a, with nothing open. + async function start() { + await set([]); + browser.history.pushState(null, "", "/a"); + browser.history.pushState(null, "", "/b"); + closed = []; + } + + initializeLayoutPreferences(); + updateLayoutPreferences({ mode: "mobile" }); + + test("one Back leaves the page after layers close together", async () => { + await start(); + await set(["files", "composer"]); + expect(guarded()).toBe(true); + await set([]); + expect([at(), guarded()]).toEqual(["/b", false]); + await userBack(); + expect(at()).toBe("/a"); + }); + + test("Back closes one layer at a time, mixed with on-screen closes", async () => { + await start(); + await set(["composer"]); + await set(["composer", "sheet"]); + await userBack(); + expect(closed).toEqual(["sheet"]); + expect([open.has("composer"), guarded()]).toEqual([true, true]); + await set([]); + expect([at(), guarded()]).toEqual(["/b", false]); + await userBack(); + expect(at()).toBe("/a"); + }); + + test("the higher level closes first when layers register together", async () => { + await start(); + await set(["sheet", "composer"]); + updateLayoutPreferences({ mode: "desktop" }); + await settle(); + expect(guarded()).toBe(false); + updateLayoutPreferences({ mode: "mobile" }); + await settle(); + await userBack(); + expect(closed).toEqual(["sheet"]); + expect(open.has("composer")).toBe(true); + await set([]); + }); + + test("a layer opened while a Back is pending keeps its entry", async () => { + await start(); + await set(["sheet"]); + await act(async () => { + show([]); + await Promise.resolve(); + show(["files"]); + }); + await settle(); + expect([open.has("files"), guarded(), at()]).toEqual([true, true, "/b"]); + await userBack(); + expect(closed).toEqual(["files"]); + await userBack(); + expect(at()).toBe("/a"); + }); + + test("unmounting leaves history alone", async () => { + await start(); + await set(["composer"]); + await act(async () => root.unmount()); + await settle(); + expect([guarded(), at()]).toEqual([true, "/b"]); + root = createRoot(container); + }); +} diff --git a/web/src/systemBack.ts b/web/src/systemBack.ts new file mode 100644 index 00000000..6a0fa520 --- /dev/null +++ b/web/src/systemBack.ts @@ -0,0 +1,121 @@ +import { useEffect, useRef } from "react"; +import { useLayoutPreferences } from "./layoutPreferences"; + +/** + * How a layer stacks for the phone's Back: a panel (Composer, a mobile view + * such as Workspaces or Files) sits below a popup (the tab sheet). Back + * closes the highest open layer; equal levels close newest first. + */ +export type SystemBackLevel = "panel" | "popup"; +const LEVELS: Record = { panel: 0, popup: 1 }; + +type Layer = { level: number; order: number; close: () => void }; + +/** + * Lets the phone's Back gesture or button close an open mobile layer before + * it leaves the page. While any layer is open, the current history entry is + * one guard entry at the current URL. Back pops it and closes only the top + * layer; the guard returns while layers remain. When the last layer closes + * on screen, the guard is removed again, so the next Back leaves the page in + * one press. + */ +export function useSystemBack( + open: boolean, + close: () => void, + level: SystemBackLevel, +) { + const { mobile } = useLayoutPreferences(); + const closeRef = useRef(close); + closeRef.current = close; + const active = mobile && open; + // Declared first so its cleanup runs first: unmounting the app is not the + // user closing a layer, and must not move history on its own. + const unmounting = useRef(false); + useEffect(() => { + unmounting.current = false; + return () => { + unmounting.current = true; + }; + }, []); + + useEffect(() => { + if (!active) return; + const layer: Layer = { + level: LEVELS[level], + order: ++openings, + close: () => closeRef.current(), + }; + layers.add(layer); + scheduleReconcile(); + return () => { + layers.delete(layer); + if (!unmounting.current) closedOnScreen = true; + scheduleReconcile(); + }; + }, [active, level]); +} + +const GUARD = "roamgateBack"; +const layers = new Set(); +let openings = 0; +// Whether a layer closed since the last reconcile other than by unmounting. +let closedOnScreen = false; +let reconcileScheduled = false; +// history.back() is asynchronous; until our own Back lands, the current entry +// is stale and a popstate is ours, not the user's. +let backPending = false; + +function isGuard(state: unknown) { + return typeof state === "object" && state !== null && GUARD in state; +} + +// Effects of one React commit run together; reconcile once after all of them, +// so layers that open and close in the same commit settle as one change. +function scheduleReconcile() { + if (reconcileScheduled) return; + reconcileScheduled = true; + queueMicrotask(reconcile); +} + +function reconcile() { + reconcileScheduled = false; + if (backPending) return; + const guarded = isGuard(window.history.state); + if (layers.size > 0 && !guarded) { + const base: unknown = window.history.state; + window.history.pushState( + { ...(typeof base === "object" && base ? base : {}), [GUARD]: true }, + "", + ); + } else if (layers.size === 0 && guarded && closedOnScreen) { + backPending = true; + window.history.back(); + } + closedOnScreen = false; +} + +if (typeof window !== "undefined") + window.addEventListener("popstate", (event) => { + if (backPending) { + backPending = false; + reconcile(); + return; + } + // Forward onto a guard, or Back with nothing open: settle the entry. + if (isGuard(event.state) || layers.size === 0) { + scheduleReconcile(); + return; + } + let top: Layer | undefined; + for (const layer of layers) + if ( + !top || + layer.level > top.level || + (layer.level === top.level && layer.order > top.order) + ) + top = layer; + // The user's Back already removed the guard: nothing to undo. + layers.delete(top!); + top!.close(); + scheduleReconcile(); + }); From 84a7d6f802b88e0dd2899444ebb4149dcf0a35bf Mon Sep 17 00:00:00 2001 From: Wangshuyi Date: Sat, 17 Oct 2026 10:12:27 +0800 Subject: [PATCH 2/2] Fix mobile Back layer ordering and history cleanup Coordinate dropdown and panel Back handling, ignore hidden Composer layers, and clean up guards on remount and reload. Preserve navigation through obsolete guards in both directions and add regression coverage. --- web/src/App.tsx | 7 + .../components/TerminalView.inputMode.test.ts | 202 +++++++++++++++++- web/src/components/TerminalView.tsx | 4 +- web/src/components/ThemedSelect.test.ts | 64 +++++- web/src/components/ThemedSelect.tsx | 7 +- web/src/systemBack.test.ts | 140 ++++++++++-- web/src/systemBack.ts | 90 ++++++-- 7 files changed, 472 insertions(+), 42 deletions(-) diff --git a/web/src/App.tsx b/web/src/App.tsx index 635c5f9d..7968b5f8 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -281,6 +281,7 @@ function PopupOverlay({ type TerminalViewProps = { paneId?: string; + visible?: boolean; terminalTheme: ITheme; terminalFontFamily: string; terminalFontScale: number; @@ -1004,6 +1005,7 @@ function resizeTargetForSplit( // Render the active tab's Herdr pane layout; single-pane and zoomed tabs keep // the old full terminal view. function TerminalPaneLayout({ + visible, terminalTheme, terminalFontFamily, terminalFontScale, @@ -1017,6 +1019,7 @@ function TerminalPaneLayout({ zenMode, onExitZenMode, }: { + visible: boolean; terminalTheme: ITheme; terminalFontFamily: string; terminalFontScale: number; @@ -1075,6 +1078,7 @@ function TerminalPaneLayout({ terminalFontScale={terminalFontScale} mobileShortcuts={mobileShortcuts} mobileSideShortcuts={mobileSideShortcuts} + visible={visible} composerOpen={composerOpen} onComposerOpenChange={onComposerOpenChange} agentHistoryOpen={agentHistoryOpen} @@ -1134,6 +1138,7 @@ function TerminalPaneLayout({ terminalFontScale={terminalFontScale} mobileShortcuts={mobileShortcuts} mobileSideShortcuts={mobileSideShortcuts} + visible={visible} composerOpen={composerOpen} onComposerOpenChange={onComposerOpenChange} agentHistoryOpen={agentHistoryOpen} @@ -1244,6 +1249,7 @@ function TerminalPaneLayout({ showMobileKeys={isActive} mobileShortcuts={mobileShortcuts} mobileSideShortcuts={mobileSideShortcuts} + visible={visible} composerOpen={isActive ? composerOpen : false} onComposerOpenChange={isActive ? onComposerOpenChange : undefined} agentHistoryOpen={isActive ? agentHistoryOpen : false} @@ -4273,6 +4279,7 @@ export default function App() { >
void)[] = []; + const replaceLocation = browser.history.replaceState.bind(browser.history); + Object.defineProperty(browser, "history", { + value: { + get state() { + return entries[historyIndex]!.state; + }, + pushState(state: object | null, _unused: string, url?: string) { + entries.splice(historyIndex + 1); + entries.push({ state, url: url ?? entries[historyIndex]!.url }); + historyIndex++; + replaceLocation(state, "", entries[historyIndex]!.url); + }, + replaceState(state: object | null, _unused: string, url?: string) { + entries[historyIndex] = { + state, + url: url ?? entries[historyIndex]!.url, + }; + replaceLocation(state, "", entries[historyIndex]!.url); + }, + back() { + traversals.push(() => { + if (historyIndex === 0) return; + const entry = entries[--historyIndex]!; + replaceLocation(entry.state, "", entry.url); + browser.dispatchEvent( + new browser.PopStateEvent("popstate", { state: entry.state }), + ); + }); + }, + }, + }); + async function settleHistory() { + for (let turn = 0; turn < 8; turn++) { + await act(async () => { + await Promise.resolve(); + for (const complete of traversals.splice(0)) complete(); + }); + if (!traversals.length) return; + } + throw new Error("history traversal loop"); + } + let mobileLayout = true; const noop = () => {}; const disposable = () => ({ dispose: noop }); type Pane = { @@ -160,8 +208,8 @@ async function registerDomTests() { }, })); mock.module("../layoutPreferences", () => ({ - isMobileLayout: () => true, - useLayoutPreferences: () => ({ mobile: true }), + isMobileLayout: () => mobileLayout, + useLayoutPreferences: () => ({ mobile: mobileLayout }), LAYOUT_CHANGE_EVENT: "test-layout-change", })); mock.module("./CreateWorkspaceDialog", () => ({ @@ -259,6 +307,7 @@ async function registerDomTests() { } mock.module("@xterm/xterm", () => ({ Terminal: FakeTerminal })); const { TerminalView } = await import("./TerminalView"); + const { useSystemBack } = await import("../systemBack"); const { TERMINAL_INPUT_MODE_STORAGE_KEY } = await import( "../terminalInputMode" ); @@ -278,6 +327,10 @@ async function registerDomTests() { let root: Root | null = null; let container: HTMLDivElement; let open = false; + let visible = true; + let filesOpen = false; + let tabsOpen = false; + let viewKey = "initial"; let strict = false; let explicitPane: string | undefined; let terminalFontScale = 100; @@ -293,7 +346,12 @@ async function registerDomTests() { localStorage.clear(); sessionStorage.clear(); open = false; + visible = true; + mobileLayout = true; + filesOpen = false; + tabsOpen = false; strict = false; + viewKey = "initial"; explicitPane = undefined; terminalFontScale = 100; mobileShortcuts = [[], []]; @@ -301,6 +359,7 @@ async function registerDomTests() { afterEach(async () => { document.documentElement.classList.remove("keyboard-open"); if (root) await act(async () => root?.unmount()); + await settleHistory(); root = null; clearTerminalComposerDraft(draftKey); document.documentElement.classList.remove("keyboard-open"); @@ -315,8 +374,35 @@ async function registerDomTests() { else Reflect.deleteProperty(globalThis, key); } }); + // Model App's mobile panels around the real TerminalView, which stays + // mounted when CSS hides the terminal surface behind Files. + function MobilePanels({ + children, + }: { + children?: import("react").ReactNode; + }) { + useSystemBack( + tabsOpen, + () => { + tabsOpen = false; + render(); + }, + "popup", + ); + useSystemBack( + filesOpen, + () => { + filesOpen = false; + visible = true; + render(); + }, + "panel", + ); + return children; + } function render() { const view = createElement(TerminalView, { + key: viewKey, paneId: explicitPane, terminalTheme: {}, terminalFontFamily: "monospace", @@ -324,13 +410,33 @@ async function registerDomTests() { showMobileKeys: false, mobileShortcuts, mobileSideShortcuts: [], + visible, composerOpen: open, onComposerOpenChange(next: boolean) { open = next; render(); }, }); - root!.render(strict ? createElement(StrictMode, null, view) : view); + // App replaces its mobile single-pane branch with a desktop split tree. + const layout = mobileLayout + ? view + : createElement( + "div", + { key: "split" }, + view, + createElement(TerminalView, { + paneId: "pane-b", + terminalTheme: {}, + terminalFontFamily: "monospace", + terminalFontScale, + showMobileKeys: false, + mobileShortcuts, + mobileSideShortcuts: [], + composerOpen: false, + }), + ); + const panels = createElement(MobilePanels, null, layout); + root!.render(strict ? createElement(StrictMode, null, panels) : panels); } async function mount(initiallyOpen = false, strictMode = false) { open = initiallyOpen; @@ -407,6 +513,96 @@ async function registerDomTests() { }); } + async function back() { + browser.history.back(); + await settleHistory(); + } + + test.each([false, true])( + "only a visible mounted Composer consumes Back (visible %s)", + async (surfaceVisible) => { + browser.history.pushState(null, "", "/previous"); + browser.history.pushState(null, "", "/terminal"); + visible = surfaceVisible; + await mount(true); + const term = currentTerminal(); + expect(container.querySelector(".terminal-composer")).not.toBeNull(); + await back(); + expect(open).toBe(!surfaceVisible); + expect(browser.location.pathname).toBe( + surfaceVisible ? "/terminal" : "/previous", + ); + expect(currentTerminal()).toBe(term); + }, + ); + + test("multi-pane mobile to desktop replacement removes the Composer guard", async () => { + browser.history.pushState(null, "", "/previous"); + browser.history.pushState(null, "", "/terminal"); + await mount(true, true); + const mobileTerminal = currentTerminal(); + await act(async () => { + mobileLayout = false; + render(); + }); + await settleHistory(); + expect(currentTerminal()).not.toBe(mobileTerminal); + expect(browser.history.state).not.toHaveProperty("roamgateBack"); + expect(open).toBe(true); + await back(); + expect(browser.location.pathname).toBe("/previous"); + expect(open).toBe(true); + }); + + test.each([false, true])( + "Type, Files, Tabs, Back, Back restores Composer (hidden pane remount %s)", + async (remount) => { + browser.history.pushState(null, "", "/terminal"); + writeTerminalComposerDraft(draftKey, "unsent draft"); + await mount(); + await typeGesture(); + let term = currentTerminal(); + await act(async () => { + visible = false; + filesOpen = true; + render(); + }); + if (remount) { + // A tab/pane replacement while Files covers the terminal must not + // re-register its still-open, but invisible Composer above Files. + await act(async () => { + viewKey = "replacement"; + render(); + }); + expect(currentTerminal()).not.toBe(term); + term = currentTerminal(); + } + await act(async () => { + tabsOpen = true; + render(); + }); + await back(); + expect(tabsOpen).toBe(false); + expect(filesOpen).toBe(true); + expect(open).toBe(true); + await back(); + expect(filesOpen).toBe(false); + expect(visible).toBe(true); + expect(open).toBe(true); + expect(currentTerminal()).toBe(term); + expect( + container.querySelector( + '[aria-label="Terminal input draft"]', + )!.value, + ).toBe("unsent draft"); + expect(readTerminalComposerDraft(draftKey)).toBe("unsent draft"); + expect(selectedMode()).toBe("composer"); + expectInputBlocked(); + await back(); + expect(open).toBe(false); + }, + ); + test.each(["closed", "direct", "composer"] as const)( "terminal focus activates %s input after touch on a coarse-pointer desktop", async (mode) => { diff --git a/web/src/components/TerminalView.tsx b/web/src/components/TerminalView.tsx index f5f8a2b2..63761015 100644 --- a/web/src/components/TerminalView.tsx +++ b/web/src/components/TerminalView.tsx @@ -313,6 +313,7 @@ export type TerminalWorkspaceFileRequest = { export function TerminalView({ paneId, + visible = true, terminalTheme, terminalFontFamily, terminalFontScale, @@ -326,6 +327,7 @@ export function TerminalView({ onOpenWorkspaceFile, }: { paneId?: string; + visible?: boolean; terminalTheme: ITheme; terminalFontFamily: string; terminalFontScale: number; @@ -602,7 +604,7 @@ export function TerminalView({ [controlledComposerOpen, onComposerOpenChange], ); useSystemBack( - composerOpen && isActivePane, + visible && composerOpen && isActivePane, () => setComposerOpen(false), "panel", ); diff --git a/web/src/components/ThemedSelect.test.ts b/web/src/components/ThemedSelect.test.ts index 19cde185..bd09901a 100644 --- a/web/src/components/ThemedSelect.test.ts +++ b/web/src/components/ThemedSelect.test.ts @@ -51,7 +51,7 @@ async function registerDomTest() { cancelAnimationFrame: browser.cancelAnimationFrame.bind(browser), IS_REACT_ACT_ENVIRONMENT: true, }); - const { act, createElement } = await import("react"); + const { act, createElement, StrictMode, useState } = await import("react"); const { createRoot } = await import("react-dom/client"); const { ThemedSelect } = await import("./ThemedSelect"); afterAll(async () => browser.happyDOM.close()); @@ -116,6 +116,68 @@ async function registerDomTest() { } }); + test("mobile Back closes the real select before its underlying panel in StrictMode", async () => { + const { updateLayoutPreferences } = await import("../layoutPreferences"); + const { useSystemBack } = await import("../systemBack"); + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + let panelCloses = 0; + function Panel() { + const [open, setOpen] = useState(true); + useSystemBack( + open, + () => { + panelCloses++; + setOpen(false); + }, + "panel", + ); + return open + ? createElement(ThemedSelect, { + value: "first", + options: [{ value: "first", label: "First" }], + onChange: () => {}, + "aria-label": "Mobile select", + }) + : null; + } + const back = async () => { + await act(async () => { + // A browser Back lands on the unguarded base before firing popstate. + browser.history.replaceState(null, ""); + browser.dispatchEvent( + new browser.PopStateEvent("popstate", { state: null }), + ); + }); + await act(async () => browser.happyDOM.whenAsyncComplete()); + }; + try { + await act(async () => updateLayoutPreferences({ mode: "mobile" })); + await act(async () => + root.render(createElement(StrictMode, null, createElement(Panel))), + ); + const trigger = container.querySelector("button")!; + await click(trigger); + expect(document.querySelector(".themed-select-content")).not.toBeNull(); + await back(); + expect(document.querySelector(".themed-select-content")).toBeNull(); + expect(panelCloses).toBe(0); + expect(document.activeElement).toBe(trigger); + expect( + browser.history.state && "roamgateBack" in browser.history.state, + ).toBe(true); + await back(); + expect(panelCloses).toBe(1); + expect(container.querySelector("button")).toBeNull(); + } finally { + await act(async () => root.unmount()); + await act(async () => updateLayoutPreferences({ mode: "desktop" })); + container.remove(); + await browser.happyDOM.whenAsyncComplete(); + } + }); + const cleanups: (() => Promise)[] = []; afterEach(async () => { for (const cleanup of cleanups.splice(0)) await cleanup(); diff --git a/web/src/components/ThemedSelect.tsx b/web/src/components/ThemedSelect.tsx index ac40781e..889a81ea 100644 --- a/web/src/components/ThemedSelect.tsx +++ b/web/src/components/ThemedSelect.tsx @@ -1,5 +1,7 @@ import { Check, ChevronDown } from "lucide-react"; import { useEffect, useRef, useState, type ReactNode } from "react"; +import { useLayoutPreferences } from "../layoutPreferences"; +import { useSystemBack } from "../systemBack"; import { cn } from "../utils"; import { Command, @@ -58,11 +60,14 @@ export function ThemedSelect({ const [highlighted, setHighlighted] = useState(value); const selected = useRef(false); const listRef = useRef(null); + const { mobile } = useLayoutPreferences(); + useSystemBack(open && !disabled, () => setOpen(false), "popup"); useEffect(() => { + if (mobile) return; const close = () => setOpen(false); window.addEventListener("popstate", close); return () => window.removeEventListener("popstate", close); - }, []); + }, [mobile]); useEffect(() => { if (disabled) setOpen(false); }, [disabled]); diff --git a/web/src/systemBack.test.ts b/web/src/systemBack.test.ts index 21c46630..ca205995 100644 --- a/web/src/systemBack.test.ts +++ b/web/src/systemBack.test.ts @@ -50,6 +50,50 @@ async function registerDomTests() { const { initializeLayoutPreferences, updateLayoutPreferences } = await import( "./layoutPreferences" ); + // Model only standard history operations. Traversals complete on an event, + // never a wall-clock sleep (happy-dom does not implement session history). + let entries: { state: object | null; url: string }[] = [ + { state: null, url: "/" }, + ]; + let index = 0; + const traversals: (() => void)[] = []; + const history = { + get state() { + return entries[index]!.state; + }, + get length() { + return entries.length; + }, + pushState(state: object | null, _unused: string, url?: string) { + entries = entries.slice(0, index + 1); + entries.push({ state, url: url ?? entries[index]!.url }); + index++; + }, + replaceState(state: object | null, _unused: string, url?: string) { + entries[index] = { state, url: url ?? entries[index]!.url }; + }, + back() { + move(-1); + }, + forward() { + move(1); + }, + }; + function move(delta: number) { + traversals.push(() => { + const next = index + delta; + if (next < 0 || next >= entries.length) return; + index = next; + browser.dispatchEvent( + new browser.PopStateEvent("popstate", { state: history.state }), + ); + }); + } + Object.defineProperty(browser, "history", { value: history }); + // Simulate a refresh starting on a guard, before the module is imported. + history.pushState(null, "", "/a"); + history.pushState(null, "", "/b"); + history.pushState({ roamgateBack: true }, ""); const { useSystemBack } = await import("./systemBack"); type Name = "sheet" | "composer" | "files"; @@ -91,10 +135,20 @@ async function registerDomTests() { open = new Set(names); render(); } - const settle = () => - act(async () => { - await new Promise((resolve) => setTimeout(resolve, 30)); - }); + const settle = async () => { + // Each traversal can schedule a React commit and one guard reconcile. + for (let turn = 0; turn < 8; turn++) { + await act(async () => { + await Promise.resolve(); + for (const complete of traversals.splice(0)) complete(); + }); + if (!traversals.length) { + await Promise.resolve(); + if (!traversals.length) return; + } + } + throw new Error("history traversal loop"); + }; async function set(names: Name[]) { await act(async () => show(names)); await settle(); @@ -107,7 +161,7 @@ async function registerDomTests() { typeof browser.history.state === "object" && browser.history.state !== null && "roamgateBack" in browser.history.state; - const at = () => browser.location.pathname; + const at = () => entries[index]!.url; // Every case starts on /b, reached from /a, with nothing open. async function start() { await set([]); @@ -119,6 +173,13 @@ async function registerDomTests() { initializeLayoutPreferences(); updateLayoutPreferences({ mode: "mobile" }); + test("refresh on an empty guard returns to its base without a duplicate Back stop", async () => { + await settle(); + expect([at(), guarded()]).toEqual(["/b", false]); + await userBack(); + expect(at()).toBe("/a"); + }); + test("one Back leaves the page after layers close together", async () => { await start(); await set(["files", "composer"]); @@ -145,10 +206,10 @@ async function registerDomTests() { test("the higher level closes first when layers register together", async () => { await start(); await set(["sheet", "composer"]); - updateLayoutPreferences({ mode: "desktop" }); + await act(async () => updateLayoutPreferences({ mode: "desktop" })); await settle(); expect(guarded()).toBe(false); - updateLayoutPreferences({ mode: "mobile" }); + await act(async () => updateLayoutPreferences({ mode: "mobile" })); await settle(); await userBack(); expect(closed).toEqual(["sheet"]); @@ -159,11 +220,9 @@ async function registerDomTests() { test("a layer opened while a Back is pending keeps its entry", async () => { await start(); await set(["sheet"]); - await act(async () => { - show([]); - await Promise.resolve(); - show(["files"]); - }); + await act(async () => show([])); + expect(traversals).toHaveLength(1); + await act(async () => show(["files"])); await settle(); expect([open.has("files"), guarded(), at()]).toEqual([true, true, "/b"]); await userBack(); @@ -172,12 +231,65 @@ async function registerDomTests() { expect(at()).toBe("/a"); }); - test("unmounting leaves history alone", async () => { + test("Forward at the end still lets one Back leave the empty guard", async () => { + await start(); + await set(["composer"]); + await set([]); + const length = history.length; + history.forward(); + await settle(); + expect([at(), guarded(), history.length]).toEqual(["/b", true, length]); + await userBack(); + expect(at()).toBe("/a"); + }); + + test("Forward and Back cross an empty guard without trapping later entries", async () => { + await start(); + await set(["composer"]); + // A later same-document entry can remain beyond the obsolete guard. + history.pushState({ later: true }, "", "/c"); + await set([]); + await userBack(); + expect([at(), guarded()]).toEqual(["/b", false]); + expect(entries[entries.length - 1]).toEqual({ + state: { later: true }, + url: "/c", + }); + history.forward(); + await settle(); + expect(at()).toBe("/c"); + await userBack(); + expect([at(), guarded()]).toEqual(["/b", false]); + await userBack(); + expect(at()).toBe("/a"); + }); + + test("pagehide does not traverse during document departure", async () => { await start(); await set(["composer"]); + browser.dispatchEvent(new browser.Event("pagehide")); await act(async () => root.unmount()); await settle(); - expect([guarded(), at()]).toEqual([true, "/b"]); + expect(guarded()).toBe(true); + browser.dispatchEvent(new browser.Event("pageshow")); + await settle(); + expect(guarded()).toBe(false); root = createRoot(container); }); + + test("StrictMode unmount and layout remount remove an unused guard", async () => { + await start(); + await set(["composer"]); + await act(async () => root.unmount()); + await settle(); + expect([guarded(), at()]).toEqual([false, "/b"]); + root = createRoot(container); + await act(async () => updateLayoutPreferences({ mode: "desktop" })); + await set(["composer"]); + expect(guarded()).toBe(false); + await userBack(); + expect(at()).toBe("/a"); + await act(async () => updateLayoutPreferences({ mode: "mobile" })); + await set([]); + }); } diff --git a/web/src/systemBack.ts b/web/src/systemBack.ts index 6a0fa520..553321de 100644 --- a/web/src/systemBack.ts +++ b/web/src/systemBack.ts @@ -28,15 +28,6 @@ export function useSystemBack( const closeRef = useRef(close); closeRef.current = close; const active = mobile && open; - // Declared first so its cleanup runs first: unmounting the app is not the - // user closing a layer, and must not move history on its own. - const unmounting = useRef(false); - useEffect(() => { - unmounting.current = false; - return () => { - unmounting.current = true; - }; - }, []); useEffect(() => { if (!active) return; @@ -49,17 +40,20 @@ export function useSystemBack( scheduleReconcile(); return () => { layers.delete(layer); - if (!unmounting.current) closedOnScreen = true; scheduleReconcile(); }; }, [active, level]); } const GUARD = "roamgateBack"; +const BASE = "roamgateBackBase"; const layers = new Set(); let openings = 0; -// Whether a layer closed since the last reconcile other than by unmounting. -let closedOnScreen = false; +let guards = 0; +let previousState: unknown; +let forwardBase: string | undefined; +// A real document departure is not an on-screen React unmount. +let leaving = false; let reconcileScheduled = false; // history.back() is asynchronous; until our own Back lands, the current entry // is stale and a popstate is ours, not the user's. @@ -69,6 +63,12 @@ function isGuard(state: unknown) { return typeof state === "object" && state !== null && GUARD in state; } +function baseId(state: unknown): string | undefined { + if (typeof state !== "object" || state === null || !(BASE in state)) return; + const value = (state as Record)[BASE]; + return typeof value === "string" ? value : undefined; +} + // Effects of one React commit run together; reconcile once after all of them, // so layers that open and close in the same commit settle as one change. function scheduleReconcile() { @@ -79,29 +79,74 @@ function scheduleReconcile() { function reconcile() { reconcileScheduled = false; - if (backPending) return; + if (backPending || leaving) return; const guarded = isGuard(window.history.state); + if (layers.size > 0) forwardBase = undefined; if (layers.size > 0 && !guarded) { - const base: unknown = window.history.state; - window.history.pushState( - { ...(typeof base === "object" && base ? base : {}), [GUARD]: true }, - "", - ); - } else if (layers.size === 0 && guarded && closedOnScreen) { + const state: unknown = window.history.state; + // Pair the entries so an obsolete guard can be crossed in either direction. + const base = { + ...(typeof state === "object" && state ? state : {}), + [BASE]: `${Date.now()}:${++guards}`, + }; + window.history.replaceState(base, ""); + window.history.pushState({ ...base, [GUARD]: true }, ""); + previousState = window.history.state; + } else if (layers.size === 0 && guarded && forwardBase === undefined) { backPending = true; window.history.back(); } - closedOnScreen = false; } -if (typeof window !== "undefined") +if (typeof window !== "undefined") { + previousState = window.history.state; + window.addEventListener("pagehide", () => { + leaving = true; + forwardBase = undefined; + }); + window.addEventListener("pageshow", () => { + leaving = false; + previousState = window.history.state; + scheduleReconcile(); + }); + // A restored/reloaded guard has no UI state of its own. Return to its base + // without pushing another entry or truncating valid Forward history. + scheduleReconcile(); window.addEventListener("popstate", (event) => { + const from = previousState; + previousState = event.state; if (backPending) { backPending = false; reconcile(); return; } - // Forward onto a guard, or Back with nothing open: settle the entry. + if (forwardBase !== undefined) { + const id = forwardBase; + forwardBase = undefined; + if ( + layers.size === 0 && + !isGuard(event.state) && + baseId(event.state) === id + ) { + backPending = true; + window.history.back(); + return; + } + } + const id = baseId(event.state); + if ( + layers.size === 0 && + isGuard(event.state) && + id !== undefined && + !isGuard(from) && + baseId(from) === id + ) { + // Continue Forward past an empty guard. At the end, its next Back skips + // the paired base instead of consuming an extra press. + forwardBase = id; + window.history.forward(); + return; + } if (isGuard(event.state) || layers.size === 0) { scheduleReconcile(); return; @@ -119,3 +164,4 @@ if (typeof window !== "undefined") top!.close(); scheduleReconcile(); }); +}