Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 104 additions & 4 deletions packages/app/component-tests/browser-pane-restore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -42,6 +43,7 @@ story.beforeEach(async ({ mount, page }) => {
platform,
solid,
closed,
extensionCommands,
] = await Promise.all([
import(modules.fixture),
import(modules.host),
Expand All @@ -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),
])
Expand Down Expand Up @@ -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<string>) => void
}) => {
Expand All @@ -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) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copies the sideKey / file.close / file.tab.new logic from use-session-commands.tsx into the fixture, so the stories test the copy, not the commands the app ships. If the real when or closeTab changes, these stories will still pass. Could the fixture reuse the real command definitions, or could the stories be cut down to the region and focus behaviour they actually exercise?

!!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",
Expand Down Expand Up @@ -305,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()
Expand Down Expand Up @@ -344,6 +380,70 @@ 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 }) => {
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")

// 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()
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_/)
await expect(root.getByTestId("session-state")).toHaveText("open")
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 }) => {
Expand Down
8 changes: 4 additions & 4 deletions packages/app/src/runtime/extension/closed-tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
24 changes: 23 additions & 1 deletion packages/app/src/runtime/extension/commands.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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<Command>[],
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()
Expand Down
30 changes: 29 additions & 1 deletion packages/app/src/runtime/extension/panels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
batch,
createEffect,
createMemo,
createSignal,
For,
mergeProps,
on,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand All @@ -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()
Expand Down
26 changes: 11 additions & 15 deletions packages/app/src/runtime/extension/side-region.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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() }}
Expand Down Expand Up @@ -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

Expand All @@ -253,7 +249,7 @@ export function PaneTabs(props: {
selectionEvent.target instanceof Element &&
selectionEvent.target.closest('[role="tab"]')
) {
setAdding(undefined)
dismiss()
props.region.select(value)
}
}}
Expand Down Expand Up @@ -292,7 +288,7 @@ export function PaneTabs(props: {
<Tabs.Trigger
value="new"
class="group"
onMiddleClick={() => setAdding(undefined)}
onMiddleClick={dismiss}
closeButton={
<IconButton
size="small"
Expand All @@ -304,7 +300,7 @@ export function PaneTabs(props: {
onClick={(event) => {
event.preventDefault()
event.stopPropagation()
setAdding(undefined)
dismiss()
}}
icon={<Icon name="xmark-small" />}
aria-label={language.t("common.closeTab")}
Expand Down Expand Up @@ -335,7 +331,7 @@ export function PaneTabs(props: {
icon={<Icon name="plus" />}
variant="ghost-muted"
size="large"
onClick={() => setAdding({ session: props.view.key, over: props.pane.active() })}
onClick={add}
aria-label={language.t("session.tab.add")}
/>
</Tooltip>
Expand All @@ -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}
/>
)}
</Show>
Expand Down
Loading
Loading