fix(app): keep app shortcuts working inside the browser pane (#56697)

This commit is contained in:
Kit Langton
2026-10-19 22:12:18 -07:00
committed by GitHub
Unverified
parent 4504918028
commit c8ddadfb0c
11 changed files with 304 additions and 6 deletions
@@ -81,7 +81,7 @@ story.beforeEach(async ({ mount, page }) => {
})
},
}),
Commands: (props: { close: () => void }) => {
Commands: (props: { close: () => void; review: () => void }) => {
const command = commands.useCommand()
const extensions = host.useExtensionHost()
command.register("fixture-session", () => [
@@ -91,6 +91,12 @@ story.beforeEach(async ({ mount, page }) => {
keybind: "mod+w",
onSelect: props.close,
},
{
id: "review.toggle",
title: "Toggle review",
keybind: "mod+shift+r",
onSelect: props.review,
},
])
// Storybook aliases the app's command hook to a no-op. Bridge the real extension contributions
// to the real dispatcher imported by source URL, so the shortcut runs rather than its alias.
@@ -185,6 +191,35 @@ story("toggles the element picker from the shortcut only inside the browser pane
)
})
story("keeps in-page shortcuts like ⌘K in the page while forwarding ⌘⇧R and ⌘W to the pane", async ({ page }) => {
const root = page.getByTestId("browser-region-fixture")
const mac = await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform))
const mod = mac ? "Meta" : "Control"
const editor = page
.frameLocator('iframe[title="Browser page"]')
.getByRole("textbox", { name: "Editor page input", exact: true })
await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible()
await root.getByRole("button", { name: "Beta", exact: true }).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")
// In-page shortcuts like ⌘K stay with the focused page.
await editor.focus()
await editor.press(`${mod}+k`)
await expect(editor).toHaveValue("handled:k")
// Shell shortcuts like ⌘⇧R and pane-scoped ⌘W are claimed before the page sees them.
await editor.press(`${mod}+Shift+r`)
await expect(root.getByTestId("review-toggles")).toHaveText("1")
await editor.press(`${mod}+w`)
await expect(editor).toHaveValue("handled:k")
await expect(root.getByTestId("browser-closes")).toHaveText('["tab_33333333-3333-3333-3333-333333333333"]')
await expect(root.getByTestId("session-state")).toHaveText("open")
})
story("keeps a restored browser tab selected and undrawn until the desktop's first inventory", async ({ page }) => {
const root = page.getByTestId("browser-region-fixture")
const tabs = root.getByRole("tab")
@@ -7,7 +7,7 @@ import { createCdp, abortError, waitFor } from "./cdp"
import { createBrowserFiles } from "./files"
import { createDiagnostics } from "./diagnostics"
import { loadIcon } from "./icon"
import { pageLetter } from "./keys"
import { pageLetter, pageShortcut, type PageShortcut } from "./keys"
import { createProfiling, type Recording } from "./profiling"
import type { BrowserNetwork } from "./network"
import {
@@ -137,6 +137,8 @@ export function createBrowserPage(
zoomed?: () => void
/** The user pressed the address shortcut while the page had focus. */
address?: () => void
/** Hands a browser or shell shortcut to the app window. */
shortcut?: (shortcut: PageShortcut) => void
fail: () => void
/** Adopts a page the document opened; a background one leaves the current tab selected. */
popup: (options: Electron.BrowserWindowConstructorOptions, background: boolean) => WebContents
@@ -190,6 +192,15 @@ export function createBrowserPage(
return
}
const shortcut = pageShortcut(input, process.platform === "darwin")
if (shortcut) {
event.preventDefault()
options.shortcut?.(shortcut)
return
}
if (input.alt || !(process.platform === "darwin" ? input.meta : input.control)) return
// The same chord as Chromium DevTools' element picker.
@@ -111,6 +111,7 @@ function fixture(strip: string[] = []) {
inspect: (event) => routed.inspect.push(event),
page: () => undefined,
address: () => undefined,
shortcut: () => undefined,
})
const emit = (index: number, event: PaneEvent) => listeners.get(calls[index].input.binding)?.(event)
@@ -8,6 +8,8 @@ export type InspectEvent = Extract<PaneEvent, { type: "inspect" }>
export type PageEvent = Extract<PaneEvent, { type: "page" }>
export type ShortcutEvent = Extract<PaneEvent, { type: "shortcut" }>
export type Zoom = "in" | "out" | "reset"
export type Connection = ReturnType<typeof createConnection>
@@ -58,6 +60,8 @@ export function createConnection(input: {
page: (event: PageEvent) => void
/** The user pressed the address shortcut while the page had focus. */
address: (tabID: Browser.TabID) => void
/** A focused page handed a browser or shell shortcut to the window. */
shortcut: (event: ShortcutEvent) => void
}) {
const state: ConnectionState = { browser: null, embeds: {}, suspended: false }
let disposed = false
@@ -158,6 +162,8 @@ export function createConnection(input: {
if (event.type === "address") return input.address(event.tabID)
if (event.type === "shortcut") return input.shortcut(event)
if (event.type === "embed") {
state.embeds = { ...state.embeds, [event.tabID]: event.embed }
@@ -52,6 +52,16 @@ 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 page handed a browser or shell shortcut to the window, which runs it as its own key press.
Schema.Struct({
type: Schema.Literal("shortcut"),
key: text(64),
code: detail(64),
ctrl: Schema.Boolean,
meta: Schema.Boolean,
shift: Schema.Boolean,
alt: Schema.Boolean,
}),
])
export type PaneEvent = typeof PaneEvent.Type
@@ -1,5 +1,20 @@
import { expect, test } from "bun:test"
import { pageLetter } from "./keys"
import { pageLetter, pageShortcut, type PageKey } from "./keys"
/** A key press with `mod` as the platform's modifier: Command on macOS, Control elsewhere. */
const press = (modifiers: string, key: string, mac: boolean, code = ""): PageKey => {
const held = modifiers.split("+")
const mod = held.includes("mod")
return {
key,
code,
control: held.includes("ctrl") || (mod && !mac),
meta: mod && mac,
shift: held.includes("shift"),
alt: held.includes("alt"),
}
}
test.each([
{ layout: "QWERTY", input: { key: "C", code: "KeyC" }, letter: "c" },
@@ -13,3 +28,89 @@ test.each([
])("$layout $input.key on $input.code is $letter", (row) => {
expect(pageLetter(row.input)).toBe(row.letter)
})
test("forwards browser and shell shortcuts while leaving page editing, find, reload and in-page chords to the page", () => {
const cases: [modifiers: string, key: string, mac: boolean, forwarded: string | undefined, code?: string][] = [
// Browser tab, window and shell chords forwarded from the page.
["mod", "t", true, "t"],
["mod", "t", false, "t"],
// Dvorak: T is on the physical K key and Y on the physical T key; the character decides.
["mod", "t", true, "t", "KeyK"],
["mod", "y", true, undefined, "KeyT"],
["mod", "w", true, "w"],
["mod", "w", false, "w"],
["mod", "\u0432", true, "w", "KeyW"],
["mod", "n", true, "n"],
["mod", "n", false, "n"],
["mod", "1", true, "1"],
["mod", "9", false, "9"],
["mod", ",", true, ","],
["mod", "\\", false, "\\"],
["mod+shift", "T", true, "t"],
["mod+shift", "R", true, "r"],
["mod+shift", "R", false, "r"],
["mod+shift", "P", true, "p"],
["mod+shift", "L", true, "l"],
["mod+shift", "E", true, "e"],
["mod+alt", "ArrowLeft", true, "ArrowLeft"],
["mod+alt", "ArrowRight", false, "ArrowRight"],
["ctrl", "Tab", true, "Tab"],
["ctrl+shift", "Tab", false, "Tab"],
["ctrl", "`", true, "`"],
["ctrl", "`", false, "`"],
// 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],
["mod+shift", "N", true, undefined],
["mod+shift", "N", false, undefined],
// In-page editor chords (link/command menu, bold, italic, save) stay with the page.
["mod", "k", true, undefined],
["mod", "k", false, undefined],
["mod", "b", true, undefined],
["mod", "i", true, undefined],
["mod", "s", true, undefined],
// Clipboard, select all, undo and redo stay with the page.
["mod", "c", true, undefined],
["mod", "x", true, undefined],
["mod", "v", true, undefined],
["mod+shift", "V", true, undefined],
["mod", "a", true, undefined],
["mod", "z", true, undefined],
["mod+shift", "Z", true, undefined],
["mod", "c", false, undefined],
["mod", "v", false, undefined],
["mod", "a", false, undefined],
["mod", "z", false, undefined],
["mod", "y", false, undefined],
// Find, reload, history, and the pane's own address, picker and zoom chords.
["mod", "f", true, undefined],
["mod", "g", true, undefined],
["mod+shift", "G", true, undefined],
["mod", "r", true, undefined],
["mod", "r", false, undefined],
["mod", "[", true, undefined],
["mod", "]", true, undefined],
["mod", "l", true, undefined],
["mod+shift", "C", true, undefined],
["mod", "=", true, undefined],
["mod", "-", true, undefined],
["mod", "0", false, undefined],
// Caret movement, macOS Control keys, and plain typing.
["mod", "ArrowLeft", true, undefined],
["mod+shift", "ArrowRight", false, undefined],
["mod", "Backspace", false, undefined],
["ctrl", "b", true, undefined],
["ctrl+shift", "E", true, undefined],
["", "k", true, undefined],
["shift", "Tab", true, undefined],
]
const results = cases.map(([modifiers, key, mac, , code]) => [
modifiers,
key,
mac,
pageShortcut(press(modifiers, key, mac, code), mac)?.key,
])
expect(results).toEqual(cases.map(([modifiers, key, mac, forwarded]) => [modifiers, key, mac, forwarded]))
})
@@ -1,3 +1,24 @@
/** A key press in the page, with Electron's `Input` field names. */
export type PageKey = Pick<Electron.Input, "key" | "code" | "control" | "meta" | "shift" | "alt">
export type PageShortcut = {
readonly key: string
readonly code: string
readonly ctrl: boolean
readonly meta: boolean
readonly shift: boolean
readonly alt: boolean
}
// Unshifted chords with the platform's modifier that belong to the app shell or browser pane rather than the page:
// opening, closing or switching tabs, opening settings, and toggling the file tree.
const shellKeys = ["t", "w", "n", "1", "2", "3", "4", "5", "6", "7", "8", "9", ",", "\\"]
// Shifted chords with the platform's modifier that belong to the app shell rather than the page: reopening a tab,
// toggling the side panel, opening the command palette, or running session commands. New Window and Close Window
// (⇧⌘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 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.
@@ -9,3 +30,33 @@ export function pageLetter(input: Pick<Electron.Input, "key" | "code">) {
return input.code.slice(3).toLowerCase()
}
/**
* Returns the shortcut to forward to the app window when a focused page receives a key press that belongs to the
* browser pane or app shell, or `undefined` when the key belongs to the page (typing, editing, clipboard, find,
* reload, history, or in-page shortcuts like ⌘K and ⌘B).
*/
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 matched =
(input.control && !input.meta && !input.alt && (key === "tab" || (key === "`" && !input.shift))) ||
(mod &&
(input.alt
? !input.shift && (key === "arrowleft" || key === "arrowright")
: input.shift
? shellShiftKeys.includes(key)
: shellKeys.includes(key)))
if (!matched) return undefined
return {
key: /^[a-z]$/.test(key) ? key : input.key,
code: input.code,
ctrl: input.control,
meta: input.meta,
shift: input.shift,
alt: input.alt,
}
}
@@ -10,6 +10,7 @@ import {
type Connection,
type InspectEvent,
type Registration,
type ShortcutEvent,
type Zoom,
} from "./connection"
import { recordable, remember, withIcon } from "./history"
@@ -64,6 +65,8 @@ type PaneHandle = {
inspect: () => void
/** Focuses the address field and selects its text. */
focusAddress: () => void
/** Runs a key press the page handed to the app, as if it was pressed in this pane. */
press: (event: ShortcutEvent) => void
}
export type Model = ReturnType<typeof createModel>
@@ -214,6 +217,10 @@ export function createModel(ctx: SetupContext<typeof definition>) {
if (visits) history.set({ visits })
},
address: (tabID) => addressed.get(id)?.forEach((listener) => listener(tabID)),
// Only the pane on screen holds the page that had focus.
shortcut: (event) => {
if (sessions.current()?.key === id) panes()[0]?.press(event)
},
change: (next, mirror, native) => {
if (next.error === "browser.pane.unsupported") {
setState("unsupported", ref.server.id, true)
@@ -506,6 +506,13 @@ export function createBrowserPane(input: {
entry.win.webContents.focus()
publish(entry, { type: "address", tabID: id })
},
shortcut: (shortcut) => {
if (!entry.pages.has(id) || entry.focusedTabID !== id || entry.win.isDestroyed()) return
// As for the address shortcut: the command runs in the app, which may move focus there.
if (entry.win.isFocused()) entry.win.webContents.focus()
publish(entry, { type: "shortcut", ...shortcut })
},
popup: (popupOptions, background) => {
const popup = create(entry, false, popupOptions)
@@ -49,6 +49,7 @@ import type definition from "./index"
import { createModel } from "./model"
import SessionBrowserPane from "./panel"
import { BrowserPane, type PaneEvent } from "./ipc"
import { pageShortcut } from "./keys"
import { refusal } from "./policy"
/** The renderer host pieces the pane runs on, passed in by `packages/app/component-tests/browser-pane.spec.ts`. */
@@ -602,7 +603,7 @@ type RegionHost = {
/** Real shell command providers for shortcut cases. */
Shell: ParentComponent
/** Shell fallback and the real extension command adapter. */
Commands: Component<{ close: () => void }>
Commands: Component<{ close: () => void; review: () => void }>
/** The real host wrapper that renders an extension in its context. */
Contribution: Component<{ extension: string; children: () => JSX.Element }>
}
@@ -616,6 +617,8 @@ type RegionFixtureState = {
away: boolean
/** Whether the fixture's shell session was closed by its fallback shortcut. */
closed: boolean
/** How many times the review side panel toggled from a shortcut. */
reviews: number
/** The page main reports, blank or loaded. */
blank: boolean
/** Native browser tab close requests, by tab id. */
@@ -647,6 +650,7 @@ export function mountBrowserRegion(input: RegionHost) {
registrations: [],
away: false,
closed: false,
reviews: 0,
blank: false,
closes: [],
inspects: [],
@@ -880,6 +884,39 @@ export function mountBrowserRegion(input: RegionHost) {
)
}
// Wires an embedded page with the same before-input-event shortcut interception as chromium.ts.
const wirePage = (frame: HTMLIFrameElement) => {
const win = frame.contentWindow
if (!win) return
const mac = /(Mac|iPod|iPhone|iPad)/.test(navigator.platform)
win.addEventListener(
"keydown",
(event) => {
const shortcut = pageShortcut(
{
key: event.key,
code: event.code,
control: event.ctrlKey,
meta: event.metaKey,
shift: event.shiftKey,
alt: event.altKey,
},
mac,
)
const binding = latest()?.binding
if (!shortcut || !binding) return
event.preventDefault()
event.stopImmediatePropagation()
listeners.forEach((listener) => listener({ binding, event: { type: "shortcut", ...shortcut } }))
},
true,
)
}
// Mounts once every extension is active, as the session screen does.
function Strip() {
const region = input.createRegion({
@@ -974,7 +1011,10 @@ export function mountBrowserRegion(input: RegionHost) {
}}
ipc={(token) => (token.id === BrowserPane.id && !store.away ? pane : undefined)}
>
<input.Commands close={() => setStore("closed", true)} />
<input.Commands
close={() => setStore("closed", true)}
review={() => setStore("reviews", (count) => count + 1)}
/>
<h1 style={{ "font-size": "24px", "margin-bottom": "16px" }}>Restored side strip</h1>
<nav style={{ display: "flex", gap: "12px", margin: "16px 0" }}>
<button onClick={() => setStore({ session: beta, closed: false })}>Beta</button>
@@ -997,10 +1037,17 @@ export function mountBrowserRegion(input: RegionHost) {
<button onClick={() => setStore("away", true)}>Pane away</button>
<button onClick={() => setStore("away", false)}>Pane back</button>
</nav>
<iframe
title="Browser page"
srcdoc='<!doctype html><input aria-label="Editor page input" onkeydown="if ((event.ctrlKey || event.metaKey) &amp;&amp; /^[kw]$/i.test(event.key)) { event.preventDefault(); this.value = &apos;handled:&apos; + event.key.toLowerCase() }" />'
onLoad={(event) => wirePage(event.currentTarget)}
style={{ height: "48px" }}
/>
<p>Registrations: {store.registrations.length}</p>
<p>Beta restores: {latest()?.restore ?? 0}</p>
<p data-testid="tree">{JSON.stringify(tree())}</p>
<p data-testid="session-state">{store.closed ? "closed" : "open"}</p>
<p data-testid="review-toggles">{store.reviews}</p>
<p data-testid="browser-closes">{JSON.stringify(store.closes)}</p>
<p data-testid="browser-inspects">{JSON.stringify(store.inspects)}</p>
<Ready>
+23 -1
View File
@@ -166,6 +166,7 @@ export default function SessionBrowserPane(props: {
return tab ? props.model.embed(props.session, tab.id) : undefined
}
let root: HTMLElement | undefined
let box: HTMLDivElement | undefined
let input: HTMLInputElement | undefined
let anchor: HTMLFormElement | undefined
@@ -402,6 +403,23 @@ export default function SessionBrowserPane(props: {
inspectable,
inspect: toggleInspect,
focusAddress,
// Dispatched inside the pane, so keybinds scoped to it, such as closing the browser tab, apply.
press: (event) => {
if (!visible()) return
root?.dispatchEvent(
new KeyboardEvent("keydown", {
key: event.key,
code: event.code,
ctrlKey: event.ctrl,
metaKey: event.meta,
shiftKey: event.shift,
altKey: event.alt,
bubbles: true,
cancelable: true,
composed: true,
}),
)
},
}),
)
@@ -534,7 +552,11 @@ export default function SessionBrowserPane(props: {
)
return (
<aside id="browser-panel" class="relative size-full min-w-0 overflow-hidden bg-v2-background-bg-base flex flex-col">
<aside
ref={root}
id="browser-panel"
class="relative size-full min-w-0 overflow-hidden bg-v2-background-bg-base flex flex-col"
>
<div
data-component="browser-bar"
class="h-11 shrink-0 flex items-center gap-2 px-3 border-b-[0.5px] border-v2-border-border-muted"