From 9a57f64f9a61a4a435ed66cdab124443074e2dd2 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 21 Oct 2026 14:57:41 -0700 Subject: [PATCH 1/3] fix(app): scope tab shortcuts to the focused panel and calm the side tab strip --- .../browser-pane-restore.spec.ts | 99 ++++++++++++++++++- .../app/src/runtime/extension/closed-tabs.ts | 8 +- .../app/src/runtime/extension/commands.tsx | 24 ++++- packages/app/src/runtime/extension/panels.tsx | 30 +++++- .../app/src/runtime/extension/side-region.tsx | 26 +++-- .../session/commands/use-session-commands.tsx | 32 +++++- .../gui-extensions/src/browser/chromium.ts | 24 ++++- .../src/browser/connection.test.ts | 1 + .../gui-extensions/src/browser/connection.ts | 4 + packages/gui-extensions/src/browser/ipc.ts | 2 + packages/gui-extensions/src/browser/model.ts | 5 + packages/gui-extensions/src/browser/pane.ts | 3 + .../src/browser/panel.fixture.tsx | 36 ++++++- packages/gui-extensions/src/browser/panel.tsx | 6 +- .../gui-extensions/src/browser/renderer.tsx | 10 +- packages/ui/src/navigation/tabs/tabs.css | 12 +++ 16 files changed, 282 insertions(+), 40 deletions(-) diff --git a/packages/app/component-tests/browser-pane-restore.spec.ts b/packages/app/component-tests/browser-pane-restore.spec.ts index c3f53266f2ce..a508d6201a5b 100644 --- a/packages/app/component-tests/browser-pane-restore.spec.ts +++ b/packages/app/component-tests/browser-pane-restore.spec.ts @@ -17,6 +17,7 @@ const modules = { file: source("../../gui-extensions/src/file/index.ts"), fileRenderer: source("../../gui-extensions/src/file/renderer.tsx"), commands: source("../src/shell/commands/command.tsx"), + extensionCommands: source("../src/runtime/extension/commands.tsx"), sdk: source("../../gui-extensions/src/sdk/index.ts"), render: source("../src/runtime/extension/render.tsx"), settings: source("../src/settings/model.tsx"), @@ -42,6 +43,7 @@ story.beforeEach(async ({ mount, page }) => { platform, solid, closed, + extensionCommands, ] = await Promise.all([ import(modules.fixture), import(modules.host), @@ -56,6 +58,7 @@ story.beforeEach(async ({ mount, page }) => { import(modules.platform), import(modules.solid), import(modules.closed), + import(modules.extensionCommands), // Load the pane before mounting: Vite must not reload the page to optimize its dependencies mid-fixture. import(modules.browserPanel), ]) @@ -94,7 +97,15 @@ story.beforeEach(async ({ mount, page }) => { reopenSide: () => void switchSession: () => void side: () => - | { keys: () => readonly string[]; active: () => string | undefined; select: (key: string) => void } + | { + keys: () => readonly string[] + active: () => string | undefined + select: (key: string) => void + close: (key: string) => void + focus: (event: Event) => boolean + openNewTab: () => void + closeNewTab: () => boolean + } | undefined shortcut: (shortcut: Accessor) => void }) => { @@ -110,7 +121,32 @@ story.beforeEach(async ({ mount, page }) => { return item?.menu === "session.panel" && item.keybind ? command.keybind(item.keybind) : "" }) + + // As the session's commands: ⌘T and ⌘W inside the side region, unless the focused panel binds the key there. + const sideKey = (event: KeyboardEvent) => + !!props.side()?.focus(event) && + !extensionCommands.extensionClaims(extensions.items(sdk.Command), command.matches, event) + command.register("fixture-session", () => [ + { + id: "file.close", + title: "Close side tab", + keybind: "mod+w", + when: sideKey, + onSelect: () => { + const side = props.side() + const key = side?.active() + + if (side && !side.closeNewTab() && key) side.close(key) + }, + }, + { + id: "file.tab.new", + title: "", + keybind: "mod+t", + when: sideKey, + onSelect: () => props.side()?.openNewTab(), + }, { id: "tab.close", title: "Close session tab", @@ -346,6 +382,67 @@ story("opens a browser tab with ⌘T inside the pane and a session everywhere el await expect(sessions).toHaveText("1") }) +story("⌘T and ⌘W follow focus: side tabs inside the side panel, session tabs elsewhere", async ({ page }) => { + const root = page.getByTestId("browser-region-fixture") + const mod = (await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform))) ? "Meta" : "Control" + const outside = root.getByRole("button", { name: "Beta", exact: true }) + const panel = page.locator("#review-panel") + const editor = page.frameLocator('iframe[title="Browser page"]').getByRole("textbox", { name: "Editor page input" }) + const sessions = root.getByTestId("new-sessions") + const opens = root.getByTestId("browser-opens") + const newTab = root.getByTestId("side-new-tab") + await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible() + await outside.click() + await root.getByRole("button", { name: "First inventory", exact: true }).click() + await expect(root.getByTestId("selected")).toHaveText(/^browser:/) + + // In the main column the chords keep the session's tabs, and the side panel reads as unfocused. + await outside.focus() + await outside.press(`${mod}+t`) + await expect(sessions).toHaveText("1") + await expect(panel).not.toHaveAttribute("data-focused") + + // A click on the side panel's own surface, nothing focusable, leaves focus on the panel: ⌘T opens its new tab view + // and ⌘W closes that view, then the selected tab. + await panel.getByTestId("selected").click() + await expect(panel).toHaveAttribute("data-focused") + await page.keyboard.press(`${mod}+t`) + await expect(newTab).toHaveText("shown") + await page.keyboard.press(`${mod}+w`) + await expect(newTab).toHaveText("none") + await expect(root.getByTestId("browser-closes")).toHaveText("[]") + await page.keyboard.press(`${mod}+w`) + await expect(root.getByTestId("browser-closes")).toHaveText('["tab_33333333-3333-3333-3333-333333333333"]') + await expect(root.getByTestId("session-state")).toHaveText("open") + await expect(sessions).toHaveText("1") + + // The page's focus, which reaches the app only as the pane's report, counts as the side panel's. + await outside.focus() + await expect(panel).not.toHaveAttribute("data-focused") + await editor.click() + await expect(panel).toHaveAttribute("data-focused") + await editor.press(`${mod}+t`) + await expect(opens).toHaveText("1") + await expect(sessions).toHaveText("1") +}) + +story("keeps ⌘T in the browser pane after a click on its loading surface", async ({ page }) => { + const root = page.getByTestId("browser-region-fixture") + const mod = (await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform))) ? "Meta" : "Control" + await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible() + await root.getByRole("button", { name: "Beta", exact: true }).click() + await root.getByRole("button", { name: "First inventory", exact: true }).click() + await expect(root.getByTestId("selected")).toHaveText(/^browser:/) + + // A page still loading shows the pane's own surface where the native page goes; clicking it must not drop focus + // to the side panel around the pane, where ⌘T would open the region's new tab view instead of a browser tab. + await page.locator('#browser-panel [data-component="browser-bar"]').click({ position: { x: 2, y: 2 } }) + await page.keyboard.press(`${mod}+t`) + await expect(root.getByTestId("browser-opens")).toHaveText("1") + await expect(root.getByTestId("side-new-tab")).toHaveText("none") + await expect(root.getByTestId("new-sessions")).toHaveText("0") +}) + story("reloads the browser tab with ⌘R inside the pane and leaves it to the window elsewhere", async ({ page }) => { const root = page.getByTestId("browser-region-fixture") const mac = await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform)) diff --git a/packages/app/src/runtime/extension/closed-tabs.ts b/packages/app/src/runtime/extension/closed-tabs.ts index d9afe28e7bdb..8606da9cfd09 100644 --- a/packages/app/src/runtime/extension/closed-tabs.ts +++ b/packages/app/src/runtime/extension/closed-tabs.ts @@ -137,15 +137,15 @@ export function createModifierHeld(modifier: ClosedTabsModifier) { /** * Whether a key event belongs to the side panel: its target is in it, or, when the target is the body because closing - * the focused tab dropped focus, the user's last focus, click or key was. + * the focused tab dropped focus, the user's last focus, click or key was. `focused` reads that last answer. */ export function createSideFocus() { - let last = false - createEventListener(document, ["focusin", "pointerdown", "keydown"], (event) => (last = inSide(event, last)), { + const [last, setLast] = createSignal(false) + createEventListener(document, ["focusin", "pointerdown", "keydown"], (event) => setLast(inSide(event, last())), { capture: true, }) - return (event: Event) => inSide(event, last) + return Object.assign((event: Event) => inSide(event, last()), { focused: last }) } /** As in a browser: inside the side panel, ⌘1–8 select that side tab, ⌘9 the last one, and ⇧⌘[ and ⇧⌘] the previous and next. */ diff --git a/packages/app/src/runtime/extension/commands.tsx b/packages/app/src/runtime/extension/commands.tsx index 098b869923f6..d04967fee066 100644 --- a/packages/app/src/runtime/extension/commands.tsx +++ b/packages/app/src/runtime/extension/commands.tsx @@ -3,7 +3,7 @@ import { Command } from "@opencode/gui-extensions/sdk" import { useCommand, type CommandSource } from "@/shell/commands/command" import { useSettings } from "@/settings/model" import { migrateKeybinds } from "@/settings/keybinds/migration" -import { useExtensionHost } from "./host" +import { useExtensionHost, type Item } from "./host" /** Publishes extension commands as `${extension}.${id}` in the host command registry. */ export function ExtensionCommands() { @@ -44,6 +44,28 @@ export function ExtensionCommands() { const scopedTo = (scope: string) => (event: KeyboardEvent) => event.target instanceof Element && !!event.target.closest(scope) +/** Whether an enabled extension command scoped around the key's target is bound to that key, so it answers instead. */ +export function useExtensionClaims() { + const host = useExtensionHost() + const command = useCommand() + + return (event: KeyboardEvent) => extensionClaims(host.items(Command), command.matches, event) +} + +export function extensionClaims( + commands: readonly Item[], + matches: (id: string, event: KeyboardEvent) => boolean, + event: KeyboardEvent, +) { + return commands.some( + (item) => + !!item.value.scope && + item.value.enabled !== false && + scopedTo(item.value.scope)(event) && + matches(`${item.extension}.${item.value.id}`, event), + ) +} + /** Focus inside an extension command's scope belongs to that extension, which binds its own shortcuts there. */ export function useExtensionScoped() { const host = useExtensionHost() diff --git a/packages/app/src/runtime/extension/panels.tsx b/packages/app/src/runtime/extension/panels.tsx index 7d38fe2e7d63..b126b40e8232 100644 --- a/packages/app/src/runtime/extension/panels.tsx +++ b/packages/app/src/runtime/extension/panels.tsx @@ -2,6 +2,7 @@ import { batch, createEffect, createMemo, + createSignal, For, mergeProps, on, @@ -34,7 +35,7 @@ import { useExtensionHost } from "./host" import { Contribution } from "./render" import { useExtensionAttachment } from "./attachment" import { legacyKeys, panelKey } from "./panel-keys" -import { closedEntry, type ClosedTabs } from "./closed-tabs" +import { closedEntry, createSideFocus, type ClosedTabs } from "./closed-tabs" type Tabs = Accessor<{ tabs(): SessionTabs @@ -308,7 +309,19 @@ export function createRegion(input: { return entry ? paneOf(entry) : "side" }) + // The host's new tab view: the pane it shows in, and the session and selection it opened over, so a tab that opens + // and selects itself replaces it. + const [adding, setAdding] = createSignal<{ pane: SessionPane; session: string; over: string | undefined }>() + + const newTab = (pane: SessionPane) => { + const value = adding() + + return value?.pane === pane && value.session === input.view().key && value.over === panes[pane].active() + } + return { + /** Whether a key event belongs to the side region rather than the session around it. */ + focus: createSideFocus(), entries, /** The drawn strip of the pane that holds the selected tab, by key. */ keys: () => panes[activePane()].keys(), @@ -334,6 +347,21 @@ export function createRegion(input: { select(key: string) { input.tabs().setActive(key) }, + newTab, + /** Shows the new tab view in a pane, by default the one holding the selected tab. */ + openNewTab(pane: SessionPane = activePane()) { + setAdding({ pane, session: input.view().key, over: panes[pane].active() }) + }, + /** Closes the new tab view, in one pane or wherever it is; returns whether it was showing. */ + closeNewTab(pane?: SessionPane) { + const value = adding() + + if (!value || (pane && value.pane !== pane)) return false + const shown = newTab(value.pane) + setAdding(undefined) + + return shown + }, /** Opens or closes the side region for the user. Opening moves focus into it once it lands; closing gives focus back. */ toggle() { const side = input.side() diff --git a/packages/app/src/runtime/extension/side-region.tsx b/packages/app/src/runtime/extension/side-region.tsx index b032289ed682..431e1ed0d06d 100644 --- a/packages/app/src/runtime/extension/side-region.tsx +++ b/packages/app/src/runtime/extension/side-region.tsx @@ -113,6 +113,7 @@ export function SideRegion(props: { tabIndex={-1} aria-label={language.t("session.panel.reviewAndFiles")} aria-hidden={!tabsOpen()} + data-focused={props.region.focus.focused() || undefined} class="relative h-full shrink-0 min-w-0 flex overflow-hidden bg-v2-background-bg-base rounded-[10px] shadow-[var(--v2-elevation-raised)] outline-none" classList={{ "flex-1": tabsVisible() }} style={{ width: panelWidth() }} @@ -223,18 +224,13 @@ export function PaneTabs(props: { const modifier = closedTabsModifier() - // Without a new tab item, "+" opens a host-owned new tab view listing the launchers. It shows while the session and - // the pane's selection are the ones it opened over, so a tab that opens and selects itself replaces it. - const [adding, setAdding] = createSignal<{ session: string; over: string | undefined }>() - - const newTab = createMemo(() => { - const value = adding() - - return !!value && value.session === props.view.key && value.over === props.pane.active() - }) + // Without a new tab item, "+" opens a host-owned new tab view listing the launchers, as ⌘T does inside the region. + const newTab = createMemo(() => props.region.newTab(props.pane.id)) + const add = () => props.region.openNewTab(props.pane.id) + const dismiss = () => props.region.closeNewTab(props.pane.id) // Once replaced, the view is gone: returning to the tab or session it opened over does not show it again. - createEffect(on(newTab, (shown) => !shown && setAdding(undefined), { defer: true })) + createEffect(on(newTab, (shown) => !shown && dismiss(), { defer: true })) let selectionEvent: Event | undefined @@ -253,7 +249,7 @@ export function PaneTabs(props: { selectionEvent.target instanceof Element && selectionEvent.target.closest('[role="tab"]') ) { - setAdding(undefined) + dismiss() props.region.select(value) } }} @@ -292,7 +288,7 @@ export function PaneTabs(props: { setAdding(undefined)} + onMiddleClick={dismiss} closeButton={ { event.preventDefault() event.stopPropagation() - setAdding(undefined) + dismiss() }} icon={} aria-label={language.t("common.closeTab")} @@ -335,7 +331,7 @@ export function PaneTabs(props: { icon={} variant="ghost-muted" size="large" - onClick={() => setAdding({ session: props.view.key, over: props.pane.active() })} + onClick={add} aria-label={language.t("session.tab.add")} /> @@ -346,7 +342,7 @@ export function PaneTabs(props: { modifier={key()} entries={closed.entries} onReopen={closed.onReopen} - onNewTab={() => setAdding({ session: props.view.key, over: props.pane.active() })} + onNewTab={add} /> )} diff --git a/packages/app/src/session/commands/use-session-commands.tsx b/packages/app/src/session/commands/use-session-commands.tsx index d99c06088db8..e3dafaa638d0 100644 --- a/packages/app/src/session/commands/use-session-commands.tsx +++ b/packages/app/src/session/commands/use-session-commands.tsx @@ -14,9 +14,9 @@ import { fetchSessionExport, saveSessionExport, sessionExportFilename } from "@/ import { usePlatform } from "@/runtime/platform/platform" import type { SessionModel } from "@/session/model" import type { SessionRevert } from "@/session/revert" -import { useExtensionScoped } from "@/runtime/extension/commands" +import { useExtensionClaims, useExtensionScoped } from "@/runtime/extension/commands" import type { Region } from "@/runtime/extension/panels" -import { createSideFocus, sideTabShortcuts } from "@/runtime/extension/closed-tabs" +import { sideTabShortcuts } from "@/runtime/extension/closed-tabs" type SessionCommandSource = { identity: SessionModel["identity"] @@ -58,6 +58,7 @@ export const useSessionCommands = (actions: SessionCommandContext) => { const settings = useSettings() const platform = usePlatform() const extensionScoped = useExtensionScoped() + const extensionClaims = useExtensionClaims() const openDialog = async (load: () => Promise, show: (value: T) => void) => { const owner = actions.session.ownership.capture() @@ -77,7 +78,9 @@ export const useSessionCommands = (actions: SessionCommandContext) => { return entry && !entry.tab.pinned ? entry.key : undefined } - const inSide = createSideFocus() + const inSide = actions.region.focus + // ⌘T and ⌘W inside the side region open and close its tabs, unless the focused panel binds the key itself there. + const sideKey = (event: KeyboardEvent) => inSide(event) && !extensionClaims(event) const cycleSideTabs = () => actions.session.layout.view().side.opened() && actions.region.keys().length > 1 const sideFocused = (target: EventTarget | null) => { @@ -202,11 +205,19 @@ export const useSessionCommands = (actions: SessionCommandContext) => { } const closeTab = () => { + if (actions.region.closeNewTab()) return const tab = closableTab() if (tab) actions.region.close(tab) } + // In the pane holding focus; the region itself, or a key a native page handed over, means the selected tab's pane. + const openSideTab = () => { + const pane = document.activeElement?.closest("[data-pane-content]")?.getAttribute("data-pane-content") + + actions.region.openNewTab(pane === "side" || pane === "split" ? pane : undefined) + } + const selectAdjacentSideTab = (offset: -1 | 1) => { const key = adjacentTabKey(actions.region.keys(), actions.region.active(), offset) @@ -380,6 +391,7 @@ export const useSessionCommands = (actions: SessionCommandContext) => { const fileCmds = () => { const tab = closableTab() const cycle = cycleSideTabs() + const side = actions.session.layout.view().side.opened() return [ fileCommand({ @@ -390,14 +402,24 @@ export const useSessionCommands = (actions: SessionCommandContext) => { slash: "open", onSelect: openFile, }), - tab && + side && fileCommand({ id: "file.close", title: language.t("command.tab.close"), keybind: settings.keybinds.get("tab.close") ?? "mod+w", - when: (event) => !extensionScoped(event.target), + hidden: !tab, + when: sideKey, onSelect: closeTab, }), + side && + fileCommand({ + id: "file.tab.new", + title: "", + keybind: settings.keybinds.get("tab.new") ?? "mod+t", + hidden: true, + when: sideKey, + onSelect: openSideTab, + }), // As in a browser: in the side panel, ⌘⇧T reopens a side tab rather than a session tab. fileCommand({ id: "file.tab.reopen", diff --git a/packages/gui-extensions/src/browser/chromium.ts b/packages/gui-extensions/src/browser/chromium.ts index 5509c3ba50f7..e40f2528a050 100644 --- a/packages/gui-extensions/src/browser/chromium.ts +++ b/packages/gui-extensions/src/browser/chromium.ts @@ -140,6 +140,8 @@ export function createBrowserPage( address?: () => void /** Hands a browser or shell shortcut to the app window. */ shortcut?: (shortcut: PageShortcut) => void + /** The page took keyboard focus. */ + focused?: () => void fail: () => void /** Adopts a page the document opened; a background one leaves the current tab selected. */ popup: (options: Electron.BrowserWindowConstructorOptions, background: boolean) => WebContents @@ -306,12 +308,16 @@ export function createBrowserPage( let failure: { url: string; message: string } | undefined // A blank document is never shown, so its load does not count as loading: a new tab shows its new tab page at once. let blank = options.initialize !== false && !options.restore?.url + // Chromium names a document after its URL until its parses. A navigation on the same site keeps the shown + // title until the new document names itself or stops loading, so a page that reloads or redirects to itself does + // not flash its URL in the tab strip. + let title = "" const state = (): Browser.Tab => { const page = { id: options.id, url: (failure?.url ?? contents.getURL()).slice(0, 16_384), - title: contents.getTitle().slice(0, 2_048), + title: title.slice(0, 2_048), loading: contents.isLoading() && !blank, } @@ -360,6 +366,7 @@ export function createBrowserPage( if (next !== origin) { icon = undefined iconRequest++ + title = contents.getTitle() } origin = next @@ -378,12 +385,21 @@ export function createBrowserPage( publish() }) contents.on("dom-ready", settle) + + const retitle = () => { + title = contents.getTitle() + publish() + } + contents.on("did-stop-loading", () => { settle() - publish() + retitle() + }) + contents.on("did-navigate-in-page", retitle) + contents.on("focus", () => { + if (!closed) options.focused?.() }) - contents.on("did-navigate-in-page", publish) - contents.on("page-title-updated", publish) + contents.on("page-title-updated", retitle) contents.on("render-process-gone", () => { if (!closed) options.fail() }) diff --git a/packages/gui-extensions/src/browser/connection.test.ts b/packages/gui-extensions/src/browser/connection.test.ts index 58ee7218bac2..0d0e21d754cb 100644 --- a/packages/gui-extensions/src/browser/connection.test.ts +++ b/packages/gui-extensions/src/browser/connection.test.ts @@ -112,6 +112,7 @@ function fixture(strip: string[] = []) { page: () => undefined, address: () => undefined, shortcut: () => undefined, + focused: () => undefined, }) const emit = (index: number, event: PaneEvent) => listeners.get(calls[index].input.binding)?.(event) diff --git a/packages/gui-extensions/src/browser/connection.ts b/packages/gui-extensions/src/browser/connection.ts index b71804c10908..e7628bfa4961 100644 --- a/packages/gui-extensions/src/browser/connection.ts +++ b/packages/gui-extensions/src/browser/connection.ts @@ -62,6 +62,8 @@ export function createConnection(input: { address: (tabID: Browser.TabID) => void /** A focused page handed a browser or shell shortcut to the window. */ shortcut: (event: ShortcutEvent) => void + /** The user put keyboard focus in a page. */ + focused: () => void }) { const state: ConnectionState = { browser: null, embeds: {}, suspended: false } let disposed = false @@ -164,6 +166,8 @@ export function createConnection(input: { if (event.type === "shortcut") return input.shortcut(event) + if (event.type === "focused") return input.focused() + if (event.type === "embed") { state.embeds = { ...state.embeds, [event.tabID]: event.embed } diff --git a/packages/gui-extensions/src/browser/ipc.ts b/packages/gui-extensions/src/browser/ipc.ts index c6d385bc8958..79e10d850338 100644 --- a/packages/gui-extensions/src/browser/ipc.ts +++ b/packages/gui-extensions/src/browser/ipc.ts @@ -52,6 +52,8 @@ export const PaneEvent = Schema.Union([ }), // The page asked for the address field, with the platform's address shortcut. Schema.Struct({ type: Schema.Literal("address"), tabID: Browser.TabID }), + // The user put keyboard focus in the page, outside the window's own document. + Schema.Struct({ type: Schema.Literal("focused"), tabID: Browser.TabID }), // The page handed a browser or shell shortcut to the window, which runs it as its own key press. Schema.Struct({ type: Schema.Literal("shortcut"), diff --git a/packages/gui-extensions/src/browser/model.ts b/packages/gui-extensions/src/browser/model.ts index 4974652811e8..04cc272b0cb0 100644 --- a/packages/gui-extensions/src/browser/model.ts +++ b/packages/gui-extensions/src/browser/model.ts @@ -67,6 +67,8 @@ type PaneHandle = { focusAddress: () => void /** Runs a key press the page handed to the app, as if it was pressed in this pane. */ press: (event: ShortcutEvent) => void + /** Marks the pane as holding the user's focus, which the page took outside the window's document. */ + focused: () => void } export type Model = ReturnType<typeof createModel> @@ -221,6 +223,9 @@ export function createModel(ctx: SetupContext<typeof definition>) { shortcut: (event) => { if (sessions.current()?.key === id) panes()[0]?.press(event) }, + focused: () => { + if (sessions.current()?.key === id) panes()[0]?.focused() + }, change: (next, mirror, native) => { if (next.error === "browser.pane.unsupported") { setState("unsupported", ref.server.id, true) diff --git a/packages/gui-extensions/src/browser/pane.ts b/packages/gui-extensions/src/browser/pane.ts index 39ada847eaf3..5cc16b581d95 100644 --- a/packages/gui-extensions/src/browser/pane.ts +++ b/packages/gui-extensions/src/browser/pane.ts @@ -517,6 +517,9 @@ export function createBrowserPane(input: { if (entry.win.isFocused()) entry.win.webContents.focus() publish(entry, { type: "shortcut", ...shortcut }) }, + focused: () => { + if (entry.pages.has(id) && entry.focusedTabID === id) publish(entry, { type: "focused", tabID: id }) + }, popup: (popupOptions, background) => { const popup = create(entry, false, popupOptions) diff --git a/packages/gui-extensions/src/browser/panel.fixture.tsx b/packages/gui-extensions/src/browser/panel.fixture.tsx index 5ea25bfca346..f3a9b642b308 100644 --- a/packages/gui-extensions/src/browser/panel.fixture.tsx +++ b/packages/gui-extensions/src/browser/panel.fixture.tsx @@ -587,6 +587,20 @@ type StripTabs = { type PaneClient = IpcClient<(typeof BrowserPane)["spec"]> +/** Whether a key event belongs to the side region, and whether the user's focus last was there. */ +type SideFocus = ((event: Event) => boolean) & { focused: () => boolean } + +/** What the side region's shortcuts drive. */ +type SideShortcuts = { + keys: () => readonly string[] + active: () => string | undefined + select: (key: string) => void + close: (key: string) => void + focus: SideFocus + openNewTab: () => void + closeNewTab: () => boolean +} + /** The app's extension host and side region, passed in by `packages/app/component-tests/browser-pane-restore.spec.ts`. */ type RegionHost = { LanguageProvider: Component<{ locale: string; children: JSX.Element }> @@ -616,6 +630,10 @@ type RegionHost = { select(key: string): void close(key: string): void reopen(): void + focus: SideFocus + newTab(pane: "side"): boolean + openNewTab(): void + closeNewTab(): boolean } /** The real browser and file extensions. */ definitions: readonly Definition[] @@ -630,9 +648,7 @@ type RegionHost = { reopenSide: () => void switchSession: () => void /** The side region, for its tab shortcuts. */ - side: () => - | { keys: () => readonly string[]; active: () => string | undefined; select: (key: string) => void } - | undefined + side: () => SideShortcuts | undefined /** The shortcut the browser's + menu item shows, as the host formats it; empty when it shows none. */ shortcut: (shortcut: Accessor<string>) => void }> @@ -976,6 +992,12 @@ export function mountBrowserRegion(input: RegionHost) { }, true, ) + // As chromium.ts reports the native page taking keyboard focus. + win.addEventListener("focus", () => { + const binding = latest()?.binding + + if (binding) listeners.forEach((listener) => listener({ binding, event: { type: "focused", tabID } })) + }) } // Mounts once every extension is active, as the session screen does. @@ -1010,7 +1032,12 @@ export function mountBrowserRegion(input: RegionHost) { } return ( - <aside id="review-panel" tabIndex={-1} aria-label="Side region"> + <aside + id="review-panel" + tabIndex={-1} + aria-label="Side region" + data-focused={region.focus.focused() || undefined} + > {/* The host strip draws a trigger for each of these keys. */} <div role="tablist" aria-label="Side panel" style={{ display: "flex", gap: "8px", margin: "16px 0" }}> <For each={region.keys()}> @@ -1022,6 +1049,7 @@ export function mountBrowserRegion(input: RegionHost) { </For> </div> <p data-testid="selected">{region.active() ?? "none"}</p> + <p data-testid="side-new-tab">{region.newTab("side") ? "shown" : "none"}</p> <Show when={browser()}> {(entry) => ( <PanelContext.Provider diff --git a/packages/gui-extensions/src/browser/panel.tsx b/packages/gui-extensions/src/browser/panel.tsx index 10c6d59071e1..8beb81a79846 100644 --- a/packages/gui-extensions/src/browser/panel.tsx +++ b/packages/gui-extensions/src/browser/panel.tsx @@ -410,6 +410,8 @@ export default function SessionBrowserPane(props: { }), ) }, + // A focusin inside the pane, so the app's focus tracking sees the page's focus as the side panel's. + focused: () => root?.dispatchEvent(new FocusEvent("focusin", { bubbles: true, composed: true })), }), ) @@ -545,7 +547,9 @@ export default function SessionBrowserPane(props: { <aside ref={root} id="browser-panel" - class="relative size-full min-w-0 overflow-hidden bg-v2-background-bg-base flex flex-col" + // A click on the pane's own surface, like its loading or error view, keeps focus here, so its shortcuts apply. + tabIndex={-1} + class="relative size-full min-w-0 overflow-hidden bg-v2-background-bg-base flex flex-col outline-none" > <div data-component="browser-bar" diff --git a/packages/gui-extensions/src/browser/renderer.tsx b/packages/gui-extensions/src/browser/renderer.tsx index aeab7f208e3e..6a9dc452c31f 100644 --- a/packages/gui-extensions/src/browser/renderer.tsx +++ b/packages/gui-extensions/src/browser/renderer.tsx @@ -11,8 +11,9 @@ import tabStyles from "./tabs.css?inline" const setup: Setup<typeof definition> = (ctx) => { const sessions = ctx.sessions const [model, setModel] = createSignal<Model>() - // Settings > Shortcuts lists the command on every platform; it stays disabled until the pane can open. Unbound by - // default: the sidebar owns mod+shift+b, and mod+t opens a tab from inside the pane. + const mac = /(Mac|iPhone|iPad)/.test(navigator.platform) + // Settings > Shortcuts lists the command on every platform; it stays disabled until the pane can open. Bound like + // Review's and Open file's launchers, since the sidebar owns mod+shift+b and mod+t opens a tab inside the pane. ctx.add(Command, (): Command | undefined => { const view = sessions.current() @@ -23,6 +24,8 @@ const setup: Setup<typeof definition> = (ctx) => { id: "open", title: ctx.t("command.open"), group: ctx.t("command.category.view"), + bind: mac ? "ctrl+meta+b" : "ctrl+alt+b", + editable: true, enabled: !!value?.available(view), run: () => value?.open(view), } @@ -180,8 +183,7 @@ const setup: Setup<typeof definition> = (ctx) => { id: "open", title: ctx.t("tab.title"), icon: "globe", - // mod+t opens a browser tab only inside the pane, so advertise it only while the pane is on screen. - keybind: value.pane()?.visible() ? "browser.newTab" : undefined, + keybind: "browser.open", order: 15, run: () => value.open(view), } diff --git a/packages/ui/src/navigation/tabs/tabs.css b/packages/ui/src/navigation/tabs/tabs.css index 3ef0c0f54fdd..55d8bf7f4975 100644 --- a/packages/ui/src/navigation/tabs/tabs.css +++ b/packages/ui/src/navigation/tabs/tabs.css @@ -536,6 +536,7 @@ color: var(--v2-text-text-muted); font-variation-settings: "slnt" 0; font-variant-numeric: tabular-nums; + transition: color 150ms cubic-bezier(0.2, 0.8, 0.2, 1); &[data-selected] { color: var(--v2-text-text-base); @@ -549,6 +550,17 @@ } } +/* While the keyboard belongs to the main column, the side panel's selected tab rests at the strip's muted color; its + fill still marks the selection. */ +:root + body + #review-panel:not([data-focused]) + [data-component="tabs"] + .session-review-v2-tabs-bar + [data-slot="tabs-trigger"][data-selected] { + color: var(--v2-text-text-muted); +} + :root body :is(#review-panel, #bottom-panel) [data-component="tabs"] .session-review-v2-tabs-bar { width: 100%; min-width: 0; From 20b9146c37f44d9416d7473c346ce0ef0e294d6e Mon Sep 17 00:00:00 2001 From: Kit Langton <kit.langton@gmail.com> Date: Wed, 21 Oct 2026 15:22:45 -0700 Subject: [PATCH 2/3] test(app): cover focus-scoped tab shortcuts after the page takes focus --- .../browser-pane-restore.spec.ts | 24 +++++++++---------- .../src/browser/panel.fixture.tsx | 12 ++++++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/app/component-tests/browser-pane-restore.spec.ts b/packages/app/component-tests/browser-pane-restore.spec.ts index a508d6201a5b..44a832fc1329 100644 --- a/packages/app/component-tests/browser-pane-restore.spec.ts +++ b/packages/app/component-tests/browser-pane-restore.spec.ts @@ -341,13 +341,13 @@ story("opens a browser tab with ⌘T inside the pane and a session everywhere el const opens = root.getByTestId("browser-opens") const sessions = root.getByTestId("new-sessions") await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible() - // Alpha shows a file tab, so the + advertises no browser shortcut. - await expect(root.getByTestId("add-shortcut")).toHaveText("none") + // The + advertises the browser launcher's own shortcut, whichever tab shows; ⌘T stays the pane's. + await expect(root.getByTestId("add-shortcut")).toHaveText(mac ? "⌃⌘B" : "Ctrl+Alt+B") await outside.click() await expect(root.getByText("Registrations: 2", { exact: true })).toBeVisible() await root.getByRole("button", { name: "First inventory", exact: true }).click() await expect(root.getByRole("tab", { name: "Preview", exact: true })).toHaveAttribute("aria-selected", "true") - await expect(root.getByTestId("add-shortcut")).toHaveText(mac ? "⌘T" : "Ctrl+T") + await expect(root.getByTestId("add-shortcut")).toHaveText(mac ? "⌃⌘B" : "Ctrl+Alt+B") // Outside the pane the shell keeps ⌘T. await outside.focus() @@ -402,6 +402,13 @@ story("⌘T and ⌘W follow focus: side tabs inside the side panel, session tabs await expect(sessions).toHaveText("1") await expect(panel).not.toHaveAttribute("data-focused") + // The page's focus, which reaches the app only as the pane's report, counts as the side panel's. + await editor.click() + await expect(panel).toHaveAttribute("data-focused") + await editor.press(`${mod}+t`) + await expect(opens).toHaveText("1") + await expect(sessions).toHaveText("1") + // A click on the side panel's own surface, nothing focusable, leaves focus on the panel: ⌘T opens its new tab view // and ⌘W closes that view, then the selected tab. await panel.getByTestId("selected").click() @@ -412,18 +419,9 @@ story("⌘T and ⌘W follow focus: side tabs inside the side panel, session tabs await expect(newTab).toHaveText("none") await expect(root.getByTestId("browser-closes")).toHaveText("[]") await page.keyboard.press(`${mod}+w`) - await expect(root.getByTestId("browser-closes")).toHaveText('["tab_33333333-3333-3333-3333-333333333333"]') + await expect(root.getByTestId("browser-closes")).toHaveText(/^\["tab_/) await expect(root.getByTestId("session-state")).toHaveText("open") await expect(sessions).toHaveText("1") - - // The page's focus, which reaches the app only as the pane's report, counts as the side panel's. - await outside.focus() - await expect(panel).not.toHaveAttribute("data-focused") - await editor.click() - await expect(panel).toHaveAttribute("data-focused") - await editor.press(`${mod}+t`) - await expect(opens).toHaveText("1") - await expect(sessions).toHaveText("1") }) story("keeps ⌘T in the browser pane after a click on its loading surface", async ({ page }) => { diff --git a/packages/gui-extensions/src/browser/panel.fixture.tsx b/packages/gui-extensions/src/browser/panel.fixture.tsx index f3a9b642b308..62548c980c08 100644 --- a/packages/gui-extensions/src/browser/panel.fixture.tsx +++ b/packages/gui-extensions/src/browser/panel.fixture.tsx @@ -992,12 +992,14 @@ export function mountBrowserRegion(input: RegionHost) { }, true, ) - // As chromium.ts reports the native page taking keyboard focus. - win.addEventListener("focus", () => { - const binding = latest()?.binding + // As chromium.ts reports the native page taking keyboard focus, which arrives over IPC after the click. + win.addEventListener("focus", () => + setTimeout(() => { + const binding = latest()?.binding - if (binding) listeners.forEach((listener) => listener({ binding, event: { type: "focused", tabID } })) - }) + if (binding) listeners.forEach((listener) => listener({ binding, event: { type: "focused", tabID } })) + }), + ) } // Mounts once every extension is active, as the session screen does. From 90645d16630466a5b56374f62661a7f981b8e4d2 Mon Sep 17 00:00:00 2001 From: Kit Langton <kit.langton@gmail.com> Date: Wed, 21 Oct 2026 15:43:12 -0700 Subject: [PATCH 3/3] fix(app): forward the side panel launcher chords from the page and respect reduced motion --- .../app/component-tests/browser-pane-restore.spec.ts | 5 +++++ packages/gui-extensions/src/browser/keys.test.ts | 9 +++++++++ packages/gui-extensions/src/browser/keys.ts | 6 ++++++ packages/ui/src/navigation/tabs/tabs.css | 4 ++++ 4 files changed, 24 insertions(+) diff --git a/packages/app/component-tests/browser-pane-restore.spec.ts b/packages/app/component-tests/browser-pane-restore.spec.ts index 44a832fc1329..ff87a96ac657 100644 --- a/packages/app/component-tests/browser-pane-restore.spec.ts +++ b/packages/app/component-tests/browser-pane-restore.spec.ts @@ -380,6 +380,11 @@ story("opens a browser tab with ⌘T inside the pane and a session everywhere el await editor.press(`${mod}+t`) await expect(opens).toHaveText("3") await expect(sessions).toHaveText("1") + + // The Browser launcher's own chord reaches the app from the page too. + await editor.focus() + await editor.press(mac ? "Control+Meta+b" : "Control+Alt+b") + await expect(opens).toHaveText("4") }) story("⌘T and ⌘W follow focus: side tabs inside the side panel, session tabs elsewhere", async ({ page }) => { diff --git a/packages/gui-extensions/src/browser/keys.test.ts b/packages/gui-extensions/src/browser/keys.test.ts index bf4474466c99..cf07909e6bba 100644 --- a/packages/gui-extensions/src/browser/keys.test.ts +++ b/packages/gui-extensions/src/browser/keys.test.ts @@ -65,6 +65,15 @@ test("forwards browser and shell shortcuts while leaving page editing, find, rel ["ctrl+shift", "Tab", false, "Tab"], ["ctrl", "`", true, "`"], ["ctrl", "`", false, "`"], + // The side panel's launchers: Review, Browser and Open file. + ["ctrl+mod", "b", true, "b"], + ["ctrl+mod", "r", true, "r"], + ["ctrl+mod", "p", true, "p"], + ["ctrl+alt", "b", false, "b"], + ["ctrl+alt", "p", false, "p"], + // AltGr typing on Windows and Linux layouts arrives as Ctrl+Alt with the character it types. + ["ctrl+alt", "\u00a7", false, undefined, "KeyP"], + ["ctrl+mod", "k", true, undefined], // New Window and Close Window belong to native menu accelerators, which a claimed chord would block. ["mod+shift", "W", true, undefined], ["mod+shift", "W", false, undefined], diff --git a/packages/gui-extensions/src/browser/keys.ts b/packages/gui-extensions/src/browser/keys.ts index ad0c7c4ffd9f..ff9b39197c2d 100644 --- a/packages/gui-extensions/src/browser/keys.ts +++ b/packages/gui-extensions/src/browser/keys.ts @@ -19,6 +19,10 @@ const shellKeys = ["t", "w", "n", "1", "2", "3", "4", "5", "6", "7", "8", "9", " // (⇧⌘N, ⇧⌘W) stay unclaimed: only their menu accelerators handle them, and claiming a chord also blocks the menu. const shellShiftKeys = ["t", "[", "]", "r", "p", "l", "e", "o", "s"] +// The side panel's launchers: Review, Browser and Open file, with ⌃⌘ on macOS and Ctrl+Alt elsewhere. Only a typed +// Latin letter counts, so AltGr (Ctrl+Alt) typing on other layouts stays with the page. +const launcherKeys = ["r", "b", "p"] + /** * The letter a page key press names, as the app's keybinds read it: the layout's character, so Dvorak's C is "c" * although it sits on the physical I key. A layout that types no Latin letter falls back to the physical key. @@ -57,9 +61,11 @@ export function pageReload(input: PageKey, mac: boolean) { export function pageShortcut(input: PageKey, mac: boolean): PageShortcut | undefined { const key = pageLetter(input) const mod = mac ? input.meta && !input.control : input.control && !input.meta + const launcher = mac ? input.meta && input.control && !input.alt : input.control && input.alt && !input.meta const matched = (input.control && !input.meta && !input.alt && (key === "tab" || (key === "`" && !input.shift))) || + (launcher && !input.shift && launcherKeys.includes(input.key.toLowerCase())) || (mod && (input.alt ? !input.shift && (key === "arrowleft" || key === "arrowright") diff --git a/packages/ui/src/navigation/tabs/tabs.css b/packages/ui/src/navigation/tabs/tabs.css index 55d8bf7f4975..7d6f4f8d8f20 100644 --- a/packages/ui/src/navigation/tabs/tabs.css +++ b/packages/ui/src/navigation/tabs/tabs.css @@ -538,6 +538,10 @@ font-variant-numeric: tabular-nums; transition: color 150ms cubic-bezier(0.2, 0.8, 0.2, 1); + @media (prefers-reduced-motion: reduce) { + transition: none; + } + &[data-selected] { color: var(--v2-text-text-base); }