diff --git a/packages/app/e2e/regression/remote-session-settings.spec.ts b/packages/app/e2e/regression/remote-session-settings.spec.ts index af64ef42dbd3..2194d86c0ff1 100644 --- a/packages/app/e2e/regression/remote-session-settings.spec.ts +++ b/packages/app/e2e/regression/remote-session-settings.spec.ts @@ -35,7 +35,8 @@ test("session settings use the remote server context", async ({ page }) => { await expect(activeTabs).toHaveCount(0) await expect(page.getByRole("dialog")).toHaveCount(0) await expect(settings.getByRole("tablist")).toHaveCSS("width", "328px") - await expect(sessionHeading).toBeAttached() + await expect(page).toHaveURL((url) => url.pathname === "/settings" && url.searchParams.get("server") === serverB) + await expect(sessionHeading).not.toBeAttached() await expect(sessionHeading).toBeHidden() const autoAccept = settings.locator('[data-action="settings-auto-accept-permissions"]') const input = autoAccept.getByRole("switch") @@ -65,12 +66,23 @@ test("session settings use the remote server context", async ({ page }) => { ]) await settings.getByRole("tab", { name: "Models" }).click() + await expect(page).toHaveURL((url) => url.pathname === "/settings" && url.searchParams.get("tab") === "models") await expect(settings.getByRole("switch", { name: "Server B Model" })).toBeEnabled() await expect(settings.getByRole("switch", { name: "Server A Model" })).toHaveCount(0) + await page.reload() + await expect(settings.getByRole("tab", { name: "Models" })).toHaveAttribute("aria-selected", "true") + await expect(settings.getByRole("switch", { name: "Server B Model" })).toBeEnabled() + await expect(page).toHaveURL((url) => url.searchParams.get("directory") === directoryB) + await expect(activeTabs).toHaveCount(0) await settings.getByRole("button", { name: "Back to app" }).click() await expect(settings).toBeHidden() await expect(sessionHeading).toBeVisible() await expect(activeTabs).toHaveCount(1) + await page.goForward() + await expect(settings.getByRole("tab", { name: "Models" })).toHaveAttribute("aria-selected", "true") + await expect(activeTabs).toHaveCount(0) + await page.goBack() + await expect(sessionHeading).toBeVisible() await page.keyboard.press("Control+,") await expect(settings).toBeVisible() await page.locator(`[data-titlebar-tab-slot]:has(a[href$="/session/${sessionB.id}"])`).click() @@ -79,6 +91,146 @@ test("session settings use the remote server context", async ({ page }) => { await expect(activeTabs).toHaveCount(1) }) +test("settings returns to a vertical draft tab without losing its prompt", async ({ page }) => { + await installSseTransport(page, { server: serverA }) + await installSseTransport(page, { server: serverB }) + await mockServers(page, []) + await configureServers(page) + await page.addInitScript( + ({ server, directory }) => { + localStorage.setItem("settings.v3", JSON.stringify({ appearance: { tabLayout: "vertical" } })) + localStorage.setItem( + "opencode.window.browser.dat:tabs", + JSON.stringify([{ type: "draft", draftID: "settings-draft", server, directory }]), + ) + }, + { server: serverB, directory: directoryB }, + ) + await page.goto("/new-session?draftId=settings-draft") + const editor = page.locator('[data-component="composer-editor"][contenteditable="true"]') + await expect(editor).toBeEditable() + await editor.fill("Keep this draft while visiting settings") + const activeTabs = page.locator('[data-slot="vertical-tabs-sidebar"] [data-titlebar-tab-slot][data-active="true"]') + await expect(activeTabs).toHaveCount(1) + await page.keyboard.press("Control+,") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await expect(activeTabs).toHaveCount(0) + await expect(page).toHaveURL( + (url) => + url.pathname === "/settings" && + url.searchParams.get("server") === serverB && + url.searchParams.get("directory") === directoryB, + ) + await page.keyboard.press("Escape") + await expect(page).toHaveURL(/\/new-session\?draftId=settings-draft$/) + await expect(editor).toHaveText("Keep this draft while visiting settings") + await expect(activeTabs).toHaveCount(1) +}) + +test("direct settings entry works on mobile and returns home", async ({ page }) => { + await page.setViewportSize({ width: 390, height: 720 }) + await installSseTransport(page, { server: serverA }) + await installSseTransport(page, { server: serverB }) + await mockServers(page, []) + await configureServers(page, [{ type: "session", server: serverB, sessionId: sessionB.id }]) + await page.goto("/settings?tab=unknown") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await expect(settings.getByRole("tab", { name: "Preferences" })).toHaveAttribute("aria-selected", "true") + await expect(page.locator('[data-titlebar-tab-slot][data-active="true"]')).toHaveCount(0) + await settings.getByRole("button", { name: "Back to app" }).click() + await expect(page).toHaveURL((url) => url.pathname === "/") + await expect(settings).not.toBeAttached() +}) + +test("closing the previous session tab does not leave settings", async ({ page }) => { + await installSseTransport(page, { server: serverA }) + await installSseTransport(page, { server: serverB }) + await mockServers(page, []) + await configureServers(page) + await page.goto(`/server/${base64Encode(serverB)}/session/${sessionB.id}`) + await expect(page.getByRole("heading", { name: sessionB.title, exact: true })).toBeVisible() + await page.keyboard.press("Control+,") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + const tab = page.locator(`[data-titlebar-tab-slot]:has(a[href$="/session/${sessionB.id}"])`) + await tab.hover() + await tab.locator('[data-slot="tab-close"]').click() + await expect(tab).not.toBeAttached() + await expect(settings).toBeVisible() + await expect(page).toHaveURL((url) => url.pathname === "/settings") +}) + +test("new tabs opened from settings keep the originating remote directory and model", async ({ page }) => { + await installSseTransport(page, { server: serverA }) + await installSseTransport(page, { server: serverB }) + await mockServers(page, []) + await configureServers(page) + await page.route(`${serverB}/api/model**`, (route) => { + if (new URL(route.request().url()).pathname !== "/api/model") return route.fallback() + return json(route, { + location: { directory: directoryB }, + data: [model(true), { ...model(true), id: "alternate", modelID: "alternate", name: "Alternate B Model" }], + }) + }) + await page.addInitScript(() => { + localStorage.setItem( + "opencode.global.dat:model", + JSON.stringify({ + user: [{ providerID: "server-b", modelID: "alternate", visibility: "show" }], + }), + ) + }) + await page.goto(`/server/${base64Encode(serverB)}/session/${sessionB.id}`) + await expect(page.getByRole("heading", { name: sessionB.title, exact: true })).toBeVisible() + const modelControl = page.locator('[data-action="composer-model"]') + await modelControl.click() + await page.getByRole("menuitemradio", { name: "Alternate B Model", exact: true }).click() + await expect(modelControl).toContainText("Alternate B Model") + await page.keyboard.press("Control+,") + await expect(page.getByTestId("settings-screen")).toBeFocused() + await page.locator('[data-slot="titlebar-v2"]').getByRole("button", { name: "New session", exact: true }).click() + await expect(page).toHaveURL((url) => url.pathname === "/new-session" && !!url.searchParams.get("draftId")) + await expect(page.locator('[data-component="composer-editor"][contenteditable="true"]')).toBeEditable() + await expect(modelControl).toContainText("Alternate B Model") + await expect + .poll(() => + page.evaluate(() => { + const draftID = new URLSearchParams(location.search).get("draftId") + const tabs = JSON.parse(localStorage.getItem("opencode.window.browser.dat:tabs") ?? "[]") + return tabs.find((tab: { draftID?: string }) => tab.draftID === draftID) + }), + ) + .toMatchObject({ type: "draft", server: serverB, directory: directoryB }) +}) + +test("settings navigation stays aligned with app Back and Forward commands", async ({ page }) => { + await installSseTransport(page, { server: serverA }) + await installSseTransport(page, { server: serverB }) + await mockServers(page, []) + await configureServers(page, [{ type: "session", server: serverB, sessionId: sessionB.id }]) + await page.goto("/") + await page.locator(`[data-titlebar-tab-slot]:has(a[href$="/session/${sessionB.id}"])`).click() + const heading = page.getByRole("heading", { name: sessionB.title, exact: true }) + await expect(heading).toBeVisible() + await page.keyboard.press("Control+,") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await settings.getByRole("tab", { name: "Models" }).click() + await expect(settings.getByRole("switch", { name: "Server B Model" })).toBeEnabled() + await settings.getByRole("button", { name: "Back to app" }).click() + await expect(heading).toBeVisible() + await page.keyboard.press("Control+[") + await expect(page).toHaveURL((url) => url.pathname === "/") + await page.keyboard.press("Control+]") + await expect(heading).toBeVisible() + await page.keyboard.press("Control+]") + await expect(settings.getByRole("tab", { name: "Models" })).toHaveAttribute("aria-selected", "true") + await settings.getByRole("button", { name: "Back to app" }).click() + await expect(heading).toBeVisible() +}) + test("new session works while draft settings are open", async ({ page }) => { await installSseTransport(page, { server: serverA }) await installSseTransport(page, { server: serverB }) @@ -435,6 +587,7 @@ async function mockServers( return json(route, { directory, project: { id: remote ? sessionB.projectID : "project-server-a" } }) if (url.pathname === "/api/vcs") return json(route, { location: { directory }, data: { branch: { current: "main", default: "main" } } }) + if (url.pathname === "/api/vcs/branches") return json(route, { location: { directory }, data: ["main"] }) if (url.pathname === "/api/pty/shells") return json(route, { location: { directory }, data: [] }) return json(route, {}) }) diff --git a/packages/app/e2e/regression/settings-history.spec.ts b/packages/app/e2e/regression/settings-history.spec.ts new file mode 100644 index 000000000000..7f40be0b61a1 --- /dev/null +++ b/packages/app/e2e/regression/settings-history.spec.ts @@ -0,0 +1,106 @@ +import { expect, test } from "@playwright/test" +import { mockOpenCodeServer } from "../utils/mock-server" + +const server = `http://${process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"}:${process.env.PLAYWRIGHT_SERVER_PORT ?? "4096"}` +const directory = "/settings-history" +const draft = "/new-session?draftId=settings-history" + +test.beforeEach(async ({ page }) => { + await mockOpenCodeServer(page, { + directory, + project: { + id: "proj_settings_history", + worktree: directory, + vcs: "git", + name: "settings-history", + time: { created: 1700000000000, updated: 1700000000000 }, + sandboxes: [], + }, + provider: { all: [], connected: [], default: {} }, + sessions: [], + pageMessages: () => ({ items: [] }), + }) + await page.addInitScript( + ({ server, directory }) => { + localStorage.setItem( + "opencode.window.browser.dat:tabs", + JSON.stringify([{ type: "draft", draftID: "settings-history", server, directory }]), + ) + }, + { server, directory }, + ) +}) + +test("settings close, app commands, and browser traversal share one history", async ({ page }) => { + await page.goto("/") + await page.locator(`[data-titlebar-tab-slot] a[href="${draft}"]`).click() + const editor = page.locator('[data-component="composer-editor"][contenteditable="true"]') + await expect(editor).toBeEditable() + await page.keyboard.press("Control+,") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await settings.getByRole("tab", { name: "Appearance", exact: true }).click() + + await page.keyboard.press("Control+[") + await expect(editor).toBeEditable() + await expect(page).toHaveURL((url) => `${url.pathname}${url.search}` === draft) + await page.goForward() + await expect(settings.getByRole("tab", { name: "Appearance", exact: true })).toHaveAttribute("aria-selected", "true") + await page.keyboard.press("Escape") + await expect(editor).toBeEditable() + await page.keyboard.press("Control+]") + await expect(settings).toBeFocused() + await expect(settings.getByRole("tab", { name: "Appearance", exact: true })).toHaveAttribute("aria-selected", "true") + await settings.getByRole("button", { name: "Back to app" }).click() + await expect(editor).toBeEditable() + + await page.keyboard.press("Control+[") + await expect(page).toHaveURL((url) => url.pathname === "/") + await expect(settings).not.toBeAttached() + await page.goForward() + await expect(editor).toBeEditable() + await page.goForward() + await expect(settings).toBeFocused() + await page.goBack() + await expect(editor).toBeEditable() + await page.keyboard.press("Control+]") + await expect(settings).toBeFocused() +}) + +test("direct settings entry and reload fall back home without leaving the app", async ({ page }) => { + await page.goto("/settings?tab=appearance") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await page.reload() + await expect(settings).toBeFocused() + await page.keyboard.press("Escape") + await expect(page).toHaveURL((url) => url.pathname === "/") + await expect(settings).not.toBeAttached() + await page.keyboard.press("Control+]") + await expect(page).toHaveURL((url) => url.pathname === "/") +}) + +test("reload retains known browser history but direct entry does not adopt stale history", async ({ page }) => { + await page.goto(draft) + const editor = page.locator('[data-component="composer-editor"][contenteditable="true"]') + await expect(editor).toBeEditable() + await page.keyboard.press("Control+,") + const settings = page.getByTestId("settings-screen") + await expect(settings).toBeFocused() + await settings.getByRole("tab", { name: "Appearance", exact: true }).click() + const url = page.url() + await page.reload() + await expect(settings).toBeFocused() + await settings.getByRole("button", { name: "Back to app" }).click() + await expect(editor).toBeEditable() + await expect(page).toHaveURL((url) => `${url.pathname}${url.search}` === draft) + await page.goForward() + await expect(settings.getByRole("tab", { name: "Appearance", exact: true })).toHaveAttribute("aria-selected", "true") + await page.keyboard.press("Escape") + await expect(editor).toBeEditable() + + await page.goto(url) + await expect(settings).toBeFocused() + await page.keyboard.press("Escape") + await expect(page).toHaveURL((url) => url.pathname === "/") +}) diff --git a/packages/app/src/settings/command.tsx b/packages/app/src/settings/command.tsx index 4258c7d3d3f4..7d222ae33a97 100644 --- a/packages/app/src/settings/command.tsx +++ b/packages/app/src/settings/command.tsx @@ -1,9 +1,9 @@ import { useCommand } from "@/shell/commands/command" import { useLanguage } from "@/runtime/i18n/language" -import { useSettingsSurface } from "./surface" +import { useSettingsNavigation } from "./navigation" export function useSettingsDialog(defaultValue?: string) { - const settings = useSettingsSurface() + const settings = useSettingsNavigation() return () => settings.open(defaultValue) } diff --git a/packages/app/src/settings/navigation.ts b/packages/app/src/settings/navigation.ts new file mode 100644 index 000000000000..6865112b72a0 --- /dev/null +++ b/packages/app/src/settings/navigation.ts @@ -0,0 +1,42 @@ +import { useLocation, useNavigate } from "@solidjs/router" +import { useLayout } from "@/shell/state/layout" +import { useTabs } from "@/shell/tabs/tabs" +import { useGlobal } from "@/runtime/server/runtime" +import { ServerConnection } from "@/runtime/server/registry" +import { useTitlebarHistory } from "@/shell/titlebar/history-context" + +export function useSettingsNavigation() { + const location = useLocation() + const navigate = useNavigate() + const layout = useLayout() + const tabs = useTabs() + const global = useGlobal() + const history = useTitlebarHistory() + + return { + open(tab = "general") { + const route = layout.route() + const query = new URLSearchParams(route.type === "settings" ? location.search : "") + query.set("tab", tab) + if (route.type !== "settings") { + query.set("from", `${location.pathname}${location.search}${location.hash}`) + const draft = route.type === "draft" ? tabs.draft(route.draftID) : undefined + const server = route.type === "session" ? route.server : (draft?.server ?? layout.home.selection().server) + const connection = global.servers.list().find((item) => ServerConnection.key(item) === server) + const directory = + route.type === "session" && connection + ? global.ensureServerCtx(connection).data.session.get(route.sessionId)?.location.directory + : draft?.directory + query.set("server", server) + if (directory) query.set("directory", directory) + } + navigate(`/settings?${query}`, { + replace: route.type === "settings", + }) + }, + close() { + if (layout.route().type !== "settings") return + history.back() + }, + } +} diff --git a/packages/app/src/settings/shell.tsx b/packages/app/src/settings/shell.tsx index d9f2326757c1..6a868172510a 100644 --- a/packages/app/src/settings/shell.tsx +++ b/packages/app/src/settings/shell.tsx @@ -1,4 +1,5 @@ -import { Component, createEffect, createMemo, createSignal, onCleanup, onMount, startTransition } from "solid-js" +import { createEffect, createMemo, onMount } from "solid-js" +import { useLocation, useNavigate } from "@solidjs/router" import { Tabs } from "@opencode-ai/ui/tabs" import { Icon } from "@opencode-ai/ui/icon" import { useLanguage } from "@/runtime/i18n/language" @@ -14,50 +15,55 @@ import { SettingsProjects } from "./workspaces/projects" import { SettingsExtensions } from "./providers/extensions" import { SettingsServerScope } from "./server-scope" import { useDialog } from "@opencode-ai/ui/context/dialog" -import { useLayout } from "@/shell/state/layout" -import { useTabs } from "@/shell/tabs/tabs" -import { useGlobal, useServerCtx } from "@/runtime/server/runtime" +import { useGlobal } from "@/runtime/server/runtime" import { ServerConnection, useServers } from "@/runtime/server/registry" -import { useCommand } from "@/shell/commands/command" -import { useSettingsSurface } from "./surface" +import { useSettingsNavigation } from "./navigation" +import { useSettingsCommand } from "./command" import "@/settings/settings.css" -export const SettingsScreen: Component<{ - defaultValue?: string -}> = (props) => { +export function SettingsScreen() { const language = useLanguage() const dialog = useDialog() - const command = useCommand() - const surface = useSettingsSurface() - const layout = useLayout() + const navigation = useSettingsNavigation() + const location = useLocation() + const navigate = useNavigate() const servers = useServers() - const tabs = useTabs() const global = useGlobal() - const [tab, setTab] = createSignal(props.defaultValue ?? "general") + useSettingsCommand() + const tab = () => { + const value = location.query.tab + return typeof value === "string" && + [ + "general", + "appearance", + "notifications", + "shortcuts", + "servers", + "projects", + "workspaces", + "providers", + "models", + "extensions", + ].includes(value) + ? value + : "general" + } + const setTab = (value: string) => { + const query = new URLSearchParams(location.search) + query.set("tab", value) + navigate(`/settings?${query}`, { replace: true, scroll: false }) + } let root: HTMLDivElement | undefined onMount(() => { - command.keybinds(false) root?.focus({ preventScroll: true }) }) - onCleanup(() => command.keybinds(true)) - - createEffect(() => setTab(props.defaultValue ?? "general")) - const server = createMemo(() => { - const route = layout.route() - switch (route.type) { - case "draft": { - const draft = tabs.store.find((item) => item.type === "draft" && item.draftID === route.draftID) - return servers.list.find((item) => ServerConnection.key(item) === draft?.server) - } - case "session": - return servers.list.find((item) => ServerConnection.key(item) === route.server) - case "home": - return servers.list.find((item) => ServerConnection.key(item) === layout.home.selection().server) - } - }) - const serverCtx = useServerCtx(server) + const server = createMemo( + () => + servers.list.find((item) => ServerConnection.key(item) === location.query.server) ?? + global.settings.server.selected(), + ) createEffect(() => { const current = server() @@ -68,13 +74,8 @@ export const SettingsScreen: Component<{ const selected = global.settings.server.selected() const current = server() if (!selected || !current || ServerConnection.key(selected) !== ServerConnection.key(current)) return - const route = layout.route() - if (route.type === "draft") { - const draft = tabs.store.find((item) => item.type === "draft" && item.draftID === route.draftID) - return draft?.type === "draft" ? draft.directory : undefined - } - if (route.type === "session") return serverCtx()?.data.session.get(route.sessionId)?.location.directory - return undefined + if (location.query.server !== ServerConnection.key(current)) return + return typeof location.query.directory === "string" ? location.query.directory : undefined }) const showProviders = () => { @@ -91,19 +92,13 @@ export const SettingsScreen: Component<{ onKeyDown={(event) => { if (event.key !== "Escape" || event.defaultPrevented || dialog.active) return event.preventDefault() - surface.close() + navigation.close() }} > - void startTransition(() => setTab(value))} - class="settings" - > +
- diff --git a/packages/app/src/settings/surface.tsx b/packages/app/src/settings/surface.tsx deleted file mode 100644 index cc24ee840f22..000000000000 --- a/packages/app/src/settings/surface.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import { useLocation } from "@solidjs/router" -import { createEffect, on } from "solid-js" -import { createStore } from "solid-js/store" -import { createSimpleContext } from "@opencode-ai/ui/context" - -export const { use: useSettingsSurface, provider: SettingsSurfaceProvider } = createSimpleContext({ - name: "SettingsSurface", - gate: false, - init: () => { - const location = useLocation() - const [store, setStore] = createStore({ open: false, tab: "general" }) - let focus: HTMLElement | undefined - - const close = () => { - if (!store.open) return - setStore("open", false) - if (focus?.isConnected) focus.focus({ preventScroll: true }) - focus = undefined - } - - createEffect(on(() => `${location.pathname}${location.search}`, close, { defer: true })) - - return { - store, - open(tab = "general") { - if (!store.open && document.activeElement instanceof HTMLElement) focus = document.activeElement - setStore({ open: true, tab }) - }, - close, - } - }, -}) diff --git a/packages/app/src/shell/routes/routes.tsx b/packages/app/src/shell/routes/routes.tsx index 8edc1b9c7fc1..756bf2fcd53e 100644 --- a/packages/app/src/shell/routes/routes.tsx +++ b/packages/app/src/shell/routes/routes.tsx @@ -6,7 +6,7 @@ import { useGlobal } from "@/runtime/server/runtime" import { ServerConnection } from "@/runtime/server/registry" import { SessionPanelFrame, SessionRouteFrame } from "@/session/session-frame" import { LayoutProvider } from "@/shell/state/layout" -import { SettingsSurfaceProvider } from "@/settings/surface" +import { TitlebarHistoryProvider } from "@/shell/titlebar/history-context" import Shell from "@/shell/shell" import { requireServerKey } from "./session" @@ -14,12 +14,14 @@ export const File = lazy(() => import("@opencode-ai/session-ui/file").then((modu const loadDraftRoute = () => Promise.all([import("@/new-session/route"), File.preload()]).then(([module]) => module) const loadSessionRoute = () => Promise.all([import("@/session/route"), File.preload()]).then(([module]) => module) const DraftRoute = lazy(() => loadDraftRoute().then((module) => ({ default: module.DraftRoute }))) +const SettingsRoute = lazy(() => import("@/settings/shell").then((module) => ({ default: module.SettingsScreen }))) const TargetSessionRouteContent = lazy(() => loadSessionRoute().then((module) => ({ default: module.TargetSessionRouteContent })), ) export function preloadRoute(url: string) { const pathname = url.split(/[?#]/, 1)[0] + if (pathname === "/settings") return SettingsRoute.preload().then(() => undefined) if (pathname === "/new-session") return DraftRoute.preload().then(() => undefined) if (/^\/server\/[^/]+\/session\/[^/]+$/.test(pathname)) return TargetSessionRouteContent.preload().then(() => undefined) @@ -30,6 +32,7 @@ export function AppRoutes() { return ( + ( @@ -70,9 +73,9 @@ function TargetServerRoute(props: ParentProps) { function AppLayout(props: ParentProps) { return ( - + {props.children} - + ) } diff --git a/packages/app/src/shell/shell.tsx b/packages/app/src/shell/shell.tsx index ea914843ce41..817cbe6264b7 100644 --- a/packages/app/src/shell/shell.tsx +++ b/packages/app/src/shell/shell.tsx @@ -6,15 +6,12 @@ import { Titlebar, type TitlebarUpdate } from "@/shell/titlebar/titlebar" import { usePlatform } from "@/runtime/platform/platform" import { ToastRegion } from "@/shell/notifications/toast" import { TitlebarRightProvider } from "@/shell/titlebar/right-slot" -import { useSettingsSurface } from "@/settings/surface" import { useSettings } from "@/settings/model" const DebugBar = lazy(() => import("@/shell/debug/debug-bar").then((module) => ({ default: module.DebugBar }))) -const SettingsScreen = lazy(() => import("@/settings/shell").then((module) => ({ default: module.SettingsScreen }))) export default function Layout(props: ParentProps) { const platform = usePlatform() - const settings = useSettingsSurface() const preferences = useSettings() const mobile = createMediaQuery("(max-width: 767px)") const [state, setState] = createStore({ @@ -73,19 +70,9 @@ export default function Layout(props: ParentProps) {
-
diff --git a/packages/app/src/shell/state/layout.tsx b/packages/app/src/shell/state/layout.tsx index a962ca23808d..19d0a44c7972 100644 --- a/packages/app/src/shell/state/layout.tsx +++ b/packages/app/src/shell/state/layout.tsx @@ -62,6 +62,7 @@ export type ReviewPanelSource = "context-button" | "other" export type LayoutRoute = | { type: "home" } + | { type: "settings" } | { type: "draft"; draftID: string } | { type: "session"; sessionId: string; server: ServerConnection.Key } @@ -100,6 +101,7 @@ const normalizeStoredSessionTabs = (key: string, tabs: SessionTabs) => { export const currentRoute = (pathname: string, search: string): LayoutRoute => { const parts = pathname.split("/").filter(Boolean) if (parts.length === 0) return { type: "home" } + if (parts.length === 1 && parts[0] === "settings") return { type: "settings" } if (parts[0] === "new-session") { const draftID = new URLSearchParams(search).get("draftId") diff --git a/packages/app/src/shell/tabs/tabs.tsx b/packages/app/src/shell/tabs/tabs.tsx index c9234d16f1d8..a2acfcae164b 100644 --- a/packages/app/src/shell/tabs/tabs.tsx +++ b/packages/app/src/shell/tabs/tabs.tsx @@ -174,7 +174,12 @@ export const { use: useTabs, provider: TabsProvider } = createSimpleContext({ if (!tab) return const key = tabKey(tab) const draftID = tab.type === "draft" ? tab.draftID : undefined - const nextTab = nextTabAfterClose(store, index, recentKey() === key && location.pathname !== "/") + const route = currentRoute() + const nextTab = nextTabAfterClose( + store, + index, + recentKey() === key && (route.type === "session" || route.type === "draft"), + ) closing.add(key) void startTransition(() => { setStore( diff --git a/packages/app/src/shell/titlebar/history-context.ts b/packages/app/src/shell/titlebar/history-context.ts new file mode 100644 index 000000000000..645bddfb3051 --- /dev/null +++ b/packages/app/src/shell/titlebar/history-context.ts @@ -0,0 +1,67 @@ +import { useBeforeLeave, useLocation, useNavigate } from "@solidjs/router" +import { createEffect, on } from "solid-js" +import { createStore } from "solid-js/store" +import { createSimpleContext } from "@opencode-ai/ui/context" +import { makePersisted } from "@solid-primitives/storage" +import { usePlatform } from "@/runtime/platform/platform" +import { applyPath, type TitlebarAction, type TitlebarHistory } from "./history" + +export function createTitlebarHistory(input: { storage?: Storage; restore?: boolean } = {}) { + const location = useLocation() + const navigate = useNavigate() + const path = () => `${location.pathname}${location.search}${location.hash}` + const state = createStore({ stack: [], index: 0 }) + const [store, setStore] = input.storage + ? makePersisted(state, { storage: input.storage, name: "opencode.navigation-history" }) + : state + if (!input.restore || !Array.isArray(store.stack) || store.stack[store.index] !== path()) { + setStore({ stack: [], index: 0 }) + } + let action: TitlebarAction | undefined + + useBeforeLeave((event) => { + action = typeof event.to === "number" ? event.to : { replace: event.options?.replace } + }) + + createEffect( + on(path, (path) => { + setStore(applyPath(store, path, action)) + action = undefined + }), + ) + + const go = (delta: number) => { + if (!store.stack[store.index + delta]) return false + // Set the intent ourselves as MemoryRouter.go bypasses useBeforeLeave. + action = delta + navigate(delta) + return true + } + + return { + back() { + if (!go(-1) && location.pathname !== "/") navigate("/", { replace: true }) + }, + forward() { + go(1) + }, + } +} + +export const { use: useTitlebarHistory, provider: TitlebarHistoryProvider } = createSimpleContext({ + name: "TitlebarHistory", + init: () => { + const platform = usePlatform() + // Browser history survives reload; desktop recreates its MemoryRouter with one entry. + return createTitlebarHistory( + platform.platform === "web" + ? { + storage: sessionStorage, + restore: performance + .getEntriesByType("navigation") + .some((entry) => entry instanceof PerformanceNavigationTiming && entry.type === "reload"), + } + : undefined, + ) + }, +}) diff --git a/packages/app/src/shell/titlebar/history.test.ts b/packages/app/src/shell/titlebar/history.test.ts index 3754ae7c5f61..d004b5e585c3 100644 --- a/packages/app/src/shell/titlebar/history.test.ts +++ b/packages/app/src/shell/titlebar/history.test.ts @@ -1,63 +1,44 @@ import { describe, expect, test } from "bun:test" -import { applyPath, backPath, forwardPath, type TitlebarHistory } from "./history" - -function history(): TitlebarHistory { - return { stack: [], index: 0, action: undefined } -} +import { applyPath, type TitlebarHistory } from "./history" describe("titlebar history", () => { - test("append and trim keeps max bounded", () => { - let state = history() - state = applyPath(state, "/", 3) - state = applyPath(state, "/a", 3) - state = applyPath(state, "/b", 3) - state = applyPath(state, "/c", 3) - - expect(state.stack).toEqual(["/a", "/b", "/c"]) - expect(state.stack.length).toBe(3) - expect(state.index).toBe(2) + test("starts with only the actual entry", () => { + expect(applyPath({ stack: [], index: 0 }, "/settings")).toEqual({ stack: ["/settings"], index: 0 }) }) - test("back and forward indexes stay correct after trimming", () => { - let state = history() - state = applyPath(state, "/", 3) - state = applyPath(state, "/a", 3) - state = applyPath(state, "/b", 3) - state = applyPath(state, "/c", 3) - - expect(state.stack).toEqual(["/a", "/b", "/c"]) - expect(state.index).toBe(2) - - const back = backPath(state) - expect(back?.to).toBe("/b") - expect(back?.state.index).toBe(1) - - const afterBack = applyPath(back!.state, back!.to, 3) - expect(afterBack.stack).toEqual(["/a", "/b", "/c"]) - expect(afterBack.index).toBe(1) - - const forward = forwardPath(afterBack) - expect(forward?.to).toBe("/c") - expect(forward?.state.index).toBe(2) + test("append and trim keeps max bounded", () => { + const state = ["/", "/a", "/b", "/c"].reduce((state, path) => applyPath(state, path, {}, 3), { + stack: [], + index: 0, + }) + expect(state).toEqual({ stack: ["/a", "/b", "/c"], index: 2 }) + expect(applyPath(state, "/b", -1).index).toBe(1) + }) - const afterForward = applyPath(forward!.state, forward!.to, 3) - expect(afterForward.stack).toEqual(["/a", "/b", "/c"]) - expect(afterForward.index).toBe(2) + test("replacing settings tabs keeps the forward entries", () => { + const state = { stack: ["/session", "/settings?tab=general", "/other"], index: 1 } + const next = applyPath(state, "/settings?tab=models", { replace: true }) + expect(next).toEqual({ stack: ["/session", "/settings?tab=models", "/other"], index: 1 }) + expect(applyPath(next, "/session", -1).index).toBe(0) + expect(applyPath(next, "/other", 1).index).toBe(2) }) - test("action-driven navigation does not push duplicate history entries", () => { - const state: TitlebarHistory = { - stack: ["/", "/a", "/b"], - index: 2, - action: undefined, - } + test("native traversal does not append entries", () => { + const state = { stack: ["/session", "/settings?tab=models"], index: 1 } + const back = applyPath(state, "/session", -1) + expect(back).toEqual({ ...state, index: 0 }) + expect(applyPath(back, "/settings?tab=models", 1)).toEqual(state) + }) - const back = backPath(state) - expect(back?.to).toBe("/a") + test("memory history traversal without a leave event preserves the stack", () => { + const state = { stack: ["/session", "/settings?tab=models"], index: 1 } + const back = applyPath(state, "/session") + expect(back).toEqual({ ...state, index: 0 }) + expect(applyPath(back, "/settings?tab=models")).toEqual(state) + }) - const next = applyPath(back!.state, back!.to, 10) - expect(next.stack).toEqual(["/", "/a", "/b"]) - expect(next.index).toBe(1) - expect(next.action).toBeUndefined() + test("pushing a previously visited path is not mistaken for traversal", () => { + const state = { stack: ["/session", "/settings", "/other"], index: 1 } + expect(applyPath(state, "/session", {})).toEqual({ stack: ["/session", "/settings", "/session"], index: 2 }) }) }) diff --git a/packages/app/src/shell/titlebar/history.ts b/packages/app/src/shell/titlebar/history.ts index 44dbbfa3a492..f75febb31ba6 100644 --- a/packages/app/src/shell/titlebar/history.ts +++ b/packages/app/src/shell/titlebar/history.ts @@ -1,26 +1,39 @@ export const MAX_TITLEBAR_HISTORY = 100 -export type TitlebarAction = "back" | "forward" | undefined +export type TitlebarAction = number | { replace?: boolean } export type TitlebarHistory = { stack: string[] index: number - action: TitlebarAction } -export function applyPath(state: TitlebarHistory, current: string, max = MAX_TITLEBAR_HISTORY): TitlebarHistory { - if (!state.stack.length) { - const stack = current === "/" ? ["/"] : ["/", current] - return { stack, index: stack.length - 1, action: undefined } +export function applyPath( + state: TitlebarHistory, + current: string, + action?: TitlebarAction, + max = MAX_TITLEBAR_HISTORY, +): TitlebarHistory { + if (!state.stack.length) return { stack: [current], index: 0 } + + if (typeof action === "number") { + const index = state.index + action + if (state.stack[index] === current) return { ...state, index } + return { stack: [current], index: 0 } } - const active = state.stack[state.index] - if (current === active) { - if (!state.action) return state - return { ...state, action: undefined } + if (action?.replace) { + return { ...state, stack: state.stack.map((path, index) => (index === state.index ? current : path)) } } - if (state.action) return { ...state, action: undefined } + if (current === state.stack[state.index]) return state + + // MemoryRouter history traversal does not notify useBeforeLeave. + if (!action) { + const before = state.stack.findLastIndex((path, index) => index < state.index && path === current) + const after = state.stack.findIndex((path, index) => index > state.index && path === current) + const index = before < 0 ? after : after < 0 || state.index - before <= after - state.index ? before : after + if (index >= 0) return { ...state, index } + } return pushPath(state, current, max) } @@ -28,7 +41,7 @@ export function applyPath(state: TitlebarHistory, current: string, max = MAX_TIT export function pushPath(state: TitlebarHistory, path: string, max = MAX_TITLEBAR_HISTORY): TitlebarHistory { const stack = state.stack.slice(0, state.index + 1).concat(path) const next = trimHistory(stack, stack.length - 1, max) - return { ...state, ...next, action: undefined } + return { ...state, ...next } } export function trimHistory(stack: string[], index: number, max = MAX_TITLEBAR_HISTORY) { @@ -39,19 +52,3 @@ export function trimHistory(stack: string[], index: number, max = MAX_TITLEBAR_H index: Math.max(0, index - cut), } } - -export function backPath(state: TitlebarHistory) { - if (state.index <= 0) return - const index = state.index - 1 - const to = state.stack[index] - if (!to) return - return { state: { ...state, index, action: "back" as const }, to } -} - -export function forwardPath(state: TitlebarHistory) { - if (state.index >= state.stack.length - 1) return - const index = state.index + 1 - const to = state.stack[index] - if (!to) return - return { state: { ...state, index, action: "forward" as const }, to } -} diff --git a/packages/app/src/shell/titlebar/titlebar.tsx b/packages/app/src/shell/titlebar/titlebar.tsx index 28f87dc1bb7f..31ff60eee336 100644 --- a/packages/app/src/shell/titlebar/titlebar.tsx +++ b/packages/app/src/shell/titlebar/titlebar.tsx @@ -1,7 +1,6 @@ -import { createEffect, createMemo, createResource, Match, createSignal, Show, Switch, untrack } from "solid-js" -import { createStore } from "solid-js/store" +import { createEffect, createMemo, createResource, Match, createSignal, Show, Switch } from "solid-js" import { Portal } from "solid-js/web" -import { useLocation, useNavigate } from "@solidjs/router" +import { useLocation } from "@solidjs/router" import { IconButton } from "@opencode-ai/ui/icon-button" import { Icon } from "@opencode-ai/ui/icon" import { Keybind } from "@opencode-ai/ui/keybind" @@ -12,16 +11,15 @@ import { usePlatform } from "@/runtime/platform/platform" import { useCommand } from "@/shell/commands/command" import { useLanguage } from "@/runtime/i18n/language" import { useSettings } from "@/settings/model" -import { useSettingsSurface } from "@/settings/surface" import { WindowsAppMenu } from "./windows-menu" -import { applyPath, backPath, forwardPath } from "./history" +import { useTitlebarHistory } from "./history-context" import { TitlebarTabStrip } from "@/shell/titlebar/tab-strip" import { makeEventListener } from "@solid-primitives/event-listener" import { createMediaQuery } from "@solid-primitives/media" import { readSessionTabsRemovedDetail, SESSION_TABS_REMOVED_EVENT } from "@/shell/titlebar/session-events" import { useGlobal } from "@/runtime/server/runtime" import { ServerConnection } from "@/runtime/server/registry" -import { tabKey, useTabs } from "@/shell/tabs/tabs" +import { tabHref, tabKey, useTabs } from "@/shell/tabs/tabs" import type { ComposerState } from "@/composer/persistence" import "./titlebar.css" import { newTabTooltipKeybind } from "@/shell/commands/tooltip-keybind" @@ -47,8 +45,7 @@ export function Titlebar(props: { const command = useCommand() const language = useLanguage() const settings = useSettings() - const surface = useSettingsSurface() - const navigate = useNavigate() + const history = useTitlebarHistory() const location = useLocation() const mobile = createMediaQuery("(max-width: 767px)") const bottom = createMemo(() => mobile() && settings.general.mobileTitlebarPosition() === "bottom") @@ -66,24 +63,6 @@ export function Titlebar(props: { } const windowsControlsWidth = () => `${windowsControlsBaseWidth / Math.max(titlebarZoom(), 1)}px` - const [history, setHistory] = createStore({ - stack: [] as string[], - index: 0, - action: undefined as "back" | "forward" | undefined, - }) - - const path = () => `${location.pathname}${location.search}${location.hash}` - - createEffect(() => { - const current = path() - - untrack(() => { - const next = applyPath(history, current) - if (next === history) return - setHistory(next) - }) - }) - const updateState = createMemo(() => { const installing = props.update?.installing ?? false const version = props.update?.version @@ -100,34 +79,20 @@ export function Titlebar(props: { update: updateState(), })) - const back = () => { - const next = backPath(history) - if (!next) return - setHistory(next.state) - navigate(next.to) - } - - const forward = () => { - const next = forwardPath(history) - if (!next) return - setHistory(next.state) - navigate(next.to) - } - command.register(() => [ { id: "common.goBack", title: language.t("common.goBack"), category: language.t("command.category.view"), keybind: "mod+[", - onSelect: back, + onSelect: history.back, }, { id: "common.goForward", title: language.t("common.goForward"), category: language.t("command.category.view"), keybind: "mod+]", - onSelect: forward, + onSelect: history.forward, }, ]) @@ -201,11 +166,11 @@ export function Titlebar(props: { } } - const currentTab = () => (surface.store.open ? undefined : matchRoute(layout.route())) + const currentTab = () => matchRoute(layout.route()) createEffect(() => { const route = layout.route() - if (!tabs.ready() || surface.store.open) return + if (!tabs.ready()) return const tab = currentTab() if (tab) { const current = session() @@ -263,8 +228,12 @@ export function Titlebar(props: { void tabs.newDraft({ server: activeTab.server, directory: activeTab.directory }, "", model) return } - case "home": { - const selection = layout.home.selection() + case "home": + case "settings": { + const selection = + route.type === "settings" + ? { server: location.query.server, directory: location.query.directory } + : layout.home.selection() const conn = global.servers.list().find((item) => ServerConnection.key(item) === selection.server) ?? global.servers.list()[0] @@ -273,8 +242,17 @@ export function Titlebar(props: { projects?.list().find((item) => item.worktree === selection.directory) ?? projects?.list().find((item) => item.worktree === projects.last()) ?? projects?.list()[0] - if (conn && project) { - void tabs.newDraft({ server: ServerConnection.key(conn), directory: project.worktree }, "") + const directory = + route.type === "settings" && typeof selection.directory === "string" + ? selection.directory + : project?.worktree + if (conn && directory) { + const origin = + route.type === "settings" + ? tabsStore.find((tab) => tabHref(tab) === location.query.from) + : undefined + const model = origin ? tabs.stateValue(origin, "prompt")?.model.current() : undefined + void tabs.newDraft({ server: ServerConnection.key(conn), directory }, "", model) return } } @@ -374,7 +352,6 @@ export function Titlebar(props: { forceTruncate={tabsAreOverflowing()} onOverflowChange={setTabsAreOverflowing} onNavigate={(tab, el) => { - surface.close() tabs.select(tab) el?.scrollIntoView({ behavior: "instant" }) }} @@ -417,7 +394,6 @@ export function Titlebar(props: { forceTruncate={false} onOverflowChange={setTabsAreOverflowing} onNavigate={(tab, el) => { - surface.close() tabs.select(tab) el?.scrollIntoView({ behavior: "instant", block: "nearest" }) }} diff --git a/packages/app/test-browser/settings-history.test.ts b/packages/app/test-browser/settings-history.test.ts new file mode 100644 index 000000000000..562042ca7a98 --- /dev/null +++ b/packages/app/test-browser/settings-history.test.ts @@ -0,0 +1,114 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { MemoryRouter, Route, createMemoryHistory, useIsRouting, useLocation, useNavigate } from "@solidjs/router" +import { createComponent, createEffect, createRoot } from "solid-js" +import { render } from "solid-js/web" +import { createTitlebarHistory } from "../src/shell/titlebar/history-context" + +const cleanup: (() => void)[] = [] +afterEach(() => cleanup.splice(0).forEach((dispose) => dispose())) + +function setup(initial = "/new-session?draftId=example") { + const host = document.createElement("div") + document.body.append(host) + const memory = createMemoryHistory() + memory.set({ value: initial, replace: true, scroll: false }) + const state = {} as { + history: ReturnType + navigate: ReturnType + location: ReturnType + routing: ReturnType + } + const dispose = render( + () => + createComponent(MemoryRouter, { + history: memory, + root: (props) => { + state.history = createTitlebarHistory() + state.navigate = useNavigate() + state.location = useLocation() + state.routing = useIsRouting() + return props.children + }, + get children() { + return createComponent(Route, { path: "*all", component: () => null }) + }, + }), + host, + ) + cleanup.push(() => { + dispose() + host.remove() + }) + return { + ...state, + memory, + async at(path: string) { + await new Promise((resolve) => { + createRoot((dispose) => { + createEffect(() => { + if (state.routing()) return + dispose() + resolve() + }) + }) + }) + expect(`${state.location.pathname}${state.location.search}`).toBe(path) + expect(memory.get()).toBe(path) + }, + } +} + +describe("settings memory history", () => { + test("closing and app forward traverse the same entries after replacing the settings tab", async () => { + const app = setup() + app.navigate("/settings?tab=general") + await app.at("/settings?tab=general") + app.navigate("/settings?tab=models", { replace: true }) + await app.at("/settings?tab=models") + app.history.back() + await app.at("/new-session?draftId=example") + app.history.forward() + await app.at("/settings?tab=models") + app.history.back() + await app.at("/new-session?draftId=example") + }) + + test("native back and forward retain the return destination without location state", async () => { + const app = setup() + app.navigate("/settings?tab=models") + await app.at("/settings?tab=models") + app.memory.back() + await app.at("/new-session?draftId=example") + app.memory.forward() + await app.at("/settings?tab=models") + expect(app.location.state).toBeUndefined() + app.history.back() + await app.at("/new-session?draftId=example") + app.history.forward() + await app.at("/settings?tab=models") + }) + + test("navigating after going back discards the actual forward history", async () => { + const app = setup() + app.navigate("/settings") + await app.at("/settings") + app.history.back() + await app.at("/new-session?draftId=example") + app.navigate("/new-session?draftId=other") + await app.at("/new-session?draftId=other") + app.history.forward() + await app.at("/new-session?draftId=other") + app.memory.back() + await app.at("/new-session?draftId=example") + }) + + test("direct settings entry and a restarted memory router fall back home", async () => { + const app = setup("/settings?tab=models") + app.history.back() + await app.at("/") + app.history.forward() + await app.at("/") + app.memory.back() + await app.at("/") + }) +}) diff --git a/packages/desktop/src/renderer/window/route-storage.test.ts b/packages/desktop/src/renderer/window/route-storage.test.ts index 29d08c549736..23d632acb7e6 100644 --- a/packages/desktop/src/renderer/window/route-storage.test.ts +++ b/packages/desktop/src/renderer/window/route-storage.test.ts @@ -4,6 +4,10 @@ import { acceptedLastActiveUrl } from "./route-storage" describe("desktop last active route", () => { test("restores current desktop routes", () => { expect(acceptedLastActiveUrl("/")).toBe("/") + expect(acceptedLastActiveUrl("/settings")).toBe("/settings") + expect(acceptedLastActiveUrl("/settings?tab=models&server=local&directory=%2Fproject")).toBe( + "/settings?tab=models&server=local&directory=%2Fproject", + ) expect(acceptedLastActiveUrl("/new-session?directory=C%3A%5Cwork#draft")).toBe( "/new-session?directory=C%3A%5Cwork#draft", ) @@ -14,7 +18,7 @@ describe("desktop last active route", () => { test("falls back for invalid saved routes", () => { expect(acceptedLastActiveUrl(undefined)).toBe("/") - expect(acceptedLastActiveUrl("/settings")).toBe("/") + expect(acceptedLastActiveUrl("/settings/extra")).toBe("/") expect(acceptedLastActiveUrl("/new-session/extra")).toBe("/") expect(acceptedLastActiveUrl("/server/sidecar/session/abc/extra")).toBe("/") }) diff --git a/packages/desktop/src/renderer/window/route-storage.ts b/packages/desktop/src/renderer/window/route-storage.ts index 63595c8576ef..2ead2ee574ad 100644 --- a/packages/desktop/src/renderer/window/route-storage.ts +++ b/packages/desktop/src/renderer/window/route-storage.ts @@ -17,7 +17,7 @@ export function setLastActiveUrl(windowID: string, value: string) { export function acceptedLastActiveUrl(value: string | null | undefined) { if (value === "/") return value const path = value?.split(/[?#]/, 1)[0] - if (path === "/new-session") return value ?? "/" + if (path === "/new-session" || path === "/settings") return value ?? "/" if (/^\/server\/[^/]+\/session\/[^/]+$/.test(path ?? "")) return value ?? "/" return "/" }