diff --git a/apps/desktop/src/main/browser/app-shortcut-relay.test.ts b/apps/desktop/src/main/browser/app-shortcut-relay.test.ts new file mode 100644 index 000000000..85a199076 --- /dev/null +++ b/apps/desktop/src/main/browser/app-shortcut-relay.test.ts @@ -0,0 +1,87 @@ +import type { BrowserWindow } from "electron"; +import type { BrowserAppShortcutEvent } from "@volli/shared"; +import { describe, expect, it, vi } from "vite-plus/test"; + +import { createBrowserShortcutRelay } from "./app-shortcut-relay"; + +const EVENT: BrowserAppShortcutEvent = { tabId: "opaque-1", shortcut: { kind: "close-tab" } }; + +function appWindow(destroyed = false) { + const order: string[] = []; + const window = { + isDestroyed: () => destroyed, + webContents: { focus: vi.fn(() => order.push("focus")) }, + }; + return { window: window as unknown as BrowserWindow, focus: window.webContents.focus, order }; +} + +describe("createBrowserShortcutRelay (VC-301)", () => { + it("gives the app window focus before any subscriber hears the shortcut", () => { + const relay = createBrowserShortcutRelay(vi.fn()); + const { window, order } = appWindow(); + relay.subscribe(() => order.push("heard")); + + relay.deliver(window, EVENT); + + expect(order).toEqual(["focus", "heard"]); + }); + + it("hands every subscriber the shortcut until it unsubscribes", () => { + const relay = createBrowserShortcutRelay(vi.fn()); + const { window } = appWindow(); + const first = vi.fn(); + const second = vi.fn(); + const stopFirst = relay.subscribe(first); + relay.subscribe(second); + + relay.deliver(window, EVENT); + stopFirst(); + relay.deliver(window, EVENT); + + expect(first).toHaveBeenCalledOnce(); + expect(second).toHaveBeenCalledTimes(2); + expect(second).toHaveBeenCalledWith(EVENT); + }); + + it("keeps the same function subscribed twice as two streams", () => { + const relay = createBrowserShortcutRelay(vi.fn()); + const { window } = appWindow(); + const listener = vi.fn(); + const stop = relay.subscribe(listener); + relay.subscribe(listener); + stop(); + + relay.deliver(window, EVENT); + + expect(listener).toHaveBeenCalledOnce(); + }); + + it("reports a subscriber that throws and still reaches the rest", () => { + const onListenerError = vi.fn(); + const relay = createBrowserShortcutRelay(onListenerError); + const { window } = appWindow(); + const failure = new Error("stream gone"); + relay.subscribe(() => { + throw failure; + }); + const after = vi.fn(); + relay.subscribe(after); + + relay.deliver(window, EVENT); + + expect(onListenerError).toHaveBeenCalledWith(failure); + expect(after).toHaveBeenCalledWith(EVENT); + }); + + it("does nothing for a window that has gone", () => { + const relay = createBrowserShortcutRelay(vi.fn()); + const { window, focus } = appWindow(true); + const listener = vi.fn(); + relay.subscribe(listener); + + relay.deliver(window, EVENT); + + expect(focus).not.toHaveBeenCalled(); + expect(listener).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/main/browser/app-shortcut-relay.ts b/apps/desktop/src/main/browser/app-shortcut-relay.ts new file mode 100644 index 000000000..b8e9131fe --- /dev/null +++ b/apps/desktop/src/main/browser/app-shortcut-relay.ts @@ -0,0 +1,46 @@ +import type { BrowserWindow } from "electron"; +import type { BrowserAppShortcutEvent } from "@volli/shared"; +import type { BrowserShortcutsPort } from "@volli/host-core/handlers"; + +/** + * Where the Browser Tab host's app shortcuts go (VC-301): the window that + * draws the page gets keyboard focus back, then every window following the + * desktop-only `browserShortcuts.subscribe` entry hears the shortcut. + * + * {@link deliver} is the host's `deliverAppShortcut`; {@link subscribe} is the + * half of the map's `BrowserShortcutsPort` this relay owns. Focus comes back + * to the app FIRST: the palette's field, the address bar and the pane a chord + * moves to are all renderer DOM, unreachable while the page holds focus. + */ +export interface BrowserShortcutRelay { + deliver(window: BrowserWindow, event: BrowserAppShortcutEvent): void; + subscribe: BrowserShortcutsPort["subscribe"]; +} + +export function createBrowserShortcutRelay( + onListenerError: (error: unknown) => void, +): BrowserShortcutRelay { + const listeners = new Set<(event: BrowserAppShortcutEvent) => void>(); + return { + deliver(window, event) { + if (window.isDestroyed()) return; + window.webContents.focus(); + for (const listener of listeners) { + // One window's broken stream must not keep the key from another. + try { + listener(event); + } catch (error) { + onListenerError(error); + } + } + }, + subscribe(listener) { + // A wrapper per subscription, so the same function subscribed twice is two streams. + const one = (event: BrowserAppShortcutEvent) => listener(event); + listeners.add(one); + return () => { + listeners.delete(one); + }; + }, + }; +} diff --git a/apps/desktop/src/main/browser/app-shortcuts.test.ts b/apps/desktop/src/main/browser/app-shortcuts.test.ts new file mode 100644 index 000000000..69537a5dc --- /dev/null +++ b/apps/desktop/src/main/browser/app-shortcuts.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { isCommandPaletteKeyEvent } from "@renderer/lib/command-palette-shortcut"; +import { projectIndexForKeyEvent } from "@renderer/lib/project-shortcut"; +import { splitShortcutForKeyEvent } from "@renderer/lib/split-shortcut"; + +import { browserKeyRoute, type BrowserKeyInput } from "./app-shortcuts"; + +/** A key down with ⌘ alone held, unless a test says otherwise. */ +function key(overrides: Partial): BrowserKeyInput { + return { + type: "keyDown", + key: "k", + code: "KeyK", + meta: true, + control: false, + alt: false, + shift: false, + isAutoRepeat: false, + isComposing: false, + ...overrides, + }; +} + +const letter = (char: string, overrides: Partial = {}): BrowserKeyInput => + key({ key: char, code: `Key${char.toUpperCase()}`, ...overrides }); + +describe("browserKeyRoute — what the app takes", () => { + it("replays ⌘K, ⌘1–⌘9 and ⌃⌘ + an arrow as the chord they are", () => { + const pressed = [ + letter("k"), + key({ key: "1", code: "Digit1" }), + key({ key: "9", code: "Digit9" }), + key({ key: "ArrowLeft", code: "ArrowLeft", control: true }), + key({ key: "ArrowRight", code: "ArrowRight", control: true }), + key({ key: "ArrowUp", code: "ArrowUp", control: true }), + key({ key: "ArrowDown", code: "ArrowDown", control: true }), + ]; + for (const input of pressed) { + expect(browserKeyRoute(input)).toEqual({ + kind: "app", + shortcut: { + kind: "replay", + chord: { + key: input.key, + code: input.code, + metaKey: true, + ctrlKey: input.control, + altKey: false, + shiftKey: false, + repeat: false, + }, + }, + }); + } + }); + + it("matches ⌘K whatever case the layout reports", () => { + expect(browserKeyRoute(letter("K"))).toMatchObject({ kind: "app" }); + }); + + it("carries a replayed chord's repeat, so the renderer's own rule decides", () => { + expect( + browserKeyRoute( + key({ key: "ArrowLeft", code: "ArrowLeft", control: true, isAutoRepeat: true }), + ), + ).toMatchObject({ kind: "app", shortcut: { kind: "replay", chord: { repeat: true } } }); + }); + + it("names ⌘W as this tab's close and ⌘L as the way back to its address bar", () => { + expect(browserKeyRoute(letter("w"))).toEqual({ + kind: "app", + shortcut: { kind: "close-tab" }, + }); + expect(browserKeyRoute(letter("l"))).toEqual({ + kind: "app", + shortcut: { kind: "focus-address" }, + }); + }); + + it("swallows a held ⌘W or ⌘L rather than acting again or falling through to the menu", () => { + expect(browserKeyRoute(letter("w", { isAutoRepeat: true }))).toEqual({ kind: "swallow" }); + expect(browserKeyRoute(letter("l", { isAutoRepeat: true }))).toEqual({ kind: "swallow" }); + }); +}); + +describe("browserKeyRoute — what the page keeps", () => { + it.each([ + ["a key up", letter("k", { type: "keyUp" })], + ["a key mid-composition", letter("k", { isComposing: true })], + ["a bare key", letter("k", { meta: false })], + ["Escape", key({ key: "Escape", code: "Escape", meta: false })], + ["Shift+Tab", key({ key: "Tab", code: "Tab", meta: false, shift: true })], + ["⌃K, a readline and field binding", letter("k", { meta: false, control: true })], + ["⇧⌘K", letter("k", { shift: true })], + ["⌥⌘K", letter("k", { alt: true })], + ["⌃⌘K", letter("k", { control: true })], + ["⌘B, bold in a page editor", letter("b")], + ["⌘C, copy", letter("c")], + ["⌘V, paste", letter("v")], + ["⌘A, select all", letter("a")], + ["⌘Z, undo", letter("z")], + ["⌘P, the page's print", letter("p")], + ["⌘[, outdent in a page editor", key({ key: "[", code: "BracketLeft" })], + ["⌘], indent in a page editor", key({ key: "]", code: "BracketRight" })], + ["⌘0", key({ key: "0", code: "Digit0" })], + ["⌘ + an arrow, caret movement in a field", key({ key: "ArrowLeft", code: "ArrowLeft" })], + ["⇧⌃⌘ + an arrow", key({ key: "ArrowLeft", code: "ArrowLeft", control: true, shift: true })], + ["⌥⌘ + an arrow", key({ key: "ArrowLeft", code: "ArrowLeft", alt: true, control: true })], + ["⌃⌘ + a letter", letter("w", { control: true })], + ["⇧⌘W", letter("w", { shift: true })], + ])("leaves %s to the page", (_label, input) => { + expect(browserKeyRoute(input)).toEqual({ kind: "page" }); + }); +}); + +/** The replayed chord a key routes to, or a failure naming the key that was not one. */ +function asKeyboardEvent(input: BrowserKeyInput) { + const route = browserKeyRoute(input); + if (route.kind !== "app" || route.shortcut.kind !== "replay") { + throw new Error(`expected a replayed chord for ${input.key}`); + } + return route.shortcut.chord; +} + +/** + * Main decides which chords leave the page; the renderer's own predicates decide + * what they mean. A replayed chord the renderer then ignored would be a key + * taken from the page for nothing, so every one is checked against the + * predicate that owns it. + */ +describe("browserKeyRoute agrees with the renderer's chord predicates", () => { + it("replays a ⌘K the palette's predicate recognises", () => { + expect(isCommandPaletteKeyEvent(asKeyboardEvent(letter("k")))).toBe(true); + }); + + it("replays ⌘1–⌘9 the project rail's predicate recognises", () => { + for (let digit = 1; digit <= 9; digit += 1) { + const chord = asKeyboardEvent(key({ key: String(digit), code: `Digit${digit}` })); + expect(projectIndexForKeyEvent(chord)).toBe(digit - 1); + } + }); + + it("replays ⌃⌘ + an arrow the split view's predicate reads as pane focus", () => { + const directions = { ArrowLeft: "left", ArrowRight: "right", ArrowUp: "up", ArrowDown: "down" }; + for (const [code, direction] of Object.entries(directions)) { + const chord = asKeyboardEvent(key({ key: code, code, control: true })); + expect(splitShortcutForKeyEvent(chord)).toEqual({ kind: "focus", direction }); + } + }); +}); diff --git a/apps/desktop/src/main/browser/app-shortcuts.ts b/apps/desktop/src/main/browser/app-shortcuts.ts new file mode 100644 index 000000000..116ab8cc2 --- /dev/null +++ b/apps/desktop/src/main/browser/app-shortcuts.ts @@ -0,0 +1,102 @@ +import type { BrowserAppShortcut } from "@volli/shared"; + +/** + * Which keys pressed inside a Browser page belong to the app (VC-301). + * + * A Browser Tab's page is its own native WebContents, so a key pressed in it + * never reaches the renderer's window `keydown` listeners — ⌘K, the project + * chords and pane focus all went dead the moment a page had focus, and ⌘W fell + * through to the menu's Close Window. Main sees every key first + * (`before-input-event`), and this is the whole of its decision. + * + * DELIBERATELY SMALL. Everything not named here is the page's, including every + * editing chord: main cannot see whether the page's focus is a text field, so a + * chord that means something inside one (⌘[ / ⌘] outdent, ⌘B bold, ⌘P print, + * Escape closing a page's own dialog, Tab moving through its form) is never + * taken. What is taken is chosen because no page owns it in a browser either: + * + * • ⌘K — the command palette. The ticket's one exception to "a page field + * keeps its chords": the palette is the way out of anywhere. + * • ⌘1–⌘9 — select the Nth project, as everywhere else in the app. + * • ⌃⌘ + an arrow — move between split panes. + * • ⌘W — close THIS Browser Tab. The browser's own meaning, and the only safe + * one: left alone it falls through to the menu and closes the window. + * • ⌘L — return focus to the app, on this tab's address bar. The browser's + * own "leave the page" chord, where Escape and Tab are the page's. + * + * The first three are replayed into the renderer as the chord they are, so the + * listener that already owns each one — with its own guards, the terminal guard + * on ⌘K included — is the one that decides. Only the two that need to know + * WHICH tab the key came from are named here. + */ + +/** The fields of Electron's `Input` this decision reads. */ +export interface BrowserKeyInput { + type: string; + key: string; + code: string; + meta: boolean; + control: boolean; + alt: boolean; + shift: boolean; + isAutoRepeat: boolean; + isComposing: boolean; +} + +/** + * Where one key goes. + * + * • `page` — not the app's; the page gets it as it always did. + * • `swallow` — the app's, but nothing to do: a held ⌘W's repeats must not + * reach the page or, through it, the menu's Close Window. + * • `app` — the app's; main hands the shortcut to the renderer. + */ +export type BrowserKeyRoute = + | { kind: "page" } + | { kind: "swallow" } + | { kind: "app"; shortcut: BrowserAppShortcut }; + +const PAGE: BrowserKeyRoute = { kind: "page" }; +const SWALLOW: BrowserKeyRoute = { kind: "swallow" }; + +const ARROWS = new Set(["ArrowLeft", "ArrowRight", "ArrowUp", "ArrowDown"]); + +function replay(input: BrowserKeyInput): BrowserKeyRoute { + return { + kind: "app", + shortcut: { + kind: "replay", + chord: { + key: input.key, + code: input.code, + metaKey: input.meta, + ctrlKey: input.control, + altKey: input.alt, + shiftKey: input.shift, + repeat: input.isAutoRepeat, + }, + }, + }; +} + +/** An action on the tab itself fires once per press; its repeats are swallowed. */ +function once(input: BrowserKeyInput, shortcut: BrowserAppShortcut): BrowserKeyRoute { + return input.isAutoRepeat ? SWALLOW : { kind: "app", shortcut }; +} + +export function browserKeyRoute(input: BrowserKeyInput): BrowserKeyRoute { + // Key up is the page's: it never saw the down, and a stray up does nothing. + // A key mid-composition belongs to the input method, never to a shortcut. + if (input.type !== "keyDown" || input.isComposing || !input.meta || input.alt) return PAGE; + + if (input.control) { + return !input.shift && ARROWS.has(input.code) ? replay(input) : PAGE; + } + if (input.shift) return PAGE; + + const key = input.key.toLowerCase(); + if (key === "k" || /^[1-9]$/.test(input.key)) return replay(input); + if (key === "w") return once(input, { kind: "close-tab" }); + if (key === "l") return once(input, { kind: "focus-address" }); + return PAGE; +} diff --git a/apps/desktop/src/main/browser/tab-host.test.ts b/apps/desktop/src/main/browser/tab-host.test.ts index f159b4cd4..29f8d631a 100644 --- a/apps/desktop/src/main/browser/tab-host.test.ts +++ b/apps/desktop/src/main/browser/tab-host.test.ts @@ -5,7 +5,7 @@ import type { WebContentsView, WebContentsViewConstructorOptions, } from "electron"; -import type { BrowserTabState } from "@volli/shared"; +import type { BrowserAppShortcutEvent, BrowserTabState } from "@volli/shared"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { @@ -104,6 +104,9 @@ class FakeWebContents { return false; } focused = false; + focus = vi.fn(() => { + this.focused = true; + }); isFocused(): boolean { return this.focused; } @@ -257,6 +260,8 @@ let views: FakeView[]; let viewOptions: WebContentsViewConstructorOptions[]; let sessions: Map; let published: unknown[]; +/** Every app shortcut the host handed back, with the window it named (VC-301). */ +let delivered: { window: unknown; event: BrowserAppShortcutEvent }[]; let host: BrowserTabHost; let pictures: BrowserPictureStore; let persisted: Map; @@ -269,6 +274,7 @@ beforeEach(() => { viewOptions = []; sessions = new Map(); published = []; + delivered = []; persisted = new Map(); stages = []; stageBuilds = "ok"; @@ -313,6 +319,7 @@ beforeEach(() => { createStageWindow: newStage, publishState: (event) => published.push(event), publishClosed: (tabId) => published.push({ closedTabId: tabId }), + deliverAppShortcut: (window, event) => delivered.push({ window, event }), }); }); @@ -2842,3 +2849,183 @@ describe("BrowserTabHost as the desktop's Browser backend (VC-561)", () => { expect(contents.listeners.get("did-stop-loading")).toHaveLength(1); }); }); + +function appKey(overrides: Partial = {}): Electron.Input { + return { + type: "keyDown", + key: "k", + code: "KeyK", + isAutoRepeat: false, + isComposing: false, + shift: false, + control: false, + alt: false, + meta: true, + location: 0, + modifiers: ["meta"], + ...overrides, + }; +} + +/** Presses one key into a page the way Chromium does; returns the event's preventDefault. */ +function press(contents: FakeWebContents, input: Electron.Input) { + const event = { preventDefault: vi.fn() }; + contents.emit("before-input-event", event, input); + return event.preventDefault; +} + +describe("BrowserTabHost app shortcuts (VC-301)", () => { + function shownUserTab() { + const tab = host.open({ + url: "https://example.com", + projectId: "project-1", + ticketId: null, + createdBy: "user", + }); + host.show(tab.tabId); + return tab; + } + + it("takes an app chord from the focused page and hands it to the window the page is on", () => { + const tab = shownUserTab(); + const contents = views[0]!.webContents; + contents.focused = true; + + const prevented = press(contents, appKey()); + + expect(prevented).toHaveBeenCalledOnce(); + expect(delivered).toEqual([ + { + window: fakeWindow, + event: { + tabId: tab.tabId, + shortcut: { + kind: "replay", + chord: { + key: "k", + code: "KeyK", + metaKey: true, + ctrlKey: false, + altKey: false, + shiftKey: false, + repeat: false, + }, + }, + }, + }, + ]); + }); + + it("names the page's own tab for close, whichever other tab is also on screen", () => { + shownUserTab(); + const second = shownUserTab(); + const contents = views[1]!.webContents; + contents.focused = true; + + press(contents, appKey({ key: "w", code: "KeyW" })); + + expect(delivered.map(({ event }) => event)).toEqual([ + { tabId: second.tabId, shortcut: { kind: "close-tab" } }, + ]); + }); + + it("leaves every key that is not the app's to the page", () => { + shownUserTab(); + const contents = views[0]!.webContents; + contents.focused = true; + + const prevented = press(contents, appKey({ key: "b", code: "KeyB" })); + + expect(prevented).not.toHaveBeenCalled(); + expect(delivered).toEqual([]); + }); + + it("swallows a held close's repeats without closing again", () => { + shownUserTab(); + const contents = views[0]!.webContents; + contents.focused = true; + + const prevented = press(contents, appKey({ key: "w", code: "KeyW", isAutoRepeat: true })); + + // Kept from the page, and so from the menu's Close Window, but not delivered. + expect(prevented).toHaveBeenCalledOnce(); + expect(delivered).toEqual([]); + }); + + it("leaves keys alone in a page without native focus, which is how CDP input arrives", () => { + shownUserTab(); + const contents = views[0]!.webContents; + contents.focused = false; + + const prevented = press(contents, appKey({ key: "w", code: "KeyW" })); + + expect(prevented).not.toHaveBeenCalled(); + expect(delivered).toEqual([]); + }); + + it("leaves keys alone in a tab that is not on screen", () => { + host.open({ + url: "https://agent.example.com", + projectId: "project-1", + ticketId: null, + createdBy: "session", + ownerSessionId: "session-a", + }); + const contents = views[0]!.webContents; + contents.focused = true; + + const prevented = press(contents, appKey()); + + expect(prevented).not.toHaveBeenCalled(); + expect(delivered).toEqual([]); + }); + + it("hands focus back to a shown page whose shortcut the app had nothing to do with", () => { + const tab = shownUserTab(); + + host.focusPage(tab.tabId); + + expect(views[0]!.webContents.focus).toHaveBeenCalledOnce(); + }); + + it("returns no focus to a page that is off screen or gone", () => { + const tab = shownUserTab(); + host.hide(tab.tabId); + host.focusPage(tab.tabId); + host.focusPage("opaque-missing"); + + expect(views[0]!.webContents.focus).not.toHaveBeenCalled(); + }); + + it("leaves every key to the page when the host has no renderer to route to", () => { + const quiet = new BrowserTabHost({ + pictures, + now: () => clock, + createId: () => "quiet-1", + createView: () => { + const view = new FakeView(); + views.push(view); + return view as unknown as WebContentsView; + }, + fromPartition: () => new FakeSession() as unknown as Session, + getWindow: () => appWindow as unknown as BrowserWindow, + createStageWindow: newStage, + publishState: () => undefined, + publishClosed: () => undefined, + }); + const tab = quiet.open({ + url: "https://example.com", + projectId: "project-1", + ticketId: null, + createdBy: "user", + }); + quiet.show(tab.tabId); + const contents = views.at(-1)!.webContents; + contents.focused = true; + + const prevented = press(contents, appKey()); + + expect(prevented).not.toHaveBeenCalled(); + expect(delivered).toEqual([]); + }); +}); diff --git a/apps/desktop/src/main/browser/tab-host.ts b/apps/desktop/src/main/browser/tab-host.ts index aec979476..3bed952c8 100644 --- a/apps/desktop/src/main/browser/tab-host.ts +++ b/apps/desktop/src/main/browser/tab-host.ts @@ -8,7 +8,7 @@ import type { WebContentsViewConstructorOptions, WebPreferences, } from "electron"; -import type { RuntimeBrowserConsoleMessage } from "@volli/shared"; +import type { BrowserAppShortcutEvent, RuntimeBrowserConsoleMessage } from "@volli/shared"; import { BROWSER_DEFAULT_BOUNDS, browserSessionPartition, @@ -24,6 +24,7 @@ import { import { isBrowserStartUrl } from "../../browser-start-page"; import type { BrowserTabBounds, BrowserTabCaptureFrame, BrowserTabState } from "../../ipc/contract"; +import { browserKeyRoute } from "./app-shortcuts"; import { debuggerTransport, loadWaiter } from "./webcontents-cdp"; import { hostLogger } from "@volli/host-core/log"; @@ -47,6 +48,12 @@ export interface BrowserTabHostDependencies extends BrowserTabRegistryPorts { * than a BrowserWindow (VC-278). */ createStageWindow: () => BaseWindow; + /** + * Hands an app shortcut pressed inside a shown page to the window it is + * shown in (VC-301; {@link browserKeyRoute}). Absent, every key stays the + * page's, which is what a host with no app renderer to route to should do. + */ + deliverAppShortcut?: (window: BrowserWindow, event: BrowserAppShortcutEvent) => void; } /** @@ -550,6 +557,20 @@ export class BrowserTabHost extends BrowserTabRegistry< view.webContents.on("focus", () => { if (entry.parent.kind === "window") entry.lastInteractionAt = this.now(); }); + // App chords out of the page (VC-301), taken before the page or the menu + // can have them. Only while the page holds native focus on screen: a + // person's key cannot arrive any other way, and CDP key events reach this + // same hook — an agent pressing ⌘W into a page nobody is focused on is the + // page's input, never a close. + view.webContents.on("before-input-event", (event, key) => { + const deliver = this.deps.deliverAppShortcut; + const window = this.windowOf(entry); + if (deliver === undefined || window === null || !view.webContents.isFocused()) return; + const route = browserKeyRoute(key); + if (route.kind === "page") return; + event.preventDefault(); + if (route.kind === "app") deliver(window, { tabId, shortcut: route.shortcut }); + }); view.webContents.setWindowOpenHandler(({ url }) => { // A hostile page can ask indefinitely; the same cap used by every other // open door turns excess popups into ordinary denials. A popup inherits @@ -749,6 +770,23 @@ export class BrowserTabHost extends BrowserTabRegistry< return { ...entry.state }; } + /** + * Gives keyboard focus back to one shown page, after an app shortcut taken + * from it turned out to mean nothing to the renderer (VC-301). Main moved + * focus to the app before the renderer could decide, and a key that did + * nothing must not also strand the person outside the page they were typing + * in. + * + * Quiet for a tab that is gone or off screen, like {@link hide}: it asks for + * an end state, and a page nobody can see has no focus to return to. + */ + focusPage(tabId: string): void { + const entry = this.tabs.get(tabId); + if (entry === undefined || entry.parent.kind !== "window") return; + const contents = entry.view.webContents; + if (!contents.isDestroyed()) contents.focus(); + } + /** Toggles Chromium DevTools inside this tab's measured plane. */ toggleDevTools(tabId: string): void { const entry = this.requireTab(tabId); diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index ed05127f4..e0faa3514 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -310,6 +310,7 @@ import { browserTracesRoot, BrowserTraceStore, } from "@volli/host-core/browser"; +import { createBrowserShortcutRelay } from "./browser/app-shortcut-relay"; import { BrowserTabHost } from "./browser/tab-host"; import { registerOrphanProcessIpcHandlers } from "./process/ipc"; import { registerBackgroundShellIpcHandlers } from "./shell/ipc"; @@ -1281,6 +1282,13 @@ const appStartup = app.whenReady().then(async () => { now: Date.now, persist: browserPictureDisk(browserPicturesRoot(app.getPath("userData"))), }); + // App shortcuts out of a focused Browser page (VC-301): the host delivers, + // the handler map's `browserShortcuts.*` entries subscribe and hand focus back. + const browserShortcutRelay = createBrowserShortcutRelay((error) => + hostLogger("browser").warn("browser shortcut listener threw", { + error: error instanceof Error ? error.message : String(error), + }), + ); const browserTabs = new BrowserTabHost({ createId: randomUUID, createView: (options) => new WebContentsView(options), @@ -1306,6 +1314,9 @@ const appStartup = app.whenReady().then(async () => { }), publishState: (tab) => publishBrowserTabEvent({ tab }), publishClosed: (closedTabId) => publishBrowserTabEvent({ closedTabId }), + // An app chord pressed in a shown page (VC-301), to the windows following + // the desktop-only `browserShortcuts.subscribe` entry. + deliverAppShortcut: (window, event) => browserShortcutRelay.deliver(window, event), pictures: browserPictures, // A Session's steps in its own tabs, kept for the person to replay after // the live set has moved on (VC-453) — its own directory, never a Blob. @@ -1560,6 +1571,11 @@ const appStartup = app.whenReady().then(async () => { error: error instanceof Error ? error.message : String(error), }), }), + // App shortcuts out of a focused Browser page (VC-301). + browserShortcuts: { + subscribe: browserShortcutRelay.subscribe, + focusPage: (tabId) => browserTabs.focusPage(tabId), + }, }); }; /** Built once, at the first door that needs it; every later door gets the same object. */ diff --git a/apps/desktop/src/renderer/src/components/app-shell.tsx b/apps/desktop/src/renderer/src/components/app-shell.tsx index fae5edeca..ca249a825 100644 --- a/apps/desktop/src/renderer/src/components/app-shell.tsx +++ b/apps/desktop/src/renderer/src/components/app-shell.tsx @@ -31,6 +31,7 @@ import { import { Sidebar, SidebarInset, SidebarProvider } from "@renderer/components/ui/sidebar"; import { Toaster } from "@renderer/components/ui/sonner"; import { UpdateInstallDialog } from "@renderer/components/update/update-install-dialog"; +import { useBrowserAppShortcuts } from "@renderer/hooks/use-browser-app-shortcuts"; import { useFullScreen } from "@renderer/hooks/use-fullscreen"; import { useNavHistory } from "@renderer/hooks/use-nav-history"; import { useNewTicketShortcut } from "@renderer/hooks/use-new-ticket-shortcut"; @@ -345,6 +346,7 @@ export function AppShell({ mainContent }: { mainContent?: React.ReactNode } = {} useNavHistory(); useProjectRootsSync(); useZoomCommands(); + useBrowserAppShortcuts(); useBootNotice(); useCliLaunchNotice(); // What this window is showing, reported to main so an alert for a Session or diff --git a/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.test.ts b/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.test.ts new file mode 100644 index 000000000..e08429b49 --- /dev/null +++ b/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.test.ts @@ -0,0 +1,275 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import type { BrowserAppKeyChord } from "@volli/shared"; +import { isCommandPaletteKeyEvent } from "@renderer/lib/command-palette-shortcut"; +import { + BROWSER_SHORTCUTS_RESUBSCRIBE_MS, + followBrowserAppShortcuts, + registerBrowserPaneCommands, + runBrowserAppShortcut, + type BrowserShortcutsRpc, +} from "./browser-app-shortcuts"; + +const cleanups: (() => void)[] = []; + +afterEach(() => { + for (const cleanup of cleanups.splice(0)) cleanup(); +}); + +function pane(tabId: string) { + const commands = { close: vi.fn(() => true), focusAddress: vi.fn(), focusPane: vi.fn() }; + cleanups.push(registerBrowserPaneCommands(tabId, commands)); + return commands; +} + +const commandK: BrowserAppKeyChord = { + key: "k", + code: "KeyK", + metaKey: true, + ctrlKey: false, + altKey: false, + shiftKey: false, + repeat: false, +}; + +describe("runBrowserAppShortcut", () => { + it("replays a chord as a cancelable window keydown, and reports a claimed one as acted on", () => { + const seen: KeyboardEvent[] = []; + const listener = (event: Event) => { + const keyboard = event as KeyboardEvent; + seen.push(keyboard); + if (isCommandPaletteKeyEvent(keyboard)) keyboard.preventDefault(); + }; + window.addEventListener("keydown", listener); + cleanups.push(() => window.removeEventListener("keydown", listener)); + + const acted = runBrowserAppShortcut({ + tabId: "tab-1", + shortcut: { kind: "replay", chord: commandK }, + }); + + expect(acted).toBe(true); + expect(seen).toHaveLength(1); + expect(seen[0]).toMatchObject({ ...commandK, bubbles: true, defaultPrevented: true }); + }); + + it("reports a replayed chord no listener claimed, so focus can go back to the page", () => { + // ⌘K under the terminal guard, ⌃⌘ + arrow with no split: the owning + // listener returns without preventDefault, which is how it says "not mine". + const listener = vi.fn(); + window.addEventListener("keydown", listener); + cleanups.push(() => window.removeEventListener("keydown", listener)); + + expect( + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "replay", chord: commandK } }), + ).toBe(false); + expect(listener).toHaveBeenCalledOnce(); + }); + + it("focuses the pane drawing the page before the chord is replayed", () => { + const order: string[] = []; + const page = pane("tab-2"); + page.focusPane.mockImplementation(() => order.push("focus-pane")); + const other = pane("tab-1"); + const listener = () => order.push("keydown"); + window.addEventListener("keydown", listener); + cleanups.push(() => window.removeEventListener("keydown", listener)); + + runBrowserAppShortcut({ tabId: "tab-2", shortcut: { kind: "replay", chord: commandK } }); + + expect(order).toEqual(["focus-pane", "keydown"]); + expect(other.focusPane).not.toHaveBeenCalled(); + }); + + it("still replays a chord from a tab no pane draws", () => { + const listener = vi.fn(); + window.addEventListener("keydown", listener); + cleanups.push(() => window.removeEventListener("keydown", listener)); + + runBrowserAppShortcut({ tabId: "tab-gone", shortcut: { kind: "replay", chord: commandK } }); + + expect(listener).toHaveBeenCalledOnce(); + }); + + it("closes the tab the key came from, never another registered pane", () => { + const first = pane("tab-1"); + const second = pane("tab-2"); + + expect(runBrowserAppShortcut({ tabId: "tab-2", shortcut: { kind: "close-tab" } })).toBe(true); + + expect(second.close).toHaveBeenCalledOnce(); + expect(first.close).not.toHaveBeenCalled(); + expect(second.focusAddress).not.toHaveBeenCalled(); + }); + + it("returns focus to the address bar of the tab the key came from", () => { + const first = pane("tab-1"); + const second = pane("tab-2"); + + expect(runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "focus-address" } })).toBe( + true, + ); + + expect(first.focusAddress).toHaveBeenCalledOnce(); + expect(second.focusAddress).not.toHaveBeenCalled(); + expect(first.close).not.toHaveBeenCalled(); + }); + + it("reports a close the pane had no close for as not acted on", () => { + const page = pane("tab-1"); + page.close.mockReturnValue(false); + + expect(runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } })).toBe(false); + }); + + it("does nothing for a tab no mounted pane draws", () => { + const other = pane("tab-1"); + + expect(runBrowserAppShortcut({ tabId: "tab-gone", shortcut: { kind: "close-tab" } })).toBe( + false, + ); + expect(runBrowserAppShortcut({ tabId: "tab-gone", shortcut: { kind: "focus-address" } })).toBe( + false, + ); + + expect(other.close).not.toHaveBeenCalled(); + expect(other.focusAddress).not.toHaveBeenCalled(); + }); +}); + +describe("registerBrowserPaneCommands", () => { + it("stops answering for a pane once it unregisters", () => { + const commands = { close: vi.fn(() => true), focusAddress: vi.fn(), focusPane: vi.fn() }; + registerBrowserPaneCommands("tab-1", commands)(); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(commands.close).not.toHaveBeenCalled(); + }); + + it("keeps a later registration for the same tab when an earlier one unregisters", () => { + // React's development double mount: the second mount outlives the first's cleanup. + const firstMount = { close: vi.fn(() => true), focusAddress: vi.fn(), focusPane: vi.fn() }; + const unregisterFirst = registerBrowserPaneCommands("tab-1", firstMount); + const secondMount = pane("tab-1"); + unregisterFirst(); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(secondMount.close).toHaveBeenCalledOnce(); + expect(firstMount.close).not.toHaveBeenCalled(); + }); +}); + +/** A timer the test runs by hand: each armed callback, its delay, and its cancel. */ +function timers() { + const pending: { run: () => void; ms: number; cancel: ReturnType }[] = []; + const setTimer = (run: () => void, ms: number) => { + const cancel = vi.fn(); + pending.push({ run, ms, cancel }); + return cancel; + }; + return { pending, setTimer }; +} + +describe("followBrowserAppShortcuts", () => { + type Handlers = Parameters[1]; + + function fakeRpc() { + const streams: { handlers: Handlers; unsubscribe: ReturnType }[] = []; + const focusPage = vi.fn(async (_input: { tabId: string }) => null); + const rpc: BrowserShortcutsRpc = { + browserShortcuts: { + subscribe: { + subscribe: (_input, handlers) => { + const unsubscribe = vi.fn(); + streams.push({ handlers, unsubscribe }); + return { unsubscribe }; + }, + }, + focusPage: { mutate: focusPage }, + }, + }; + return { rpc, streams, focusPage }; + } + + it("acts on each shortcut, and keeps focus in the app for one it acted on", () => { + const { rpc, streams, focusPage } = fakeRpc(); + const page = pane("tab-1"); + cleanups.push(followBrowserAppShortcuts(rpc, timers().setTimer)); + + streams[0]!.handlers.onData({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(page.close).toHaveBeenCalledOnce(); + expect(focusPage).not.toHaveBeenCalled(); + }); + + it("hands focus back to the page for a shortcut the app had nothing to do with", async () => { + const { rpc, streams, focusPage } = fakeRpc(); + cleanups.push(followBrowserAppShortcuts(rpc, timers().setTimer)); + + streams[0]!.handlers.onData({ tabId: "tab-gone", shortcut: { kind: "close-tab" } }); + + expect(focusPage).toHaveBeenCalledWith({ tabId: "tab-gone" }); + }); + + it("does not let a failed hand-back escape as an unhandled rejection", async () => { + const { rpc, streams, focusPage } = fakeRpc(); + focusPage.mockRejectedValueOnce(new Error("window gone")); + cleanups.push(followBrowserAppShortcuts(rpc, timers().setTimer)); + + streams[0]!.handlers.onData({ tabId: "tab-gone", shortcut: { kind: "focus-address" } }); + await Promise.resolve(); + + expect(focusPage).toHaveBeenCalledOnce(); + }); + + it("follows again after a beat when the stream ends in error, and stops for good", () => { + const { rpc, streams } = fakeRpc(); + const { pending, setTimer } = timers(); + const stop = followBrowserAppShortcuts(rpc, setTimer); + + streams[0]!.handlers.onError(new Error("fell behind")); + expect(pending).toHaveLength(1); + expect(pending[0]!.ms).toBe(BROWSER_SHORTCUTS_RESUBSCRIBE_MS); + expect(streams).toHaveLength(1); + pending[0]!.run(); + expect(streams).toHaveLength(2); + + stop(); + expect(streams[1]!.unsubscribe).toHaveBeenCalledOnce(); + expect(streams[0]!.unsubscribe).not.toHaveBeenCalled(); + }); + + it("cancels a pending re-follow when stopped, and never re-follows after the stop", () => { + const { rpc, streams } = fakeRpc(); + const { pending, setTimer } = timers(); + const stop = followBrowserAppShortcuts(rpc, setTimer); + + streams[0]!.handlers.onError(new Error("source failed")); + stop(); + expect(pending[0]!.cancel).toHaveBeenCalledOnce(); + + // An error that lands after the stop arms nothing. + streams[0]!.handlers.onError(new Error("late")); + expect(pending).toHaveLength(1); + }); + + it("re-follows on a real timer by default", () => { + vi.useFakeTimers(); + try { + const { rpc, streams } = fakeRpc(); + const stop = followBrowserAppShortcuts(rpc); + streams[0]!.handlers.onError(new Error("fell behind")); + vi.advanceTimersByTime(BROWSER_SHORTCUTS_RESUBSCRIBE_MS); + expect(streams).toHaveLength(2); + streams[1]!.handlers.onError(new Error("again")); + stop(); + vi.advanceTimersByTime(BROWSER_SHORTCUTS_RESUBSCRIBE_MS); + expect(streams).toHaveLength(2); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.ts b/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.ts new file mode 100644 index 000000000..604fbea79 --- /dev/null +++ b/apps/desktop/src/renderer/src/components/browser/browser-app-shortcuts.ts @@ -0,0 +1,139 @@ +import type { BrowserAppShortcutEvent } from "@volli/shared"; + +/** + * The renderer half of the app shortcuts a focused Browser page hands back + * (VC-301; main's `browser/app-shortcuts.ts` decides which keys those are). + * + * Two kinds of answer, and the split is the point: + * + * • A REPLAYED chord (⌘K, ⌘1–⌘9, ⌃⌘ + arrow) is dispatched on `window` as the + * keydown it was, so the listener that already owns it decides — with every + * guard it already has, the terminal guard on ⌘K included. Nothing here + * knows what those chords mean, so nothing here can disagree with them. + * The pane drawing the page is focused first: a click into a native page + * never reaches the renderer, so the split view's focused pane is whichever + * one was last clicked in the app, and ⌃⌘ + arrow has to move from the pane + * the person is actually in. + * • ⌘W and ⌘L act on the tab the key came FROM, named by main. They go to the + * pane that draws that tab, which registered its own close and address bar + * below — never to whichever pane the split view calls focused, which can be + * a different one. A tab no mounted pane draws does nothing. + */ +export interface BrowserPaneCommands { + /** + * Closes this tab through the same path its surface's close control takes. + * False when the surface gave the pane no close to run. + */ + close(): boolean; + /** Puts focus on this tab's address bar. */ + focusAddress(): void; + /** Makes the split-view pane drawing this tab the focused one. */ + focusPane(): void; +} + +const panes = new Map(); + +/** + * Registers the pane drawing one tab. Returns the unregister, which leaves a + * later registration for the same tab alone — React may mount a pane twice in + * development's double pass, and the second mount must keep working after the + * first one's cleanup. + */ +export function registerBrowserPaneCommands( + tabId: string, + commands: BrowserPaneCommands, +): () => void { + panes.set(tabId, commands); + return () => { + if (panes.get(tabId) === commands) panes.delete(tabId); + }; +} + +/** + * Acts on one shortcut main handed back, and says whether the app did + * anything with it. Main took the key and focus before the renderer could + * decide, so a `false` is the caller's cue to hand focus back to the page: a + * replayed chord no listener claimed (`preventDefault` is how every one of + * them says it acted), or a tab no pane here draws. + */ +export function runBrowserAppShortcut(event: BrowserAppShortcutEvent): boolean { + const { shortcut } = event; + const pane = panes.get(event.tabId); + if (shortcut.kind === "replay") { + pane?.focusPane(); + return !window.dispatchEvent( + new KeyboardEvent("keydown", { ...shortcut.chord, bubbles: true, cancelable: true }), + ); + } + if (pane === undefined) return false; + if (shortcut.kind === "close-tab") return pane.close(); + pane.focusAddress(); + return true; +} + +/** What the Session RPC client offers {@link followBrowserAppShortcuts}: `browserShortcuts.*`. */ +export interface BrowserShortcutsRpc { + readonly browserShortcuts: { + readonly subscribe: { + subscribe( + input: undefined, + handlers: { + onData(event: BrowserAppShortcutEvent): void; + onError(error: unknown): void; + }, + ): { unsubscribe(): void }; + }; + readonly focusPage: { mutate(input: { tabId: string }): Promise }; + }; +} + +/** How long a stream that ended in error waits before following again. */ +export const BROWSER_SHORTCUTS_RESUBSCRIBE_MS = 1_000; + +function defaultTimer(run: () => void, ms: number): () => void { + const handle = setTimeout(run, ms); + return () => clearTimeout(handle); +} + +/** + * Follows the desktop-only `browserShortcuts.subscribe` stream for the life of + * the window, acting on each shortcut and handing focus back to the page for + * one the app had nothing to do with (`browserShortcuts.focusPage`). Returns + * the stop. + * + * Nothing is toasted if the hand-back fails: nobody asked for it, and the + * person is left in the app, one click from the page (AGENTS.md's exception + * for work no one requested). A stream that ended in error (it fell behind, + * or its source failed) is followed again after a beat: a shortcut is a press, + * not state, so there is nothing to catch up on. + */ +export function followBrowserAppShortcuts( + rpc: BrowserShortcutsRpc, + setTimer: (run: () => void, ms: number) => () => void = defaultTimer, +): () => void { + let stopped = false; + let current: { unsubscribe(): void } | null = null; + let cancelTimer: (() => void) | null = null; + const open = (): void => { + current = rpc.browserShortcuts.subscribe.subscribe(undefined, { + onData: (event) => { + if (runBrowserAppShortcut(event)) return; + void rpc.browserShortcuts.focusPage.mutate({ tabId: event.tabId }).catch(() => undefined); + }, + onError: () => { + current = null; + if (stopped) return; + cancelTimer = setTimer(() => { + cancelTimer = null; + open(); + }, BROWSER_SHORTCUTS_RESUBSCRIBE_MS); + }, + }); + }; + open(); + return () => { + stopped = true; + cancelTimer?.(); + current?.unsubscribe(); + }; +} diff --git a/apps/desktop/src/renderer/src/components/browser/browser-pane.shortcuts.test.tsx b/apps/desktop/src/renderer/src/components/browser/browser-pane.shortcuts.test.tsx new file mode 100644 index 000000000..6929df92c --- /dev/null +++ b/apps/desktop/src/renderer/src/components/browser/browser-pane.shortcuts.test.tsx @@ -0,0 +1,195 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import type { BrowserTabState } from "../../../../ipc/contract"; +import type { BrowserApi } from "./browser-api"; +import { runBrowserAppShortcut } from "./browser-app-shortcuts"; +import { BrowserPane } from "./browser-pane"; + +/** + * ⌘W and ⌘L pressed inside a page come back from main naming the tab + * (VC-301). These drive the pane that draws it, through the same entry the + * app-shell subscription calls. + */ + +class TestResizeObserver { + observe(): void {} + disconnect(): void {} +} + +function tab(tabId: string): BrowserTabState { + return { + tabId, + projectId: "project-1", + ticketId: null, + createdBy: "user", + ownerSessionId: null, + presentation: "tab", + url: `https://example.com/${tabId}`, + title: tabId, + loading: false, + error: null, + canGoBack: false, + canGoForward: false, + generation: 0, + heldBy: null, + }; +} + +const api = { + capture: vi.fn(async () => ({ ok: true, frames: [] }) as const), + setBounds: vi.fn(async () => ({ ok: true }) as const), + show: vi.fn(async () => ({ ok: true }) as const), + hide: vi.fn(async () => ({ ok: true }) as const), +} as unknown as BrowserApi; + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("ResizeObserver", TestResizeObserver); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.unstubAllGlobals(); +}); + +interface DrawnPane { + tab: BrowserTabState; + visible: boolean; + onClose?: () => void; + onFocusPane?: () => void; +} + +function draw(panes: DrawnPane[]): void { + act(() => { + root.render( + <> + {panes.map((pane) => ( + undefined} + onClose={pane.onClose} + onFocusPane={pane.onFocusPane} + /> + ))} + , + ); + }); +} + +function address(tabId: string): HTMLInputElement { + const input = container + .querySelector(`[data-browser-plane="${tabId}"]`) + ?.parentElement?.querySelector("input"); + if (!(input instanceof HTMLInputElement)) throw new Error(`no address bar for ${tabId}`); + return input; +} + +describe("BrowserPane app shortcuts", () => { + it("closes the pane's own tab on ⌘W, not the other pane on screen", () => { + const closeFirst = vi.fn(); + const closeSecond = vi.fn(); + draw([ + { tab: tab("tab-1"), visible: true, onClose: closeFirst }, + { tab: tab("tab-2"), visible: true, onClose: closeSecond }, + ]); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(closeFirst).toHaveBeenCalledOnce(); + expect(closeSecond).not.toHaveBeenCalled(); + }); + + it("calls the surface's latest close, not the one it mounted with", () => { + const stale = vi.fn(); + const latest = vi.fn(); + draw([{ tab: tab("tab-1"), visible: true, onClose: stale }]); + draw([{ tab: tab("tab-1"), visible: true, onClose: latest }]); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(latest).toHaveBeenCalledOnce(); + expect(stale).not.toHaveBeenCalled(); + }); + + it("reports ⌘W as not acted on for a pane its surface gave no close", () => { + draw([{ tab: tab("tab-1"), visible: true }]); + + expect(runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } })).toBe(false); + }); + + it("focuses its own split-view pane before a chord from inside the page is replayed", () => { + const focusFirst = vi.fn(); + const focusSecond = vi.fn(); + draw([ + { tab: tab("tab-1"), visible: true, onFocusPane: focusFirst }, + { tab: tab("tab-2"), visible: true, onFocusPane: focusSecond }, + ]); + + runBrowserAppShortcut({ + tabId: "tab-2", + shortcut: { + kind: "replay", + chord: { + key: "ArrowRight", + code: "ArrowRight", + metaKey: true, + ctrlKey: true, + altKey: false, + shiftKey: false, + repeat: false, + }, + }, + }); + + expect(focusSecond).toHaveBeenCalledOnce(); + expect(focusFirst).not.toHaveBeenCalled(); + }); + + it("returns focus to the pane's own address bar on ⌘L, with the address selected", () => { + draw([ + { tab: tab("tab-1"), visible: true }, + { tab: tab("tab-2"), visible: true }, + ]); + + runBrowserAppShortcut({ tabId: "tab-2", shortcut: { kind: "focus-address" } }); + + const field = address("tab-2"); + expect(document.activeElement).toBe(field); + expect(field.selectionStart).toBe(0); + expect(field.selectionEnd).toBe(field.value.length); + }); + + it("does not answer for a hidden pane, which has no page focus to press from", () => { + const close = vi.fn(); + draw([{ tab: tab("tab-1"), visible: false, onClose: close }]); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "focus-address" } }); + + expect(close).not.toHaveBeenCalled(); + expect(document.activeElement).not.toBe(address("tab-1")); + }); + + it("stops answering once the pane unmounts", () => { + const close = vi.fn(); + draw([{ tab: tab("tab-1"), visible: true, onClose: close }]); + draw([]); + + runBrowserAppShortcut({ tabId: "tab-1", shortcut: { kind: "close-tab" } }); + + expect(close).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/renderer/src/components/browser/browser-pane.tsx b/apps/desktop/src/renderer/src/components/browser/browser-pane.tsx index 93b1c2c12..b53442361 100644 --- a/apps/desktop/src/renderer/src/components/browser/browser-pane.tsx +++ b/apps/desktop/src/renderer/src/components/browser/browser-pane.tsx @@ -9,6 +9,7 @@ import type { Result, } from "../../../../ipc/contract"; import type { BrowserApi } from "@renderer/components/browser/browser-api"; +import { registerBrowserPaneCommands } from "@renderer/components/browser/browser-app-shortcuts"; import { BrowserChrome } from "@renderer/components/browser/browser-chrome"; import { BrowserPlaneController } from "@renderer/components/browser/browser-plane"; import { @@ -174,11 +175,24 @@ export function BrowserPane({ visible, api, onTabState, + onClose, + onFocusPane, }: { tab: BrowserTabState; visible: boolean; api: BrowserApi; onTabState(tab: BrowserTabState): void; + /** + * Closes this tab the way its surface's close control does — what ⌘W pressed + * inside the page runs (VC-301). Absent, ⌘W in the page does nothing. + */ + onClose?: () => void; + /** + * Makes the split-view pane drawing this tab the focused one — run before a + * chord from inside the page is replayed, because a click into the native + * page never told the split view which pane the person is in (VC-301). + */ + onFocusPane?: () => void; }) { const anchorRef = React.useRef(null); const addressRef = React.useRef(null); @@ -202,6 +216,30 @@ export function BrowserPane({ setAddress((current) => (current === previousUrl ? next : current)); }, [tab.url]); + // Chords pressed inside THIS page come back from main named by tab + // (VC-301), so the pane drawing the tab is the one that answers — never the + // split view's focused pane. Only while shown: a hidden plane holds no focus + // to press them from. The surface's callbacks are read through a ref so a + // fresh callback each render does not re-register. + const surfaceRef = React.useRef({ onClose, onFocusPane }); + surfaceRef.current = { onClose, onFocusPane }; + React.useEffect(() => { + if (!visible) return; + return registerBrowserPaneCommands(tab.tabId, { + close: () => { + const close = surfaceRef.current.onClose; + if (close === undefined) return false; + close(); + return true; + }, + focusAddress: () => { + addressRef.current?.focus(); + addressRef.current?.select(); + }, + focusPane: () => surfaceRef.current.onFocusPane?.(), + }); + }, [tab.tabId, visible]); + // A tab that has been sent nowhere yet is a question, so the caret starts in // the address bar — the bargain ⌘T makes in every other browser. // diff --git a/apps/desktop/src/renderer/src/components/browser/browser-preview.tsx b/apps/desktop/src/renderer/src/components/browser/browser-preview.tsx index 1d3235076..81c162198 100644 --- a/apps/desktop/src/renderer/src/components/browser/browser-preview.tsx +++ b/apps/desktop/src/renderer/src/components/browser/browser-preview.tsx @@ -113,6 +113,8 @@ export function BrowserPreview({ visible={visible} api={api} onTabState={useBrowserTabsStore.getState().receive} + // ⌘W from inside the page: the same close as the × above (VC-301). + onClose={() => void request(api.close({ tabId: tab.tabId }), "close Browser Tab")} /> diff --git a/apps/desktop/src/renderer/src/components/home/home-surface.tsx b/apps/desktop/src/renderer/src/components/home/home-surface.tsx index c7cef0e35..44ca0c3a3 100644 --- a/apps/desktop/src/renderer/src/components/home/home-surface.tsx +++ b/apps/desktop/src/renderer/src/components/home/home-surface.tsx @@ -558,43 +558,56 @@ export function HomeSurface({ visible }: { visible: boolean }) { ], ); + /** + * Close one Browser Tab of this surface — the strip's ×, and ⌘W pressed inside + * the page itself (VC-301), which names its own tab rather than a pane. + */ + const closeBrowser = React.useCallback( + (tabId: string) => { + if (selectedId === null) return; + const id = browserTabId(tabId); + void browserApi + .close({ tabId }) + .then((result) => { + if (!result.ok) { + toastError(`Could not close Browser Tab: ${result.error}`); + return; + } + useBrowserTabsStore.getState().remove(tabId); + // The split-view half of a close this store does not own — the same + // statement the chat and terminal branches make below. While split + // it lands the surface on the pane's own successor, so the MRU + // return inside closeHomeBrowserTab only ever runs unsplit. + removeHomeTabFromSplit(selectedId, id); + const sessions = useSessionsStore.getState().byOwner[selectedId]?.tabs ?? []; + const chats = useChatSessionsStore.getState().openTabs[selectedId] ?? []; + const workspace = useWorkspaceStore.getState().byProject[selectedId]; + const files = workspace?.projectFiles.tabs ?? []; + const browsers = stripBrowserTabs( + Object.values(useBrowserTabsStore.getState().byId).filter( + (tab) => tab.projectId === selectedId && tab.ticketId === null, + ), + ); + closeHomeBrowserTab(selectedId, id, [ + HOME_BOARD_TAB_ID, + ...sessions.map((tab) => tab.sessionId), + ...chats.map(chatTabId), + ...files.map((tab) => fileTabId(tab.relPath)), + ...browsers.map((tab) => browserTabId(tab.tabId)), + ]); + }) + .catch((reason: unknown) => { + toastError(`Could not close Browser Tab: ${errorMessage(reason)}`); + }); + }, + [browserApi, closeHomeBrowserTab, removeHomeTabFromSplit, selectedId], + ); + const handleClose = React.useCallback( (descriptor: HomeTabDescriptor) => { if (selectedId === null || descriptor.kind === "board") return; if (descriptor.kind === "browser") { - void browserApi - .close({ tabId: descriptor.tabId }) - .then((result) => { - if (!result.ok) { - toastError(`Could not close Browser Tab: ${result.error}`); - return; - } - useBrowserTabsStore.getState().remove(descriptor.tabId); - // The split-view half of a close this store does not own — the same - // statement the chat and terminal branches make below. While split - // it lands the surface on the pane's own successor, so the MRU - // return inside closeHomeBrowserTab only ever runs unsplit. - removeHomeTabFromSplit(selectedId, descriptor.id); - const sessions = useSessionsStore.getState().byOwner[selectedId]?.tabs ?? []; - const chats = useChatSessionsStore.getState().openTabs[selectedId] ?? []; - const workspace = useWorkspaceStore.getState().byProject[selectedId]; - const files = workspace?.projectFiles.tabs ?? []; - const browsers = stripBrowserTabs( - Object.values(useBrowserTabsStore.getState().byId).filter( - (tab) => tab.projectId === selectedId && tab.ticketId === null, - ), - ); - closeHomeBrowserTab(selectedId, descriptor.id, [ - HOME_BOARD_TAB_ID, - ...sessions.map((tab) => tab.sessionId), - ...chats.map(chatTabId), - ...files.map((tab) => fileTabId(tab.relPath)), - ...browsers.map((tab) => browserTabId(tab.tabId)), - ]); - }) - .catch((reason: unknown) => { - toastError(`Could not close Browser Tab: ${errorMessage(reason)}`); - }); + closeBrowser(descriptor.tabId); return; } if (descriptor.kind === "chat") { @@ -626,14 +639,7 @@ export function HomeSurface({ visible }: { visible: boolean }) { // The controller's own callback, not the controller: its object identity // moves whenever a tab, a dirty flag or a pending close does, and this memo // also holds the terminal close guard. - [ - browserApi, - closeGuard, - closeHomeBrowserTab, - removeHomeTabFromSplit, - requestCloseFile, - selectedId, - ], + [closeBrowser, closeGuard, removeHomeTabFromSplit, requestCloseFile, selectedId], ); const handleRename = React.useCallback((descriptor: HomeTabDescriptor, title: string) => { @@ -906,6 +912,8 @@ export function HomeSurface({ visible }: { visible: boolean }) { visible={visible} api={browserApi} onTabState={useBrowserTabsStore.getState().receive} + onClose={() => closeBrowser(paneBrowserTab.tabId)} + onFocusPane={() => focusHomePane(selectedId, pane.id)} /> ) : null} {relPath === null && diff --git a/apps/desktop/src/renderer/src/components/ticket/ticket-detail.tsx b/apps/desktop/src/renderer/src/components/ticket/ticket-detail.tsx index a5c05de33..117d4e0bb 100644 --- a/apps/desktop/src/renderer/src/components/ticket/ticket-detail.tsx +++ b/apps/desktop/src/renderer/src/components/ticket/ticket-detail.tsx @@ -1525,6 +1525,10 @@ export function TicketDetail({ visible api={browserApi} onTabState={useBrowserTabsStore.getState().receive} + // ⌘W from inside the page closes THIS pane's tab, and a chord + // from inside it starts from THIS pane (VC-301). + onClose={() => closeTab(tab)} + onFocusPane={() => focusTicketPane(projectId, ticket.id, pane.id)} /> ) : null} {tab.kind === "session" ? ( diff --git a/apps/desktop/src/renderer/src/hooks/use-browser-app-shortcuts.ts b/apps/desktop/src/renderer/src/hooks/use-browser-app-shortcuts.ts new file mode 100644 index 000000000..5079bbfaf --- /dev/null +++ b/apps/desktop/src/renderer/src/hooks/use-browser-app-shortcuts.ts @@ -0,0 +1,15 @@ +import * as React from "react"; + +import { followBrowserAppShortcuts } from "@renderer/components/browser/browser-app-shortcuts"; +import { sessionRpcClient } from "@renderer/lib/session-rpc-ipc-link"; + +/** + * Follows the app shortcuts a focused Browser page hands back (VC-301) — ⌘K + * and the navigation chords from inside a page, ⌘W and ⌘L on the page's own + * tab — over the desktop-only `browserShortcuts.*` entries. Mounted once in + * the app shell, beside the window's other chord hooks, because a page can be + * focused on any surface that draws one. + */ +export function useBrowserAppShortcuts(): void { + React.useEffect(() => followBrowserAppShortcuts(sessionRpcClient()), []); +} diff --git a/apps/desktop/src/renderer/src/hooks/use-split-shortcuts.browser.test.tsx b/apps/desktop/src/renderer/src/hooks/use-split-shortcuts.browser.test.tsx new file mode 100644 index 000000000..1ea995152 --- /dev/null +++ b/apps/desktop/src/renderer/src/hooks/use-split-shortcuts.browser.test.tsx @@ -0,0 +1,137 @@ +// @vitest-environment jsdom +/** + * ⌃⌘ + an arrow pressed inside a Browser page (VC-301), through the real split + * chord listener and the real workspace store. + * + * The subject is the seam: a click into the native page never reaches the + * renderer, so the split view's focused pane is whichever one was last clicked + * in the app. The chord replayed from the page has to move from the pane that + * draws the page instead. + */ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { SPLIT_VIEW_ROOT_PANE_ID, splitViewPanes } from "@volli/shared"; + +import { + registerBrowserPaneCommands, + runBrowserAppShortcut, +} from "@renderer/components/browser/browser-app-shortcuts"; +import { browserTabId } from "@renderer/components/home/home-tabs"; +import { useProjectsStore } from "@renderer/stores/projects"; +import { DEFAULT_WORKSPACE_UI, useWorkspaceStore } from "@renderer/stores/workspace"; + +import { useSplitShortcuts } from "./use-split-shortcuts"; + +const PROJECT = "project-1"; +const LEFT = browserTabId("tab-left"); +const MIDDLE = browserTabId("tab-middle"); +const RIGHT = browserTabId("tab-right"); +const STRIP = [LEFT, MIDDLE, RIGHT]; + +let container: HTMLDivElement; +let root: Root; +let unregister: (() => void) | null = null; + +function Probe(): null { + useSplitShortcuts({ projectId: PROJECT, ticketId: null, orderedTabIds: STRIP }); + return null; +} + +function split() { + const state = useWorkspaceStore.getState().byProject[PROJECT]?.homeSplitView; + if (state === null || state === undefined) throw new Error("expected a Home split"); + return state; +} + +/** The pane holding one tab, by id. */ +function paneOf(tabId: string): string { + const pane = splitViewPanes(split()).find((candidate) => candidate.tabIds.includes(tabId)); + if (pane === undefined) throw new Error(`no pane holds ${tabId}`); + return pane.id; +} + +/** ⌃⌘→ as main hands it back from the middle pane's page. */ +function pressRightInMiddlePage(): boolean { + return runBrowserAppShortcut({ + tabId: "tab-middle", + shortcut: { + kind: "replay", + chord: { + key: "ArrowRight", + code: "ArrowRight", + metaKey: true, + ctrlKey: true, + altKey: false, + shiftKey: false, + repeat: false, + }, + }, + }); +} + +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + useProjectsStore.setState({ selectedProjectId: PROJECT }); + useWorkspaceStore.setState({ + byProject: { [PROJECT]: { ...DEFAULT_WORKSPACE_UI, nav: "home", homeActiveTab: LEFT } }, + } as never); + // Three panes in a row, one Browser Tab each: left | middle | right. + const workspace = useWorkspaceStore.getState(); + workspace.splitHomePane(PROJECT, SPLIT_VIEW_ROOT_PANE_ID, "right", { + tabId: MIDDLE, + surfaceTabIds: STRIP, + }); + workspace.splitHomePane(PROJECT, paneOf(MIDDLE), "right", { + tabId: RIGHT, + surfaceTabIds: STRIP, + }); + // The last pane clicked IN THE APP was the left one; the person then clicked + // into the middle pane's native page, which the split view never heard. + workspace.focusHomePane(PROJECT, SPLIT_VIEW_ROOT_PANE_ID); + + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + act(() => root.render()); +}); + +afterEach(() => { + unregister?.(); + unregister = null; + act(() => root.unmount()); + container.remove(); + useWorkspaceStore.setState({ byProject: {} } as never); + useProjectsStore.setState({ selectedProjectId: null }); + vi.unstubAllGlobals(); +}); + +describe("⌃⌘ + an arrow from inside a Browser page", () => { + it("moves from the pane drawing the page, not the pane last clicked in the app", () => { + // The surface's wiring: BrowserPane's onFocusPane is focusHomePane(pane). + unregister = registerBrowserPaneCommands("tab-middle", { + close: () => true, + focusAddress: () => undefined, + focusPane: () => useWorkspaceStore.getState().focusHomePane(PROJECT, paneOf(MIDDLE)), + }); + + expect(pressRightInMiddlePage()).toBe(true); + + expect(split().focusedPaneId).toBe(paneOf(RIGHT)); + }); + + it("would land one pane short without it — the failure the pane focus exists for", () => { + // No pane registered for the page: the chord starts from the stale pane. + expect(pressRightInMiddlePage()).toBe(true); + + expect(split().focusedPaneId).toBe(paneOf(MIDDLE)); + }); + + it("reports the chord as unclaimed when the surface is not split, so focus goes back to the page", () => { + useWorkspaceStore.setState({ + byProject: { [PROJECT]: { ...DEFAULT_WORKSPACE_UI, nav: "home", homeActiveTab: MIDDLE } }, + } as never); + + expect(pressRightInMiddlePage()).toBe(false); + }); +}); diff --git a/apps/desktop/vite.config.ts b/apps/desktop/vite.config.ts index f8500c963..8f7581e34 100644 --- a/apps/desktop/vite.config.ts +++ b/apps/desktop/vite.config.ts @@ -259,6 +259,9 @@ export default defineConfig(({ mode }) => ({ // which a permanent Change Set list satisfied forever. Each transition // is gated because none of them is visible in a screenshot. "src/components/browser/browser-plane-freeze.ts", + // Which pane a ⌘W or ⌘L from inside a page acts on (VC-301): a missed + // branch here is a close landing on the wrong tab. + "src/components/browser/browser-app-shortcuts.ts", // Where the Browser replay moves (VC-453): which step a key, a drag or // a growing trace lands on. None of it shows in a screenshot. "src/components/browser/browser-trace-model.ts", @@ -693,6 +696,10 @@ export default defineConfig(({ mode }) => ({ "**/src/main/ipc-descriptors.ts", "**/src/main/ipc-registry.ts", "**/src/main/navigation.ts", + // Which keys leave a Browser page for the app (VC-301). Every branch + // is a chord taken from, or left to, a page the person is typing in. + "**/src/main/browser/app-shortcuts.ts", + "**/src/main/browser/app-shortcut-relay.ts", // The notification delivery boundary (VC-295). Enrolled for the same // reason the IPC handlers are: a missed branch here is an alert that // escapes a preference, a click that opens nothing, or a suppression diff --git a/apps/docs/src/content/docs/reference/keyboard-shortcuts.mdx b/apps/docs/src/content/docs/reference/keyboard-shortcuts.mdx index 932b9d02f..5d9403a11 100644 --- a/apps/docs/src/content/docs/reference/keyboard-shortcuts.mdx +++ b/apps/docs/src/content/docs/reference/keyboard-shortcuts.mdx @@ -158,6 +158,22 @@ announcements. Font-size shortcuts change the focused terminal only. To scale the whole app, move focus out of the terminal first. +## Browser Tab + +These work while a Browser Tab's page has focus, including inside a text field +on the page. + +| Shortcut | Action | +| --- | --- | +| `⌘K` | Open the command palette | +| `⌘1` to `⌘9` | Switch to the first through ninth project in the rail | +| `⌃⌘` + `←` / `→` / `↑` / `↓` | Move focus between panes | +| `⌘W` | Close this Browser Tab | +| `⌘L` | Leave the page and select this tab's Address field | + +Every other key goes to the page, including `Esc`, `Tab`, `⌘[` and `⌘]`, so +the page's own dialogs, forms and editors keep working. + ## Canvas editor | Shortcut | Action | When | diff --git a/docs/protocol/protocol.schema.json b/docs/protocol/protocol.schema.json index 5d0ce9396..10792f962 100644 --- a/docs/protocol/protocol.schema.json +++ b/docs/protocol/protocol.schema.json @@ -142,6 +142,138 @@ "protocolVersion": 1, "tiers": { "desktop": { + "browserShortcuts.focusPage": { + "compatibility": "client-local", + "input": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "properties": { + "tabId": { + "maxLength": 256, + "minLength": 1, + "type": "string" + } + }, + "required": [ + "tabId" + ], + "type": "object" + }, + "kind": "mutation", + "noInput": false, + "output": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "null" + }, + "outputValidation": "network-and-tests", + "voidOutput": false + }, + "browserShortcuts.subscribe": { + "compatibility": "client-local", + "input": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "null" + }, + "kind": "subscription", + "noInput": true, + "output": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "properties": { + "shortcut": { + "oneOf": [ + { + "additionalProperties": false, + "properties": { + "chord": { + "additionalProperties": false, + "properties": { + "altKey": { + "type": "boolean" + }, + "code": { + "type": "string" + }, + "ctrlKey": { + "type": "boolean" + }, + "key": { + "type": "string" + }, + "metaKey": { + "type": "boolean" + }, + "repeat": { + "type": "boolean" + }, + "shiftKey": { + "type": "boolean" + } + }, + "required": [ + "key", + "code", + "metaKey", + "ctrlKey", + "altKey", + "shiftKey", + "repeat" + ], + "type": "object" + }, + "kind": { + "const": "replay", + "type": "string" + } + }, + "required": [ + "kind", + "chord" + ], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "kind": { + "const": "close-tab", + "type": "string" + } + }, + "required": [ + "kind" + ], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "kind": { + "const": "focus-address", + "type": "string" + } + }, + "required": [ + "kind" + ], + "type": "object" + } + ] + }, + "tabId": { + "minLength": 1, + "type": "string" + } + }, + "required": [ + "tabId", + "shortcut" + ], + "type": "object" + }, + "outputValidation": "documented-yield", + "voidOutput": false + }, "hostAdd.active": { "compatibility": "client-local", "input": { diff --git a/packages/host-core/src/entries/handlers.ts b/packages/host-core/src/entries/handlers.ts index 0bc468765..2f1d8657f 100644 --- a/packages/host-core/src/entries/handlers.ts +++ b/packages/host-core/src/entries/handlers.ts @@ -32,6 +32,7 @@ export type { } from "../handlers/remote-hosts-port"; export type { RemoteSignInsPort } from "../handlers/remote-sign-ins-port"; export type { HostLinkRelayPort, HostScopeRelayPort } from "../handlers/host-link-relay-port"; +export type { BrowserShortcutsPort } from "../handlers/browser-shortcuts-port"; export { ADMITTED, admittedHandlers, diff --git a/packages/host-core/src/handlers/browser-shortcuts-port.ts b/packages/host-core/src/handlers/browser-shortcuts-port.ts new file mode 100644 index 000000000..950f758c2 --- /dev/null +++ b/packages/host-core/src/handlers/browser-shortcuts-port.ts @@ -0,0 +1,20 @@ +/** + * App shortcuts out of a focused Browser page (VC-301): what the handler map's + * desktop-only `browserShortcuts.subscribe` and `browserShortcuts.focusPage` + * call. + * + * Desktop main implements it over its Browser Tab host: a page's + * `before-input-event` takes an app chord from the page, and every subscribed + * window hears it; a shortcut the window then had nothing to do with hands + * keyboard focus back to the page. Every other host supplies none, and both + * entries answer `OperationUnavailableError`. Structural, so host-core imports + * nothing of the desktop's. + */ +import type { BrowserAppShortcutEvent } from "@volli/shared"; + +export interface BrowserShortcutsPort { + /** Calls `listener` with every shortcut a page hands back; the answer stops it. */ + subscribe(listener: (event: BrowserAppShortcutEvent) => void): () => void; + /** Focuses one shown page; quiet for a tab that is gone or off screen. */ + focusPage(tabId: string): void; +} diff --git a/packages/host-core/src/handlers/host-handlers.test.ts b/packages/host-core/src/handlers/host-handlers.test.ts index 6138aca24..d2c0aabc8 100644 --- a/packages/host-core/src/handlers/host-handlers.test.ts +++ b/packages/host-core/src/handlers/host-handlers.test.ts @@ -13,6 +13,7 @@ import { type ActiveAddHost, type AddHostEvent, type AddHostFacts, + type BrowserAppShortcutEvent, type DataChangedEvent, type HandlerCall, type ModelAccessSnapshot, @@ -39,6 +40,7 @@ import { type HostHandlerOptions, type HostHandlers, } from "./host-handlers"; +import type { BrowserShortcutsPort } from "./browser-shortcuts-port"; import type { RemoteHostsPort } from "./remote-hosts-port"; vi.mock("../session-runtime/model-access-preferences", async (importOriginal) => ({ @@ -1273,3 +1275,40 @@ describe("moving a foreground command", () => { ).toThrow(failure); }); }); + +// Browser page shortcuts (VC-301): desktop main's Browser Tab host, through +// its port; absent everywhere else. +describe("Browser page shortcuts", () => { + const EVENT: BrowserAppShortcutEvent = { tabId: "opaque-1", shortcut: { kind: "close-tab" } }; + + it("answers unavailable for both keys on a host with no Browser Tabs (hostd)", async () => { + for (const map of [handlers(), handlers({ browserShortcuts: null })]) { + const calls: (() => unknown)[] = [ + () => map["browserShortcuts.subscribe"](undefined, WINDOW, sink()), + () => map["browserShortcuts.focusPage"]({ tabId: "opaque-1" }, WINDOW), + ]; + for (const call of calls) { + expect(await unavailable(call)).toBe("Browser Tab shortcuts are unavailable on this host"); + } + } + }); + + it("feeds the window every shortcut a page hands back, and answers the port's unsubscribe", async () => { + const unsubscribe = vi.fn(); + const port = { + subscribe: vi.fn((listener: (event: BrowserAppShortcutEvent) => void) => { + listener(EVENT); + return unsubscribe; + }), + focusPage: vi.fn(), + } satisfies BrowserShortcutsPort; + const map = handlers({ browserShortcuts: port }); + const shortcuts = sink(); + + expect(await map["browserShortcuts.subscribe"](undefined, WINDOW, shortcuts)).toBe(unsubscribe); + expect(shortcuts.emit).toHaveBeenCalledWith(EVENT); + + expect(await map["browserShortcuts.focusPage"]({ tabId: "opaque-1" }, WINDOW)).toBeNull(); + expect(port.focusPage).toHaveBeenCalledWith("opaque-1"); + }); +}); diff --git a/packages/host-core/src/handlers/host-handlers.ts b/packages/host-core/src/handlers/host-handlers.ts index b3a356111..2e1185efe 100644 --- a/packages/host-core/src/handlers/host-handlers.ts +++ b/packages/host-core/src/handlers/host-handlers.ts @@ -55,6 +55,7 @@ import { type AddHostStartInput, type AddHostStepId, type AgentResponse, + type BrowserAppShortcutEvent, type CodeModePolicy, type CompactionPolicy, type ExperimentId, @@ -101,6 +102,7 @@ import { } from "@volli/shared"; import type { DetachedWorkPort } from "../detached-work"; +import type { BrowserShortcutsPort } from "./browser-shortcuts-port"; import type { BrowserPictureStore } from "../browser/picture-store"; import type { HostSignIns } from "../host-sign-ins"; import { withLogContext } from "../log/context"; @@ -376,6 +378,13 @@ export interface HostHandlerSignatures extends BoardHandlerSignatures { HostLinkRelaySubscribeCall, HostLinkRelayEvent >; + /** + * App shortcuts out of a focused Browser page (VC-301): desktop main's + * Browser Tab host, {@link HostHandlerOptions.browserShortcuts}'s, or + * unavailable. + */ + readonly "browserShortcuts.subscribe": HostSubscriptionHandler; + readonly "browserShortcuts.focusPage": HostHandler<{ tabId: string }, null>; } /** What a Session read's handler is asked: its Workspace, and the socket verb's args. */ @@ -507,6 +516,11 @@ export interface HostHandlerOptions { readonly hostLinkRelay?: HostLinkRelayPort | null; /** Desktop main's HOST links; absent or null on hostd: `hostScope.*` unavailable. */ readonly hostScopeRelay?: HostScopeRelayPort | null; + /** + * App shortcuts out of desktop main's Browser pages (VC-301); absent or null + * on hostd: `browserShortcuts.*` answer unavailable. + */ + readonly browserShortcuts?: BrowserShortcutsPort | null; } /** Messages a client may show; unchanged from the router's own (VC-564). */ @@ -524,6 +538,7 @@ const LOGS_UNAVAILABLE = "This host keeps no log to read"; const SESSION_LISTING_UNAVAILABLE = "The Session listing is unavailable on this host"; const HOST_SCOPE_RELAY_UNAVAILABLE = "Remote host operations are unavailable on this host"; const HOST_LINK_RELAY_UNAVAILABLE = "Remote projects are unavailable on this host"; +const BROWSER_SHORTCUTS_UNAVAILABLE = "Browser Tab shortcuts are unavailable on this host"; /** * Runs a handler with the ids its input names joined to the operation's log @@ -603,6 +618,8 @@ function hostHandlerEntries( const hostScopeRelay = () => present(options.hostScopeRelay ?? null, HOST_SCOPE_RELAY_UNAVAILABLE); const hostLinkRelay = () => present(options.hostLinkRelay ?? null, HOST_LINK_RELAY_UNAVAILABLE); + const browserShortcuts = () => + present(options.browserShortcuts ?? null, BROWSER_SHORTCUTS_UNAVAILABLE); const worktree = (database: Database.Database) => options.worktree ?? worktreeDeps(database, ports, { dataDir: options.dataDir }); @@ -908,5 +925,9 @@ function hostHandlerEntries( (event) => sink.emit(event), lastEventId === undefined ? {} : { lastEventId }, ), + // Browser page shortcuts (VC-301): desktop main's Browser Tab host, through its port. + "browserShortcuts.subscribe": async (_input, _call, sink) => + browserShortcuts().subscribe((event) => void sink.emit(event)), + "browserShortcuts.focusPage": ({ tabId }) => done(() => browserShortcuts().focusPage(tabId)), }; } diff --git a/packages/session-rpc/src/desktop-ipc.test.ts b/packages/session-rpc/src/desktop-ipc.test.ts index 523915b03..96ed4c693 100644 --- a/packages/session-rpc/src/desktop-ipc.test.ts +++ b/packages/session-rpc/src/desktop-ipc.test.ts @@ -115,6 +115,8 @@ describe("the desktop's IPC exposure", () => { "hostScope.query", "hostScope.mutate", "hostScope.subscribe", + "browserShortcuts.subscribe", + "browserShortcuts.focusPage", ]); }); diff --git a/packages/session-rpc/src/desktop-ipc.ts b/packages/session-rpc/src/desktop-ipc.ts index 7ce413a9d..67689965a 100644 --- a/packages/session-rpc/src/desktop-ipc.ts +++ b/packages/session-rpc/src/desktop-ipc.ts @@ -155,7 +155,8 @@ export const DESKTOP_IPC_EXPOSURE = Object.freeze({ // Its compatibility classes (VC-725) govern the schema gate, not exposure: // host commands (`project.reorder`, `worktree.trimSettings`) are // additive-only across supported skew, while the client-local families - // (`hosts.*`, `hostAdd.*`, `hostSignIns.*`, `hostLink.*`, `hostScope.*`) ship with this + // (`hosts.*`, `hostAdd.*`, `hostSignIns.*`, `hostLink.*`, `hostScope.*`, + // `browserShortcuts.*`) ship with this // renderer and main in one bundle and are not cross-version promises. "shells.moveToBackground": "ipc", "project.reorder": "ipc", @@ -197,6 +198,9 @@ export const DESKTOP_IPC_EXPOSURE = Object.freeze({ "hostScope.query": "ipc", "hostScope.mutate": "ipc", "hostScope.subscribe": "ipc", + // App shortcuts out of a focused Browser page (VC-301). + "browserShortcuts.subscribe": "ipc", + "browserShortcuts.focusPage": "ipc", } satisfies IpcExposureTable); type Exposure = typeof DESKTOP_IPC_EXPOSURE; diff --git a/packages/session-rpc/src/desktop-router.test.ts b/packages/session-rpc/src/desktop-router.test.ts index f292db521..c803af6de 100644 --- a/packages/session-rpc/src/desktop-router.test.ts +++ b/packages/session-rpc/src/desktop-router.test.ts @@ -237,6 +237,23 @@ function host(caller: RouterCaller, overrides: Partial = } /** Every handler the slice names, recording what it was called with. */ +/** A Browser page shortcut as main hands it back (VC-301). */ +const SHORTCUT = { + tabId: "opaque-1", + shortcut: { + kind: "replay", + chord: { + key: "k", + code: "KeyK", + metaKey: true, + ctrlKey: false, + altKey: false, + shiftKey: false, + repeat: false, + }, + }, +} as const; + function recordingHandlers(fixture: Host): DesktopRouterHandlers { const record = (key: string, output: Output) => @@ -294,6 +311,8 @@ function recordingHandlers(fixture: Host): DesktopRouterHandlers { "hostScope.query": record("hostScope.query", { answered: "query" }), "hostScope.mutate": record("hostScope.mutate", { answered: "mutate" }), "hostScope.subscribe": stream("hostScope.subscribe", { kind: "started" }), + "browserShortcuts.subscribe": stream("browserShortcuts.subscribe", SHORTCUT), + "browserShortcuts.focusPage": record("browserShortcuts.focusPage", null), ...fixture.overrides, }; } @@ -1066,6 +1085,33 @@ describeContract( ).toEqual(refusal); }); + // Browser page shortcuts (VC-301): desktop main's Browser Tab host, the window's alone. + it("streams the window every Browser page shortcut until it stops, and hands focus back", async () => { + const fixture = host(LOCAL_DESKTOP_CALLER); + const client = await connect(fixture); + const stream = collect((handlers) => + client.browserShortcuts.subscribe.subscribe(undefined, handlers), + ); + await vi.waitFor(() => expect(stream.values).toEqual([SHORTCUT])); + const close = { tabId: "opaque-2", shortcut: { kind: "close-tab" } } as const; + await fixture.sinks.get("browserShortcuts.subscribe")!.emit(close); + await vi.waitFor(() => expect(stream.values).toEqual([SHORTCUT, close])); + stream.unsubscribe(); + await vi.waitFor(() => expect(fixture.unsubscribed).toEqual(["browserShortcuts.subscribe"])); + + expect(await client.browserShortcuts.focusPage.mutate({ tabId: "opaque-1" })).toBeNull(); + expect(fixture.calls).toEqual([ + { key: "browserShortcuts.subscribe", input: undefined, call: WINDOW_CALL }, + { key: "browserShortcuts.focusPage", input: { tabId: "opaque-1" }, call: WINDOW_CALL }, + ]); + for (const input of [{ tabId: "" }, { tabId: "t".repeat(257) }, { tabId: "a", extra: 1 }]) { + expect( + await expectHostError(client.browserShortcuts.focusPage.mutate(input as never)), + ).toMatchObject({ code: "BAD_REQUEST" }); + } + expect(fixture.calls).toHaveLength(2); + }); + it("streams the hosts snapshot, the current first, until the window stops it", async () => { const fixture = host(LOCAL_DESKTOP_CALLER); const client = await connect(fixture); @@ -1374,6 +1420,8 @@ describe("the desktop router's grammar", () => { "hostScope.query", "hostScope.mutate", "hostScope.subscribe", + "browserShortcuts.subscribe", + "browserShortcuts.focusPage", ].toSorted(), ); expect(schemas["worktree.trimSettings"]).toMatchObject({ diff --git a/packages/session-rpc/src/desktop-router.ts b/packages/session-rpc/src/desktop-router.ts index 6a34624b9..13ae45c57 100644 --- a/packages/session-rpc/src/desktop-router.ts +++ b/packages/session-rpc/src/desktop-router.ts @@ -40,6 +40,7 @@ import { type AddHostFacts, type AddHostStartInput, type AddHostStepId, + type BrowserAppShortcutEvent, type CreateRemoteProjectInput, type CreateRemoteProjectResult, type DesktopCatalogEntry, @@ -204,6 +205,9 @@ export interface DesktopRouterHandlers { HostLinkRelaySubscribeCall, HostLinkRelayEvent >; + /** App shortcuts out of a focused Browser page (VC-301): desktop main's Browser Tab host. */ + readonly "browserShortcuts.subscribe": DesktopSubscriptionHandler; + readonly "browserShortcuts.focusPage": HostHandler<{ tabId: string }, null>; } type AssertNever = Type; @@ -335,6 +339,27 @@ async function relayAnswer(run: () => unknown): Promise { } } +/** One Browser page shortcut, as main hands it to the window (VC-301). */ +const browserAppShortcutEventSchema = z.object({ + tabId: z.string().min(1), + shortcut: z.discriminatedUnion("kind", [ + z.object({ + kind: z.literal("replay"), + chord: z.object({ + key: z.string(), + code: z.string(), + metaKey: z.boolean(), + ctrlKey: z.boolean(), + altKey: z.boolean(), + shiftKey: z.boolean(), + repeat: z.boolean(), + }), + }), + z.object({ kind: z.literal("close-tab") }), + z.object({ kind: z.literal("focus-address") }), + ]), +}); + const trimSettingsSchema = z.object({ keepPatterns: z.array(z.string()), trimOnFinish: z.boolean(), @@ -694,6 +719,31 @@ export function createDesktopRouter() { ); }), }, + /** + * App shortcuts out of a focused Browser page (VC-301). No channel + * preceded them on the bridge's terms: host-placed like `hosts.*`, the + * person's own window only; desktop main's Browser Tab host answers + * through the map, and every other host answers unavailable. + */ + browserShortcuts: { + /** Every app chord a focused page hands back, from now on; nothing is replayed to a late window. */ + subscribe: hostProcedure("browserShortcuts.subscribe").subscription(async function* ({ + ctx, + signal, + }) { + yield* desktopStream( + ctx, + "browserShortcuts.subscribe", + signal, + (sink) => ctx.handlers["browserShortcuts.subscribe"](undefined, ctx.call, sink), + ); + }), + /** Keyboard focus back to the page whose shortcut the window had nothing to do with. */ + focusPage: hostProcedure("browserShortcuts.focusPage") + .input(z.strictObject({ tabId: z.string().min(1).max(256) })) + .output(z.null()) + .mutation(({ ctx, input }) => ctx.handlers["browserShortcuts.focusPage"](input, ctx.call)), + }, }); } @@ -717,5 +767,6 @@ export function desktopProcedureSchemas(router: DesktopRouter = createDesktopRou "hostSignIns.run": hostSignInRunEventSchema, "hostLink.subscribe": hostLinkRelayEventSchema, "hostScope.subscribe": hostLinkRelayEventSchema, + "browserShortcuts.subscribe": browserAppShortcutEventSchema, }); } diff --git a/packages/session-rpc/src/handler-projection.test.ts b/packages/session-rpc/src/handler-projection.test.ts index 6eae6b35a..91e9df487 100644 --- a/packages/session-rpc/src/handler-projection.test.ts +++ b/packages/session-rpc/src/handler-projection.test.ts @@ -109,6 +109,8 @@ const SAMPLE_INPUTS: { "hostScope.query": { hostId: HOST, path: "workspaces.list" }, "hostScope.mutate": { hostId: HOST, path: "workspaces.create", input: { commandId: COMMAND } }, "hostScope.subscribe": { hostId: HOST, path: "logs.follow", lastEventId: "7" }, + "browserShortcuts.subscribe": undefined, + "browserShortcuts.focusPage": { tabId: "opaque-1" }, "ticket.move": { projectId: PROJECT, ticketId: "ticket-1", toStatus: "done" }, "sessions.create": { operationId: "op", projectId: PROJECT, ticketId: null, title: null }, "sessions.attach": { operationId: "op", ...SESSION }, diff --git a/packages/session-rpc/src/session-handlers.test-support.test.ts b/packages/session-rpc/src/session-handlers.test-support.test.ts index c898344f6..511c384aa 100644 --- a/packages/session-rpc/src/session-handlers.test-support.test.ts +++ b/packages/session-rpc/src/session-handlers.test-support.test.ts @@ -83,6 +83,8 @@ function callAll(handlers: SessionRouterHandlers & DesktopRouterHandlers) { () => handlers["hostScope.query"]({ hostId: HOST, path: "workspaces.list" }, CALL), () => handlers["hostScope.mutate"]({ hostId: HOST, path: "workspaces.create" }, CALL), () => handlers["hostScope.subscribe"]({ hostId: HOST, path: "logs.follow" }, CALL, sink), + () => handlers["browserShortcuts.subscribe"](undefined, CALL, sink), + () => handlers["browserShortcuts.focusPage"]({ tabId: "opaque-1" }, CALL), () => handlers["hostLink.query"]({ workspaceId: HOST, path: "board.snapshot" }, CALL), () => handlers["hostLink.mutate"]({ workspaceId: HOST, path: "board.setPriority" }, CALL), () => handlers["hostLink.subscribe"]({ workspaceId: HOST, path: "board.changes" }, CALL, sink), @@ -197,10 +199,12 @@ describe("sessionHandlersFrom", () => { "hostScope.query": port as never, "hostScope.mutate": port as never, "hostScope.subscribe": port as never, + "browserShortcuts.subscribe": port as never, + "browserShortcuts.focusPage": port as never, }, }; for (const call of callAll(sessionHandlersFrom(ports))) await call(); - expect(port).toHaveBeenCalledTimes(41); + expect(port).toHaveBeenCalledTimes(43); expect(port).toHaveBeenCalledWith({ flowId: "f", questionId: "q1", password: "p" }, CALL); expect(runtime.command).toHaveBeenCalledWith({ commandId: "cancel", @@ -247,6 +251,6 @@ describe("sessionHandlersFrom", () => { // projects' four (VC-710), the active add flows' one (VC-720), and // the host-scoped projects' two and HOST relay's three (VC-722), // and moving a running command to the background (VC-757). - expect(Object.keys(context.handlers)).toHaveLength(84); + expect(Object.keys(context.handlers)).toHaveLength(86); }); }); diff --git a/packages/session-rpc/src/session-handlers.test-support.ts b/packages/session-rpc/src/session-handlers.test-support.ts index 3d486a9cd..c55c6d75a 100644 --- a/packages/session-rpc/src/session-handlers.test-support.ts +++ b/packages/session-rpc/src/session-handlers.test-support.ts @@ -266,6 +266,8 @@ export function sessionHandlersFrom( "hostScope.query": remote("hostScope.query"), "hostScope.mutate": remote("hostScope.mutate"), "hostScope.subscribe": remote("hostScope.subscribe"), + "browserShortcuts.subscribe": remote("browserShortcuts.subscribe"), + "browserShortcuts.focusPage": remote("browserShortcuts.focusPage"), }; return { ...handlers, diff --git a/packages/shared/src/browser-app-shortcut.ts b/packages/shared/src/browser-app-shortcut.ts new file mode 100644 index 000000000..0f88481e1 --- /dev/null +++ b/packages/shared/src/browser-app-shortcut.ts @@ -0,0 +1,37 @@ +/** + * An app shortcut pressed while a Browser page held keyboard focus (VC-301): + * the wire shape of the desktop-only `browserShortcuts.*` entries. + * + * A Browser Tab's page is its own native WebContents, so the window's + * `keydown` listeners never see its keys. Desktop main takes the few that + * belong to the app before the page or the menu can, and hands them to the + * window as one of these: + * + * • `replay` — a chord the window already binds (⌘K, ⌘1–⌘9, ⌃⌘ + arrow), + * replayed as a `keydown` so its existing listener and guards decide. + * • `close-tab` — ⌘W, which closes THIS tab through its own surface's close. + * • `focus-address` — ⌘L, which brings focus back into the app, onto this + * tab's address bar. + */ + +/** The `KeyboardEvent` fields of one replayed chord. */ +export interface BrowserAppKeyChord { + key: string; + code: string; + metaKey: boolean; + ctrlKey: boolean; + altKey: boolean; + shiftKey: boolean; + repeat: boolean; +} + +export type BrowserAppShortcut = + | { kind: "replay"; chord: BrowserAppKeyChord } + | { kind: "close-tab" } + | { kind: "focus-address" }; + +/** One {@link BrowserAppShortcut}, naming the opaque tab whose page took it. */ +export interface BrowserAppShortcutEvent { + tabId: string; + shortcut: BrowserAppShortcut; +} diff --git a/packages/shared/src/desktop-entries.test.ts b/packages/shared/src/desktop-entries.test.ts index 116ab8d1b..bd8ce46a4 100644 --- a/packages/shared/src/desktop-entries.test.ts +++ b/packages/shared/src/desktop-entries.test.ts @@ -49,9 +49,10 @@ describe("the desktop-only tier", () => { "worktree.trimSettings", ]); for (const entry of DESKTOP_ENTRIES) { - const clientLocal = /^(hosts|hostAdd|hostSignIns|hostLink|hostScope|shells)\./u.test( - entry.key, - ); + const clientLocal = + /^(hosts|hostAdd|hostSignIns|hostLink|hostScope|shells|browserShortcuts)\./u.test( + entry.key, + ); expect(entry.compatibility, entry.key).toBe(clientLocal ? "client-local" : "host-command"); } }); diff --git a/packages/shared/src/desktop-entries.ts b/packages/shared/src/desktop-entries.ts index 21234c48a..588db54c6 100644 --- a/packages/shared/src/desktop-entries.ts +++ b/packages/shared/src/desktop-entries.ts @@ -370,6 +370,25 @@ export const DESKTOP_ENTRIES = [ idempotency: "natural", summary: "Move a running foreground command into its Session's background shells.", }, + // App shortcuts out of a focused Browser page (VC-301). Host-placed like + // `hosts.*`: the Browser Tab host and its native views are desktop main's, + // across every Workspace, and only the person's own window follows them. + // Desktop main serves them through the map's `BrowserShortcutsPort` + // (`@volli/host-core/handlers`); hostd has none, so both answer unavailable. + { + key: "browserShortcuts.subscribe", + compatibility: "client-local", + placement: "host", + idempotency: "read", + summary: "Every app shortcut a focused Browser page hands back to the window.", + }, + { + key: "browserShortcuts.focusPage", + compatibility: "client-local", + placement: "host", + idempotency: "natural", + summary: "Give keyboard focus back to a shown Browser page whose shortcut did nothing.", + }, ] as const satisfies readonly DesktopEntryDeclaration[]; export type DesktopEntry = (typeof DESKTOP_ENTRIES)[number]; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 765bab308..ffff8a173 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -22,6 +22,7 @@ export * from "./operation-unavailable"; export * from "./handler-refused"; export * from "./handler-keys"; export * from "./desktop-entries"; +export type * from "./browser-app-shortcut"; export * from "./host-link-relay"; export * from "./agent-surface"; export * from "./agent-product";