From 0f9db0624384f3d4bdd5020b4bcdae01c40a02e7 Mon Sep 17 00:00:00 2001 From: yuxino Date: Mon, 21 Sep 2026 22:34:02 +0800 Subject: [PATCH] feat(settings): start each destination at the top Avoid carrying a scroll offset between unrelated settings categories. Reset on destination changes before search anchors run, while retaining the position on same-category updates and repeated selection. Closes #812 --- .../src/features/settings/SettingsPage.tsx | 11 +- docs/spec/04-ux/06-settings-ia.md | 5 + docs/spec/06-delivery/04-e2e-test-plan.md | 18 +++ docs/zh-CN/spec/04-ux/06-settings-ia.md | 3 + .../spec/06-delivery/04-e2e-test-plan.md | 15 +++ package.json | 1 + scripts/e2e-settings-scroll.mjs | 58 ++++++++++ scripts/e2e/settings-scroll.jsx | 108 ++++++++++++++++++ 8 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 scripts/e2e-settings-scroll.mjs create mode 100644 scripts/e2e/settings-scroll.jsx diff --git a/apps/desktop/src/features/settings/SettingsPage.tsx b/apps/desktop/src/features/settings/SettingsPage.tsx index d426bb1e49..ce77c32e78 100644 --- a/apps/desktop/src/features/settings/SettingsPage.tsx +++ b/apps/desktop/src/features/settings/SettingsPage.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState, type ReactNode } from "react"; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import type { AppSettings, @@ -95,6 +95,13 @@ export function SettingsPage() { const [settingsRecoveryFailed, setSettingsRecoveryFailed] = useState(false); const [extensions, setExtensions] = useState([]); const [activeExtension, setActiveExtension] = useState(null); + const contentRef = useRef(null); + const destination = activeExtension ? `extension:${activeExtension.ref}` : `builtin:${tab}`; + + useLayoutEffect(() => { + // Reset before paint and before the search-anchor effect positions its row. + if (contentRef.current) contentRef.current.scrollTop = 0; + }, [destination]); useEffect(() => { const refresh = () => void api.listPluginScenicThemesDestinations().then(setExtensions, () => setExtensions([])); @@ -335,7 +342,7 @@ export function SettingsPage() { -
+

