fix(app): make ⇧⌘C work inside the browser pane (#55240)

This commit is contained in:
Kit Langton
2026-10-19 15:06:41 -07:00
committed by GitHub
Unverified
parent 964852d7ea
commit 67bcba0cf8
8 changed files with 91 additions and 16 deletions
@@ -147,6 +147,44 @@ story("closes a blank browser tab from its address field without closing the ses
await expect(root.getByTestId("session-state")).toHaveText("open")
})
story("toggles the element picker from the shortcut only inside the browser pane", async ({ page }) => {
const root = page.getByTestId("browser-region-fixture")
const outside = root.getByRole("button", { name: "Beta", exact: true })
await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible()
await outside.click()
await expect(root.getByText("Registrations: 2", { exact: true })).toBeVisible()
await root.getByRole("button", { name: "Blank page", exact: true }).click()
const inspect = root.getByRole("button", { name: "Select an element to comment on", exact: true })
const address = root.getByRole("combobox", { name: "Browser address", exact: true })
await expect(inspect).toBeDisabled()
await address.click()
await address.press("ControlOrMeta+Shift+c")
await expect(root.getByTestId("browser-inspects")).toHaveText("[]")
await root.getByRole("button", { name: "First inventory", exact: true }).click()
await expect(inspect).toBeEnabled()
await outside.focus()
await outside.press("ControlOrMeta+Shift+c")
await expect(inspect).toHaveAttribute("aria-pressed", "false")
await expect(root.getByTestId("browser-inspects")).toHaveText("[]")
await address.click()
await expect(address).toBeFocused()
await address.press("ControlOrMeta+Shift+c")
await expect(inspect).toHaveAttribute("aria-pressed", "true")
await expect(root.getByTestId("browser-inspects")).toHaveText(
'[{"tabID":"tab_33333333-3333-3333-3333-333333333333","enabled":true}]',
)
await address.press("ControlOrMeta+Shift+c")
await expect(inspect).toHaveAttribute("aria-pressed", "false")
await expect(root.getByTestId("browser-inspects")).toHaveText(
'[{"tabID":"tab_33333333-3333-3333-3333-333333333333","enabled":true},{"tabID":"tab_33333333-3333-3333-3333-333333333333","enabled":false}]',
)
})
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,6 +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 { createProfiling, type Recording } from "./profiling"
import type { BrowserNetwork } from "./network"
import {
@@ -192,7 +193,7 @@ export function createBrowserPage(
if (input.alt || !(process.platform === "darwin" ? input.meta : input.control)) return
// The same chord as Chromium DevTools' element picker.
if (input.shift && input.code === "KeyC") {
if (input.shift && pageLetter(input) === "c") {
event.preventDefault()
void toggleInspect(!inspecting)
@@ -200,7 +201,7 @@ export function createBrowserPage(
}
// The system browser's address shortcut. The app's own focus moves to the address field.
if (!input.shift && input.code === "KeyL") {
if (!input.shift && pageLetter(input) === "l") {
event.preventDefault()
options.address?.()
@@ -14,7 +14,6 @@ export default {
"refused.credentials": "Addresses with a user name or password can't open here.",
inspect: "Select an element to comment on",
"inspect.active": "Click an element in the page to comment on it. Press Escape to cancel.",
"inspect.pageShortcut": "While the page has focus",
"action.stop": "Stop",
replaced: "Browser control moved to another desktop window.",
unsupported: "This desktop app does not support the browser pane.",
@@ -0,0 +1,15 @@
import { expect, test } from "bun:test"
import { pageLetter } from "./keys"
test.each([
{ layout: "QWERTY", input: { key: "C", code: "KeyC" }, letter: "c" },
// Dvorak types C on the physical I key; Chromium reports the physical key as the code.
{ layout: "Dvorak", input: { key: "C", code: "KeyI" }, letter: "c" },
{ layout: "Dvorak", input: { key: "l", code: "KeyP" }, letter: "l" },
// Dvorak's J sits where QWERTY has C, so it must not open the picker.
{ layout: "Dvorak", input: { key: "J", code: "KeyC" }, letter: "j" },
{ layout: "Russian", input: { key: "\u0421", code: "KeyC" }, letter: "c" },
{ layout: "QWERTY", input: { key: "=", code: "Equal" }, letter: "=" },
])("$layout $input.key on $input.code is $letter", (row) => {
expect(pageLetter(row.input)).toBe(row.letter)
})
@@ -0,0 +1,11 @@
/**
* 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.
*/
export function pageLetter(input: Pick<Electron.Input, "key" | "code">) {
const key = input.key.toLowerCase()
if (/^[a-z]$/.test(key) || !/^Key[A-Z]$/.test(input.code)) return key
return input.code.slice(3).toLowerCase()
}
@@ -618,6 +618,8 @@ type RegionFixtureState = {
blank: boolean
/** Native browser tab close requests, by tab id. */
closes: string[]
/** Native element picker requests, by tab id and state. */
inspects: { tabID: string; enabled: boolean }[]
}
// Component-test fixture: the real side region over the real browser and file extensions, with the host's
@@ -645,6 +647,7 @@ export function mountBrowserRegion(input: RegionHost) {
closed: false,
blank: false,
closes: [],
inspects: [],
})
// The file tree's state, Changes or All files, as the file extension stores it.
@@ -813,7 +816,12 @@ export function mountBrowserRegion(input: RegionHost) {
listener({ binding: value.binding, event: { type: "state", state: { tabs: [], focusedTabID: null } } }),
)
},
inspect: async () => undefined,
inspect: async (value) => {
setStore("inspects", (items) => [...items, { tabID: value.tabID, enabled: value.enabled }])
listeners.forEach((listener) =>
listener({ binding: value.binding, event: { type: "inspect", tabID: value.tabID, active: value.enabled } }),
)
},
highlight: async () => undefined,
zoom: async () => undefined,
site: async () => ({ cookies: 0 }),
@@ -966,7 +974,14 @@ export function mountBrowserRegion(input: RegionHost) {
<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>
<button onClick={inventory}>First inventory</button>
<button
onClick={() => {
setStore("blank", false)
inventory()
}}
>
First inventory
</button>
<button
onClick={() => {
setStore("blank", true)
@@ -983,6 +998,7 @@ export function mountBrowserRegion(input: RegionHost) {
<p data-testid="tree">{JSON.stringify(tree())}</p>
<p data-testid="session-state">{store.closed ? "closed" : "open"}</p>
<p data-testid="browser-closes">{JSON.stringify(store.closes)}</p>
<p data-testid="browser-inspects">{JSON.stringify(store.inspects)}</p>
<Ready>
<Strip />
</Ready>
+1 -10
View File
@@ -789,16 +789,7 @@ export default function SessionBrowserPane(props: {
</Popover.Portal>
</Popover>
<div class="flex shrink-0 items-center gap-1">
<Tooltip
placement="top"
value={
<div class="flex flex-col gap-1">
{tip(extension.t("inspect"), keybinds.keybind("browser.inspect"))}
{/* The page claims Chromium's picker chord itself; the app leaves it to the terminal. */}
{tip(extension.t("inspect.pageShortcut"), keybinds.keys("mod+shift+c"))}
</div>
}
>
<Tooltip placement="top" value={tip(extension.t("inspect"), keybinds.keybind("browser.inspect"))}>
{/* The ghost variant sets the button color, so the active accent needs precedence over it. */}
<IconButton
variant="ghost-muted"
@@ -107,7 +107,8 @@ const setup: Setup<typeof definition> = (ctx) => {
}
})
// Ctrl+Shift+C copies in the terminal, so only the focused page claims it, as in Chromium.
// Chromium's element picker chord. While the page has focus, the page claims it itself; here it covers focus elsewhere
// in the pane. Ctrl+Shift+C copies in the terminal outside the pane.
ctx.add(Command, (): Command | undefined => {
const pane = model()?.pane()
@@ -117,6 +118,9 @@ const setup: Setup<typeof definition> = (ctx) => {
id: "inspect",
title: ctx.t("command.inspect"),
group: ctx.t("command.category.view"),
bind: "mod+shift+c",
scope: "#browser-panel",
editable: true,
enabled: pane.visible() && pane.inspectable(),
run: pane.inspect,
}