diff --git a/docs/spec/04-ux/06-settings-ia.md b/docs/spec/04-ux/06-settings-ia.md index d043475ad7..dfd077b15a 100644 --- a/docs/spec/04-ux/06-settings-ia.md +++ b/docs/spec/04-ux/06-settings-ia.md @@ -4,6 +4,11 @@ Settings is a **full-window page** that replaces the app sidebar + main chrome (Codex electron behavior): +- Switching to a different Settings destination starts the content pane at the + top, including plugin destinations. Re-selecting the current destination or + updating settings in place preserves the current scroll position. Global + search deep links still scroll to their target row after the destination + changes; consuming the search anchor does not reset the pane again. - Settings remains usable when an unrelated startup read fails: a successfully loaded settings snapshot is retained independently from the remaining bootstrap data. If the settings read itself is unavailable, the content pane diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index 7359c94bd0..aa327ef9d3 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -14347,3 +14347,21 @@ the latest destination. These assertions measure work counts, not device FPS. - **Status:** Automated by `node --experimental-strip-types scripts/e2e-scheduled-workspace.mjs`, using production Electron dispatch and real Rust/stdio/SQLite. Only external inference is replaced with an observer. + +### E2E-SETTINGS-destination-scroll-reset + +- Open Settings → AI and scroll midway down. Select Shortcuts: its title and + first settings appear at the top. Scroll and return to AI: it starts at top. +- Re-select the active destination and update settings without navigating: + the content keeps its scroll position. +- Repeat for built-in → plugin, plugin → plugin, and plugin → the previously + selected built-in destination. Re-selecting a plugin keeps its position. +- Follow a global search setting anchor into AI from another destination and + within AI: the target row is visible, and consuming the anchor keeps that + position. +- Run in light and dark themes. +- Automated coverage: `pnpm test:e2e:settings-scroll` mounts the production + SettingsPage, store, translations, and built CSS in isolated Electron. Only + preload data is stubbed; search navigation uses SearchDialog's public store + entry points. This covers renderer interaction, not host persistence or the + full global-search dialog. diff --git a/docs/zh-CN/spec/04-ux/06-settings-ia.md b/docs/zh-CN/spec/04-ux/06-settings-ia.md index 44bead94d6..1172297ca0 100644 --- a/docs/zh-CN/spec/04-ux/06-settings-ia.md +++ b/docs/zh-CN/spec/04-ux/06-settings-ia.md @@ -7,6 +7,9 @@ 设置是一个**全窗口页面**，取代了应用程序侧边栏+主镶边（Codex 电子行为）： +- 切换到不同的设置分类时，内容区回到顶部，包括插件提供的分类。再次点击 + 当前分类或在当前页更新设置时，保留滚动位置。通过全局搜索跳转时，仍在 + 分类切换后定位到目标设置项；清除已消费的搜索锚点不会再次重置滚动位置。 - 左侧设置导航宽 **275px**，与主侧栏共享 `sidebar-surface` 材质：macOS 使用原生 vibrancy 加相同 tint/sheen，Windows/Linux 使用不透明 `--ds-bg-sidebar`，并共享可选背景图。 macOS 下设置外壳透明，右侧内容区与顶部条仍不透明。只有内容区内部的入场包装播放路由入场（仅透明度，不位移）， diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index d47163dc04..10f869d31d 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -8513,3 +8513,18 @@ the latest destination. These assertions measure work counts, not device FPS. `providers::tests::a_stored_array_survives_an_entry_that_lost_a_field`）； 宿主 RPC 路径由 `scripts/e2e-smoke.mjs` 覆盖提供商的创建与列举，但没有套件 驱动手工编辑的 `config_json`。 + +### E2E-SETTINGS-destination-scroll-reset + +- 打开设置 → AI，滚动到中间，再选择快捷键：标题和首项从顶部显示。滚动后 + 返回 AI，该页也从顶部显示。 +- 再次选择当前分类，或不离开当前页更新设置，保留内容区滚动位置。 +- 覆盖内置分类 → 插件、插件 → 插件、插件 → 先前选择的内置分类；再次 + 选择当前插件分类时保留位置。 +- 从其他分类及 AI 当前页通过全局搜索设置锚点进入 AI，目标项可见，消费 + 锚点后保持定位。 +- 在明暗两种主题下运行。 +- 自动化覆盖：`pnpm test:e2e:settings-scroll` 在隔离 Electron 中挂载真实 + SettingsPage、store、翻译和构建后的 CSS。仅 preload 数据使用 fixture； + 搜索导航调用 SearchDialog 使用的公开 store 入口。该测试覆盖渲染层交互， + 不覆盖 host 持久化或完整全局搜索弹窗。 diff --git a/package.json b/package.json index 4bb7841783..4700e89680 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "test:e2e:provider-api-style": "node scripts/e2e-provider-api-style.mjs", "test:e2e:oauth-retry": "node --test apps/desktop/test/anthropic-oauth-retry.test.mjs", "test:e2e:boot": "node scripts/e2e-electron-boot.mjs", + "test:e2e:settings-scroll": "node scripts/e2e-settings-scroll.mjs", "test:e2e:layout": "node scripts/e2e-three-column-layout.mjs", "test:e2e:window-controls": "node scripts/e2e-window-controls.mjs", "test:e2e:supervision": "node scripts/e2e-supervision.mjs", diff --git a/scripts/e2e-settings-scroll.mjs b/scripts/e2e-settings-scroll.mjs new file mode 100644 index 0000000000..2ef3631ffd --- /dev/null +++ b/scripts/e2e-settings-scroll.mjs @@ -0,0 +1,58 @@ +#!/usr/bin/env node +/** Settings navigation in isolated Electron, with the production component/store/CSS. */ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { createRequire } from "node:module"; +import { cp, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { repositoryRoot, resolveElectronBinary } from "./e2e/boot.mjs"; + +const root = repositoryRoot(); +const { build } = createRequire(join(root, "packages/agent-runtime/package.json"))("esbuild"); +const temp = await mkdtemp(join(tmpdir(), "pi-settings-scroll-")); +try { + await build({ entryPoints: [join(root, "scripts/e2e/settings-scroll.jsx")], + outfile: join(temp, "renderer.js"), bundle: true, platform: "browser", format: "esm", jsx: "automatic", + define: { "process.env.NODE_ENV": '"production"', "import.meta.env.DEV": "false" }, + alias: { react: join(root, "apps/desktop/node_modules/react"), + "react-dom": join(root, "apps/desktop/node_modules/react-dom"), + i18next: join(root, "apps/desktop/node_modules/i18next"), + "react-i18next": join(root, "apps/desktop/node_modules/react-i18next") }, + nodePaths: [join(root, "apps/desktop/node_modules")], + }); + const renderer = join(root, "apps/desktop/out/renderer"); + const html = await readFile(join(renderer, "index.html"), "utf8"); + const css = [...html.matchAll(/href="([^" ]+\.css)"/g)].map((match) => match[1]); + assert(css.length, "Run pnpm build:js before this test"); + await cp(join(renderer, "assets"), join(temp, "assets"), { recursive: true }); + await writeFile(join(temp, "index.html"), `${css.map((path) => ``).join("")}
`); + await writeFile(join(temp, "main.cjs"), ` +const { app, BrowserWindow } = require("electron"); +const path = require("node:path"); +app.setPath("userData", path.join(__dirname, "profile")); +app.whenReady().then(async () => { + const win = new BrowserWindow({ show: false, width: 1000, height: 720, + webPreferences: { sandbox: true, contextIsolation: true, nodeIntegration: false, backgroundThrottling: false } }); + win.webContents.on("console-message", (event) => console.error(event.message)); + try { + await win.loadFile(path.join(__dirname, "index.html")); + const result = await win.webContents.executeJavaScript("window.settingsScrollProbe()"); + console.log("SETTINGS_SCROLL " + JSON.stringify(result)); + app.exit(0); + } catch (error) { console.error(error); app.exit(1); } +}); +`); + const env = { ...process.env }; delete env.ELECTRON_RUN_AS_NODE; + const child = spawn(resolveElectronBinary(root).electronBinary, [join(temp, "main.cjs")], { env, stdio: ["ignore", "pipe", "pipe"] }); + let output = ""; + for (const stream of [child.stdout, child.stderr]) stream.on("data", (chunk) => { output += chunk; }); + const timer = setTimeout(() => child.kill("SIGKILL"), 30_000); + let code; + try { code = await new Promise((resolve, reject) => { child.once("error", reject); child.once("close", resolve); }); } + finally { clearTimeout(timer); } + assert.equal(code, 0, output); + const result = output.split(/\r?\n/).find((line) => line.startsWith("SETTINGS_SCROLL ")); + assert(result, output); + console.log(result); +} finally { await rm(temp, { recursive: true, force: true }); } diff --git a/scripts/e2e/settings-scroll.jsx b/scripts/e2e/settings-scroll.jsx new file mode 100644 index 0000000000..25ba568e5c --- /dev/null +++ b/scripts/e2e/settings-scroll.jsx @@ -0,0 +1,108 @@ +// Mounted production SettingsPage; only the preload boundary uses fixture data. +import { createRoot } from "react-dom/client"; +import { flushSync } from "react-dom"; +import i18n from "i18next"; +import { initReactI18next } from "react-i18next"; +import { catalogs, flattenCatalog } from "@pi-desktop/i18n"; +import { IPC } from "@pi-desktop/shared"; +import { SettingsPage } from "../../apps/desktop/src/features/settings/SettingsPage"; +import { useAppStore } from "../../apps/desktop/src/stores/app-store"; + +const destinations = ["A", "B"].map((id) => ({ + ref: `fixture:${id}`, pluginId: "fixture", label: `Themes ${id}`, keywords: [], + description: "Fixture theme collection", themes: Array.from({ length: 12 }, (_, index) => ({ + themeId: `plugin:fixture:${id}${index}`, label: `Theme ${index}`, description: "Preview", + previewUrl: "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg'/%3E", blur: 6, + })), +})); +let settings = { defaultMode: "agent", theme: "light", language: "en", enterToSend: true }; +window.piDesktop = { + platform: "darwin", on: () => () => {}, + async invoke(channel, input) { + let data; + switch (channel) { + case IPC.invoke.pluginScenicThemesDestinations: data = destinations; break; + case IPC.invoke.settingsGet: data = settings; break; + case IPC.invoke.settingsSet: settings = input; data = settings; break; + case IPC.invoke.commandShellList: data = { choices: [], effective: null }; break; + default: throw new Error(`Unexpected fixture IPC: ${channel}`); + } + return { ok: true, data }; + }, +}; +await i18n.use(initReactI18next).init({ lng: "en", fallbackLng: "en", keySeparator: false, + resources: { en: { translation: flattenCatalog(catalogs.en) } }, interpolation: { escapeValue: false } }); +useAppStore.setState({ settings, settingsTab: "ai", page: "settings" }); +const root = createRoot(document.getElementById("root")); +flushSync(() => root.render()); +const frame = () => new Promise(requestAnimationFrame); +async function settle() { await frame(); await frame(); } +function assert(value, message) { if (!value) throw new Error(message); } +const pane = () => document.querySelector(".settings-content"); +async function select(label) { + const button = [...document.querySelectorAll(".settings-nav-item")] + .find((node) => node.textContent.trim() === label); + assert(button, `Missing destination: ${label}`); + flushSync(() => button.click()); + await settle(); +} +async function scroll() { + pane().scrollTop = 220; + await settle(); + assert(pane().scrollTop > 0, "Destination must be scrollable for this check"); + return pane().scrollTop; +} +window.settingsScrollProbe = async () => { + await settle(); + const checks = []; + for (const theme of ["light", "dark"]) { + document.documentElement.dataset.theme = theme; + await select("AI"); + const position = await scroll(); + await select("AI"); + assert(pane().scrollTop === position, "Re-selecting AI must retain scroll"); + flushSync(() => useAppStore.setState({ settings: { ...settings, enterToSend: false } })); + await settle(); + assert(pane().scrollTop === position, "Settings updates must retain scroll"); + await select("Shortcuts"); + assert(pane().scrollTop === 0, "AI → Shortcuts must start at top"); + await scroll(); + await select("AI"); + assert(pane().scrollTop === 0, "Returning to AI must start at top"); + await scroll(); + await select("Themes A"); + assert(pane().scrollTop === 0, "Built-in → extension must start at top"); + const extensionPosition = await scroll(); + await select("Themes A"); + assert(pane().scrollTop === extensionPosition, "Re-selecting extension must retain scroll"); + await select("Themes B"); + assert(pane().scrollTop === 0, "Extension → extension must start at top"); + await scroll(); + await select("AI"); + assert(pane().scrollTop === 0, "Extension → unchanged built-in tab must start at top"); + // The same public store entry points used by SearchDialog.openSettingsHit. + await select("Shortcuts"); + await scroll(); + flushSync(() => { + useAppStore.getState().setSettingsAnchor("settings.enterToSend"); + useAppStore.getState().setSettingsTab("ai"); + }); + await settle(); + const target = document.querySelector(".settings-anchor-flash"); + assert(target && pane().scrollTop > 0, "Search anchor must win over destination reset"); + const bounds = target.getBoundingClientRect(); + const viewport = pane().getBoundingClientRect(); + assert(bounds.top >= viewport.top && bounds.bottom <= viewport.bottom, + "Search target must be visible"); + assert(useAppStore.getState().settingsAnchor === null, "Search anchor must be consumed"); + const anchoredPosition = pane().scrollTop; + await settle(); + assert(pane().scrollTop === anchoredPosition, "Consuming anchor must not reset scroll"); + pane().scrollTop = 0; + flushSync(() => useAppStore.getState().setSettingsAnchor("settings.enterToSend")); + await settle(); + assert(pane().scrollTop > 0, "Search within the active tab must still locate its row"); + checks.push({ theme, ok: true }); + } + return { ok: true, checks }; +};