From 662089dd76d81a87b9b9bf24eb1f39a453529cd2 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 11:02:11 +0800 Subject: [PATCH 1/8] fix(client): theme the scrollbars and reserve the workspace list gutter design-platform.css declared four --dsw-alias-scrollbar-* tokens in both palettes that no rule read, so every scrolling region rendered the user agent's own scrollbar and the dark theme showed a light native bar against dark surfaces. The symptom that surfaced the gap was in the sidebar: the workspace browser's session list is its only scrolling region, and each row's trailing content (the relative timestamp, and the hover action buttons that replace it) is `flex: none` flush against the row's 8px right padding, so an overlaid scrollbar painted on top of the timestamp. ui-theme/styles/scrollbar.css becomes the sole consumer of the four tokens, imported by the web shell's base.css after design-platform.css because it reads that sheet's tokens. The rules sit on `body`, not `html`: the alias tokens are declared on `body`, custom properties inherit only downward, and from `html` they resolve to the guaranteed-invalid value with scrollbar-color computing to `auto`. scrollbar-width and scrollbar-color are declared on `body, body *` rather than inherited, because inheritance would carry the color already substituted at `body` and an elevated surface could not retint its own thumb; scrollbar-width does not inherit at all. Both the standard properties and the ::-webkit-scrollbar pseudo-elements read one indirection pair bound to the l1 tokens, so an elevated surface rebinds that pair to the l2 tokens once and retints both renderings. The command popup, slash menu, model-select panel, and settings panel do so, which gives the l2 tokens their first consumers. WorkspaceBrowser's `.list` declares scrollbar-gutter: stable, keeping the bar beside the rows. `stable` rather than `auto` so the reservation holds when the list is short enough not to scroll: expanding a workspace group would otherwise shift every row sideways at the moment it starts scrolling. --- ...d-scrollbars-and-reserved-gutter.i18n.yaml | 6 + ...8-themed-scrollbars-and-reserved-gutter.md | 57 ++++ ...hemed-scrollbars-and-reserved-gutter.zh.md | 57 ++++ apps/web/tests/sidebar-scrollbar.e2e.ts | 202 ++++++++++++ .../src/client/PopupSelectView.module.css | 4 + .../src/client/ModelSelect.module.css | 7 + .../src/client/SettingsRoot.module.css | 7 + .../ui-slash/src/client/MenuView.module.css | 4 + packages/client/ui-theme/README.i18n.yaml | 4 +- packages/client/ui-theme/README.md | 4 + packages/client/ui-theme/README.zh.md | 4 + .../client/ui-theme/src/styles/scrollbar.css | 66 ++++ .../ui-theme/tests/scrollbar-styles.spec.ts | 311 ++++++++++++++++++ .../src/client/WorkspaceBrowser.module.css | 7 + .../ui-workspace/tests/browser-styles.spec.ts | 48 +++ packages/client/web/src/base.css | 6 +- packages/client/web/tests/base-styles.spec.ts | 58 ++++ 17 files changed, 848 insertions(+), 4 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md create mode 100644 .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md create mode 100644 apps/web/tests/sidebar-scrollbar.e2e.ts create mode 100644 packages/client/ui-theme/src/styles/scrollbar.css create mode 100644 packages/client/ui-theme/tests/scrollbar-styles.spec.ts create mode 100644 packages/client/ui-workspace/tests/browser-styles.spec.ts create mode 100644 packages/client/web/tests/base-styles.spec.ts diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml new file mode 100644 index 0000000000..5d9b727b2e --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +2026-07-28-themed-scrollbars-and-reserved-gutter.md: 29aad9976610b02b42e0c69222504a84188e34d7 +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: 2c5c4eefee88680df35d1a069e6ab5de7884144f diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md new file mode 100644 index 0000000000..29aad99766 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -0,0 +1,57 @@ +# Agent Note: The scrollbar tokens get their consumer, and the workspace list reserves its gutter + +Status: implemented + +English | [中文](2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md) + +## Problem + +`design-platform.css` declares four `--dsw-alias-scrollbar-*` tokens (`bg-l1`, `bg-l2`, `hover-l1`, `hover-l2`) in both palettes, and no rule anywhere in the client read them. A defined token with no consumer is not a theme: every scrolling region rendered the user agent's own scrollbar, which knows nothing about the palette, so the dark theme showed a light native bar against dark surfaces. + +The visible symptom that surfaced the gap was elsewhere. The workspace browser's session list (`.list` in `WorkspaceBrowser.module.css`) is the sidebar's only scrolling region, and each row's trailing content sits flush against the row's 8px right padding — `.time` in `rows/Rows.module.css` is `flex: none`, as are the action buttons that replace it on hover. An overlaid scrollbar therefore painted on top of the relative timestamp. Reserving space in that one list would have left the bar itself unthemed, so the two halves are one change. + +## Decision + +`packages/client/ui-theme/src/styles/scrollbar.css` is the sole consumer of the four tokens, and the fifth ui-theme sheet in the shell's import chain (`packages/client/web/src/base.css`). It follows `design-platform.css` there because it reads that sheet's tokens. + +The rules sit on `body`, not `html`. `design-platform.css` declares the `--dsw-alias-*` tokens on `body`, with the dark overrides on `body[data-ds-dark-theme]`, and custom properties inherit only downward; an `html` rule resolves them to the guaranteed-invalid value, at which point `scrollbar-color` computes to `auto` and no theming happens at all. + +`scrollbar-width` and `scrollbar-color` are declared on `body, body *` rather than once at the top. Inheritance would pass down the color already substituted at `body`, so a descendant rebinding the indirection could not change its own scrollbar; re-declaring makes each element substitute the variable as it sees it. `scrollbar-width` is not an inherited property in the first place, so it needs the per-element declaration regardless. The `::-webkit-scrollbar*` pseudo-elements are likewise not inherited and are matched unscoped. + +Both halves read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Four surfaces rebind today: the command popup, the slash menu, the model-select panel, and the settings panel. The last two declare it on the elevated panel rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. + +The track and the corner stay transparent, so the thumb reads against whatever surface scrolls under it; only the thumb and its hover state carry a token color. + +`.list` declares `scrollbar-gutter: stable`, which keeps the bar beside the rows instead of on top of them. `stable` rather than `auto` because `auto` reserves the gutter only while the list actually overflows: expanding a workspace group would then shift every row horizontally at the moment it starts scrolling. The reservation is unconditional and the rows never move. + +## Alternatives considered + +**Per-module `::-webkit-scrollbar` rules in each scrolling component sheet.** Rejected: the client has thirteen scrolling containers across nine packages, every one would carry the same block, and the fourteenth would ship unthemed with nothing failing. A skin driven by design tokens belongs in the package that owns the tokens. + +**An opt-in utility class that each scroll container adds.** Same duplication removed, but the failure mode stays: a new scroll container is themed only if its author remembers the class, and the omission is invisible in review. The `body, body *` form has no opt-in step to forget; a container that genuinely wants a different bar overrides the indirection, which is the same mechanism elevated surfaces use. + +**Bind the properties on `html`.** The natural place for a document-wide skin, and it fails measurably: with the rule on `html` a scroll container computes `scrollbar-color: auto` in chromium, because the alias tokens are not in scope there. + +**Declare the properties once and let them inherit.** Fewer matched elements, and it breaks the rebinding contract — inheritance carries the substituted color, not the variable reference, so an elevated surface could not retint its own scrollbar. It is also incomplete on its own terms, since `scrollbar-width` does not inherit. + +**Pad the rows instead of reserving the gutter (extra right padding on `.list`, or moving `.time` inward).** Rejected: padding applies whether or not a bar is present, so it costs horizontal room in the common short-list case, and it fixes exactly one container while leaving every other scrolling region's content under its bar. + +**`scrollbar-gutter: auto` on `.list`.** The reservation appears when the list overflows, which is when the bar exists. Rejected because the sidebar's lists grow and shrink as groups expand, so the reservation would appear and disappear under the user's cursor and shift the rows with it. + +## Consequences + +- Every scroll container in the client draws the themed thumb: `rgb(229, 229, 229)` on a light base surface, `rgb(60, 60, 61)` on a dark one, and `rgb(84, 85, 87)` for a dark elevated surface that rebinds to the l2 pair. +- The two renderings are separately specified, so a change to the thumb's geometry or hover behavior has to be made twice — once in `scrollbar-width`/`scrollbar-color`, once in the pseudo-elements. Routing both through the indirection pair confines that duplication to the properties Firefox and WebKit do not share. +- `body *` matches every element, for two properties whose effect the user agent already limits to elements that actually scroll. The cost is a broad selector; the alternative was a rebinding contract that does not work. +- The workspace list is permanently narrower by the reserved band, at every list length. That is the trade the fix buys: stable row geometry instead of a timestamp that is legible only while the list is short. +- There is no track token in the palette, so a design that later wants an opaque track needs a new alias token rather than a literal color in this sheet. + +## Testing + +Three unit specs read the CSS text on disk. `ui-theme/tests/scrollbar-styles.spec.ts` scans the scrollbar token set out of `design-platform.css` rather than hardcoding it, so adding, renaming, or dropping a token moves the assertions with it, and checks that every token has a consumer and that each elevated surface rebinds a complete pair. `web/tests/base-styles.spec.ts` pins the import order and the existence of every sheet `base.css` names. `ui-workspace/tests/browser-styles.spec.ts` pins the gutter reservation on `.list`. + +`apps/web/tests/sidebar-scrollbar.e2e.ts` covers the two facts only a real engine reports: the reserved band width, and the substituted `scrollbar-color`. It needs no model calls — the list only has to overflow — so it seeds cold sessions from an existing committed fixture read-only. + +Confirmed in headless chromium on the built client by reading computed values, which is what distinguishes a working token chain from a syntactically valid one: a scroll container computes the l1 thumb color in each palette, and a container that rebinds the indirection computes the l2 color, proving the rebind reaches the computed value rather than only the custom property. + +Headless chromium draws overlay scrollbars, so a reserved gutter there does not shrink `clientWidth`. The reservation shows up as a non-zero `offsetWidth - clientWidth` band on the list; client-area geometry alone does not demonstrate it, and an assertion comparing the time element's right edge against the client-area edge holds with and without the reservation, so it would pass or fail on the platform's scrollbar style rather than on the declaration under test. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md new file mode 100644 index 0000000000..2c5c4eefee --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -0,0 +1,57 @@ +# Agent Note: 滚动条 token 有了消费方，工作区列表预留出滚动条空位 + +Status: implemented + +[English](2026-07-28-themed-scrollbars-and-reserved-gutter.md) | 中文 + +## 问题 + +`design-platform.css` 在亮色与暗色两套调色板中都声明了四个 `--dsw-alias-scrollbar-*` token（`bg-l1`、`bg-l2`、`hover-l1`、`hover-l2`），而客户端里没有任何一条规则读取它们。定义了却无人消费的 token 构不成主题：所有滚动区域渲染的都是浏览器自带的滚动条，它对调色板一无所知，因此暗色主题下暗色表面上出现的是一条亮色的原生滚动条。 + +暴露这一缺口的可见症状出在别处。工作区浏览器的会话列表（`WorkspaceBrowser.module.css` 中的 `.list`）是侧边栏里唯一的滚动区域，而每一行的尾部内容都紧贴该行 8px 的右内边距——`rows/Rows.module.css` 中的 `.time` 取 `flex: none`，hover 时取代它的操作按钮也是如此。于是覆盖式滚动条会画在相对时间戳之上。只在这一个列表里预留空间，滚动条本身仍然没有主题，因此两部分合为一次变更。 + +## 决策 + +`packages/client/ui-theme/src/styles/scrollbar.css` 是这四个 token 的唯一消费方，也是壳的导入链（`packages/client/web/src/base.css`）中第五张 ui-theme 样式表。它排在 `design-platform.css` 之后，因为它读取那张样式表的 token。 + +规则挂在 `body` 上，而非 `html`。`design-platform.css` 在 `body` 上声明 `--dsw-alias-*` token，暗色覆盖挂在 `body[data-ds-dark-theme]` 上，而自定义属性只向下继承；挂在 `html` 上的规则会把它们解析为 guaranteed-invalid 值，此时 `scrollbar-color` 计算为 `auto`，主题完全不起作用。 + +`scrollbar-width` 与 `scrollbar-color` 声明在 `body, body *` 上，而不是只在顶层声明一次。继承传下去的是已经在 `body` 处代入完成的颜色值，因此后代元素重新绑定这层间接变量也无法改变自己的滚动条；逐元素重新声明使每个元素按它自己看到的取值代入变量。`scrollbar-width` 本身就不是可继承属性，无论如何都需要逐元素声明。`::-webkit-scrollbar*` 伪元素同样不继承，因此以不加限定的选择器匹配。 + +两侧都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有四处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板与设置面板。后两者把声明写在抬升面板上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 + +轨道与两条滚动条相交的角落保持透明，因此滑块是以其下滚动的任何表面为背景被看到；只有滑块及其 hover 状态带 token 颜色。 + +`.list` 声明 `scrollbar-gutter: stable`，使滚动条位于行的旁边而非行的上方。取 `stable` 而非 `auto`，因为 `auto` 只在列表确实溢出时才预留空位：那样展开一个工作区分组时，所有行会在列表开始滚动的那一刻发生水平位移。`stable` 的预留是无条件的，行不会移动。 + +## 曾考虑的替代方案 + +**在每个滚动组件的样式表里各写一份 `::-webkit-scrollbar` 规则。** 之所以否决：客户端共有分布在九个包中的十三个滚动容器，每一个都要带上同一段规则，而第十四个会在没有任何门禁报错的情况下漏掉主题。由设计 token 驱动的皮肤应当归属于拥有这些 token 的包。 + +**提供一个工具类，由各滚动容器自行加上。** 重复同样被消除，但失败方式依旧存在：新的滚动容器只有在作者记得加类名时才有主题，而遗漏在评审中看不出来。`body, body *` 这种写法没有需要记住的启用步骤；确实想要不同滚动条的容器可以覆盖间接变量，这与抬升表面使用的机制相同。 + +**把这两个属性绑定在 `html` 上。** 这是文档级皮肤最自然的落点，而它的失败是可测量的：规则挂在 `html` 上时，chromium 中滚动容器计算出的 `scrollbar-color` 为 `auto`，因为别名 token 在那个作用域内不存在。 + +**只声明一次，靠继承下传。** 匹配的元素更少，但它破坏重新绑定契约——继承携带的是代入后的颜色，而不是变量引用，因此抬升表面无法给自己的滚动条换色。它本身也不完整，因为 `scrollbar-width` 不继承。 + +**改用内边距而不是预留空位（给 `.list` 加右内边距，或把 `.time` 向内移）。** 之所以否决：内边距无论滚动条是否存在都生效，因此在常见的短列表情形下白白占用横向空间；而且它只修好一个容器，其余每个滚动区域的内容仍然压在滚动条之下。 + +**给 `.list` 用 `scrollbar-gutter: auto`。** 空位在列表溢出时出现，也就是滚动条存在的时候。之所以否决：侧边栏的列表会随分组展开与收起而伸缩，因此空位会在用户光标之下出现又消失，并带动行一起位移。 + +## 后果 + +- 客户端的每个滚动容器都绘制带主题的滑块：亮色基础表面为 `rgb(229, 229, 229)`，暗色基础表面为 `rgb(60, 60, 61)`，重新绑定到 l2 的暗色抬升表面为 `rgb(84, 85, 87)`。 +- 两种渲染分别指定，因此改动滑块的几何或 hover 行为需要改两处：一处在 `scrollbar-width`／`scrollbar-color`，一处在伪元素。让两者都经由这组间接变量，把这份重复限制在 Firefox 与 WebKit 不共用的那些属性上。 +- `body *` 匹配所有元素，涉及的两个属性其效果本就被浏览器限制在实际会滚动的元素上。代价是一个覆盖面很宽的选择器；另一种选择是一个不生效的重新绑定契约。 +- 工作区列表在任何列表长度下都永久少了预留空位那一条宽度。这正是该修复换来的代价：以稳定的行几何，换掉只在列表较短时才可读的时间戳。 +- 调色板中没有轨道 token，因此日后若设计需要不透明轨道，要新增一个别名 token，而不是在这张样式表里写字面颜色。 + +## 测试 + +三份单元测试读取磁盘上的 CSS 文本。`ui-theme/tests/scrollbar-styles.spec.ts` 从 `design-platform.css` 中扫描出滚动条 token 集合，而不是把它写死，因此新增、重命名或删除 token 时断言会随之变化；它检查每个 token 都有消费方，且每处抬升表面重新绑定的都是完整的一对。`web/tests/base-styles.spec.ts` 锁定导入顺序，以及 `base.css` 列出的每张样式表确实存在。`ui-workspace/tests/browser-styles.spec.ts` 锁定 `.list` 上的空位预留。 + +`apps/web/tests/sidebar-scrollbar.e2e.ts` 覆盖只有真实渲染引擎才能报告的两个事实：预留条带的宽度，以及代入后的 `scrollbar-color`。它不需要任何模型调用——列表只要溢出即可——因此以只读方式复用一份既有的已提交 fixture（测试前置数据）来铺入冷会话。 + +在构建产物客户端上于 headless chromium 中读取计算值确认，这正是区分「token 链真正生效」与「语法合法」的手段：滚动容器在两套调色板下分别计算出 l1 的滑块颜色，而重新绑定间接变量的容器计算出 l2 的颜色，证明重新绑定作用到了计算值，而不只是作用到自定义属性上。 + +headless chromium 绘制的是覆盖式滚动条，因此其中预留空位不会缩小 `clientWidth`。该预留表现为列表上非零的 `offsetWidth - clientWidth` 条带；仅凭内容区几何无法证明它，而把时间元素右边缘与内容区右边缘做比较的断言，在有无预留的两种状态下都成立，因此它的通过或失败取决于平台的滚动条样式，而不是取决于被测的那条声明。 diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts new file mode 100644 index 0000000000..f54c0f9b2b --- /dev/null +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -0,0 +1,202 @@ +// Web e2e scenario: the sidebar session list's scrollbar as the browser +// actually lays it out — the observable half of the themed-scrollbar change +// (packages/client/ui-theme/src/styles/scrollbar.css plus the +// `scrollbar-gutter: stable` reservation on WorkspaceBrowser's `.list`). The +// ui-theme/ui-workspace unit specs read the CSS text; only a real engine +// reports the reserved gutter width and the substituted `scrollbar-color`, so +// those two facts live here. +// +// Zero model calls: the list only has to overflow, so the scenario seeds many +// cold sessions from another spec's committed fixture (seeded-history's +// seed.jsonl, reused read-only — this spec needs row count, not new recorded +// content) and never launches a replay row. A stray stream would fail loud +// with NO_ADAPTER. +// +// Headless-chromium caveat, load-bearing for what is asserted below: chromium +// paints an OVERLAY scrollbar that consumes no layout width. Comparing the +// time element's right edge against the list's client-area right edge +// therefore holds with and without the reservation and proves nothing; the +// reserved band width is the only layout signal that distinguishes the two +// states. See the assertions for which one is the control. +import { readFile } from 'node:fs/promises' +import { fileURLToPath } from 'node:url' +import type { Browser, Page } from 'playwright' +import { chromium } from 'playwright' +import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest' +import { launchWebScaffold, seedSession, watchConsole, webSnapshotMode, type WebScaffold } from './scaffold.ts' +import { saveFailureShot } from './support.ts' + +const SEED = fileURLToPath(new URL('./snapshots/seeded-history/seed.jsonl', import.meta.url)) +const MODE = webSnapshotMode() +/** Enough rows that the list overflows the 800px-tall viewport's sidebar; the scenario asserts the overflow rather than trusting it. */ +const SEED_COUNT = 24 + +/** Geometry and resolved scrollbar style of one scroll container, measured in the page. */ +interface ListMetrics { + /** Resolved `scrollbar-gutter`. */ + gutter: string + /** Resolved `scrollbar-width`. */ + width: string + /** Resolved `scrollbar-color` (thumb then track). */ + color: string + /** The thumb half of `scrollbar-color`, split off the track half. */ + thumb: string + /** `--dsw-alias-scrollbar-bg-l1` resolved on the list into the same colour serialization `scrollbar-color` reports. */ + token: string + /** True when the list actually scrolls. */ + overflows: boolean + /** Border-box width minus client width: the space the scrollbar takes out of the content area. */ + band: number + /** Client-area right edge in viewport coordinates (`clientWidth` excludes the scrollbar band). */ + clientRight: number + /** Border-box right edge in viewport coordinates. */ + borderRight: number + /** Right edge of the first row's relative-time element, the content the unreserved bar covered. */ + timeRight: number +} + +/** + * Measure the sidebar list in the page. + * @param page - the page under test. + * @returns the list's resolved scrollbar style and the geometry the fix changes. + */ +function measureList(page: Page): Promise { + return page.evaluate(() => { + const list = document.querySelector('[role="tree"][aria-label="Sessions"]') + if (list === null) throw new Error('sidebar session list not in the DOM') + const time = list.querySelector('[class*="time"]') + if (time === null) throw new Error('no row relative-time element in the sidebar list') + // The token needs the same serialization `scrollbar-color` reports: the + // palette sheet writes it in whatever notation it chose, so it is + // resolved through a probe element's `color`. The probe is appended to + // the list so `var()` substitution happens where the list sits in the + // cascade — the token reaching THIS element is the claim. + const probe = document.createElement('span') + list.append(probe) + probe.style.color = 'var(--dsw-alias-scrollbar-bg-l1)' + const token = getComputedStyle(probe).color + probe.remove() + const style = getComputedStyle(list) + // `scrollbar-color` serializes as ` `; both halves are + // functional colours, so the split is on the space before the track's + // opening token, not on every space. + const thumb = style.scrollbarColor.replace(/\s+rgba?\([^)]*\)$/, '') + return { + gutter: style.scrollbarGutter, + width: style.scrollbarWidth, + color: style.scrollbarColor, + thumb, + token, + overflows: list.scrollHeight > list.clientHeight, + band: list.getBoundingClientRect().width - list.clientWidth, + clientRight: list.getBoundingClientRect().left + list.clientWidth, + borderRight: list.getBoundingClientRect().right, + timeRight: time.getBoundingClientRect().right, + } + }) +} + +/** + * Reveal the seeded rows: every seeded session is unattached, so they all sit + * in the collapsed Ungrouped bucket. Converges on expanded rather than + * clicking once — startup auto-selection can expand the bucket first, and a + * second click would collapse it again. Hand-rolled polling because + * `expect.poll` is test-scoped and this runs in `beforeAll`. + * @param page - the page under test. + */ +async function expandSeededSessions(page: Page): Promise { + const bucket = page.getByText('Ungrouped', { exact: true }).locator('..').locator('..') + await bucket.waitFor({ timeout: 15_000 }) + const rows = page.locator('[role="tree"][aria-label="Sessions"] [role="treeitem"]') + const deadline = Date.now() + 30_000 + for (;;) { + if (await bucket.getAttribute('aria-expanded') !== 'true') { + await page.getByText('Ungrouped', { exact: true }).click() + } + if (await bucket.getAttribute('aria-expanded') === 'true' && await rows.count() > SEED_COUNT / 2) return + if (Date.now() > deadline) { + throw new Error(`Ungrouped bucket never revealed more than ${SEED_COUNT / 2} rows`) + } + await page.waitForTimeout(200) + } +} + +describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thumb)', () => { + let scaffold: WebScaffold + let browser: Browser + let page: Page + let tripwire: ReturnType + + beforeAll(async () => { + scaffold = await launchWebScaffold({}) + const fixture = await readFile(SEED, 'utf8') + for (let index = 0; index < SEED_COUNT; index += 1) { + await seedSession(scaffold, fixture, `sidebar-scrollbar-web-e2e-${String(index).padStart(2, '0')}`) + } + browser = await chromium.launch() + // Shorter than the other scenarios' 1000px so SEED_COUNT rows overflow + // the list with room to spare. + page = await browser.newPage({ viewport: { width: 1680, height: 800 } }) + tripwire = watchConsole(page) + await page.goto(scaffold.baseUrl, { waitUntil: 'load' }) + await page.waitForSelector('[class*="frame"]', { timeout: 30_000 }) + await expandSeededSessions(page) + }, 180_000) + + afterAll(async () => { + await browser?.close() + await scaffold?.close() + }) + + it('reserves a scrollbar gutter on the overflowing session list', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-gutter')) + // Vacuity guard: with a non-overflowing list `stable` still reserves, but + // the scenario would no longer be reproducing the reported situation. + await expect.poll(async () => (await measureList(page)).overflows, { timeout: 10_000 }).toBe(true) + const metrics = await measureList(page) + expect(metrics.gutter).toBe('stable') + // The control. `band > 0` is the whole observable effect of the + // reservation: the scrollbar is taken out of the content area instead of + // drawn over it. Removing the declaration makes it exactly 0. The value + // itself is not pinned — it tracks `scrollbar-width` and the platform. + expect(metrics.band).toBeGreaterThan(0) + // With the band reserved, the row's relative time — flush against the + // row's right padding, the element the unreserved bar covered — ends + // inside the content area, clear of the bar. Alone this would be vacuous + // under chromium's overlay scrollbar (see the file header); it is + // meaningful only conjoined with the band assertion above. + expect(metrics.timeRight).toBeLessThanOrEqual(metrics.clientRight) + expect(metrics.clientRight).toBeLessThan(metrics.borderRight) + expect(tripwire.pageErrors).toEqual([]) + }, 60_000) + + it('resolves the themed thumb colour on the list in both palettes', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-theme')) + const light = await measureList(page) + // `thin`, not `auto`: the sheet's per-element declaration reached a + // container it never names. + expect(light.width).toBe('thin') + // A concrete colour, not `auto`, and byte-equal to the alias token + // resolved on this element: the indirection carried the token here rather + // than falling back to the UA thumb. + expect(light.color).not.toBe('auto') + expect(light.thumb).toBe(light.token) + // Transparent track, so the thumb reads against the scrolling surface. + expect(light.color.endsWith('rgba(0, 0, 0, 0)')).toBe(true) + // The dark palette declares different scrollbar tokens; driving the body + // attribute pins the cascade the way lifecycle-chrome does (the Settings + // gesture that sets it is owned there). + await page.evaluate(() => { document.body.setAttribute('data-ds-dark-theme', '') }) + const dark = await measureList(page) + expect(dark.thumb).toBe(dark.token) + expect(dark.thumb).not.toBe(light.thumb) + await page.evaluate(() => { document.body.removeAttribute('data-ds-dark-theme') }) + expect((await measureList(page)).thumb).toBe(light.thumb) + expect(tripwire.pageErrors).toEqual([]) + }, 60_000) + + it.skipIf(MODE === 'record')('issued zero model calls and stayed clean', () => { + expect(tripwire.warnings).toEqual([]) + expect(tripwire.pageErrors).toEqual([]) + }) +}) diff --git a/packages/client/ui-command/src/client/PopupSelectView.module.css b/packages/client/ui-command/src/client/PopupSelectView.module.css index c3ab051223..14cf581e13 100644 --- a/packages/client/ui-command/src/client/PopupSelectView.module.css +++ b/packages/client/ui-command/src/client/PopupSelectView.module.css @@ -15,6 +15,10 @@ min-width: 220px; max-height: 320px; overflow-y: auto; + /* Elevated surface: the scrollbar thumb takes the l2 elevation tokens + (see ui-theme styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); border: 1px solid var(--dsw-alias-border-inverted); border-radius: 12px; background: var(--dsw-specific-menu); diff --git a/packages/client/ui-model/src/client/ModelSelect.module.css b/packages/client/ui-model/src/client/ModelSelect.module.css index cdf7f4be2e..6d8b18e6e4 100644 --- a/packages/client/ui-model/src/client/ModelSelect.module.css +++ b/packages/client/ui-model/src/client/ModelSelect.module.css @@ -77,6 +77,13 @@ background: var(--dsw-specific-input-major); box-shadow: var(--dsw-shadow-lv3); color: var(--dsw-alias-label-primary); + /* Elevated surface: the scrollbar thumb takes the l2 elevation tokens. + Declared here rather than on the scrolling `.groups` child so the + elevation choice sits with the surface; the custom properties inherit + down to whichever descendant actually scrolls (see ui-theme + styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } .status, diff --git a/packages/client/ui-settings/src/client/SettingsRoot.module.css b/packages/client/ui-settings/src/client/SettingsRoot.module.css index e2b2c878df..8633145613 100644 --- a/packages/client/ui-settings/src/client/SettingsRoot.module.css +++ b/packages/client/ui-settings/src/client/SettingsRoot.module.css @@ -74,6 +74,13 @@ overflow: hidden; background: var(--dsw-alias-bg-layer-1); box-shadow: var(--dsw-shadow-lv3); + /* Elevated surface: the scrollbar thumb takes the l2 elevation tokens. + Declared on the panel rather than the scrolling `.options` child so the + elevation choice sits with the surface; the custom properties inherit + down to whichever descendant scrolls (see ui-theme + styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } /* Nav rail (figma .Setting-nav 501:29958): 188 wide, pad (12,22,12,0), diff --git a/packages/client/ui-slash/src/client/MenuView.module.css b/packages/client/ui-slash/src/client/MenuView.module.css index bb41e949d0..7bcd95b2de 100644 --- a/packages/client/ui-slash/src/client/MenuView.module.css +++ b/packages/client/ui-slash/src/client/MenuView.module.css @@ -13,6 +13,10 @@ max-width: 537px; max-height: 320px; overflow-y: auto; + /* Elevated surface: the scrollbar thumb takes the l2 elevation tokens + (see ui-theme styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); padding: 4px; display: flex; flex-direction: column; diff --git a/packages/client/ui-theme/README.i18n.yaml b/packages/client/ui-theme/README.i18n.yaml index bc9349de7f..cebdba55d0 100644 --- a/packages/client/ui-theme/README.i18n.yaml +++ b/packages/client/ui-theme/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/client/ui-theme/README.md -README.md: 1227df357cb93241fcf28da9b74d7ba15207e9c5 -README.zh.md: cd87ede7264c8d47dd780acaa11128e83d7862f9 +README.md: 9bf232d506c599a6302c04d5769b43993d84dbf6 +README.zh.md: 84dba38d751b74c13f4af42c40484995900dfc12 diff --git a/packages/client/ui-theme/README.md b/packages/client/ui-theme/README.md index 1227df357c..9bf232d506 100644 --- a/packages/client/ui-theme/README.md +++ b/packages/client/ui-theme/README.md @@ -4,6 +4,10 @@ English | [中文](README.zh.md) Theme plugin: ThemeService over the --dsw-* token base stylesheets (static scale + alias semantic layers). The service owns the theme preference (`light`/`dark`/`system`, persisted under `dsh.theme`), resolves `system` through `prefers-color-scheme`, and publishes immutable `ThemeSnapshot`s on the `theme/change` event; it never touches the DOM — ui-layout's presenter applies the resolved snapshot (`html { color-scheme }`, `body[data-ds-dark-theme]`, and inline alias tokens). Contract: api-contracts v3 §8. +`src/styles/` holds five sheets, all imported by the web shell's `base.css`: `base.css`, `design-platform.css`, `scrollbar.css`, `gradient-shadow-text.css`, and `shiki.css`. `scrollbar.css` is the sole consumer of the `--dsw-alias-scrollbar-*` tokens and must follow `design-platform.css`, which declares them. + +Scrollbar rebinding contract: `scrollbar.css` binds `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover` on `body` to the l1 (base-surface) tokens, and both the standard `scrollbar-color` and the `::-webkit-scrollbar-thumb` rules read that pair. An elevated surface (menu, popover, dialog) sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container; one rebind retints both renderings. Reasoning and the measured computed values: [the scrollbar Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md). + ## Model Experience None, as the theme service manages a browser preference; nothing here reaches a model request. diff --git a/packages/client/ui-theme/README.zh.md b/packages/client/ui-theme/README.zh.md index cd87ede726..84dba38d75 100644 --- a/packages/client/ui-theme/README.zh.md +++ b/packages/client/ui-theme/README.zh.md @@ -4,6 +4,10 @@ 主题插件：基于 --dsw-* token 基础样式表（静态尺度 + 别名语义层）的 ThemeService。该服务拥有主题偏好（`light`／`dark`／`system`，以 `dsh.theme` 为键持久化），将 `system` 通过 `prefers-color-scheme` 解析为实际主题，并发布不可变的 `ThemeSnapshot`，通过 `theme/change` 事件通知变化；它绝不接触 DOM：ui-layout 的呈现器会应用解析后的快照（`html { color-scheme }`、`body[data-ds-dark-theme]`，以及主题的别名 token 内联变量）。契约：api-contracts v3 §8。 +`src/styles/` 下有五张样式表，全部由 web 壳的 `base.css` 导入：`base.css`、`design-platform.css`、`scrollbar.css`、`gradient-shadow-text.css` 与 `shiki.css`。`scrollbar.css` 是 `--dsw-alias-scrollbar-*` token 的唯一消费方，必须排在声明这些 token 的 `design-platform.css` 之后。 + +滚动条重新绑定契约：`scrollbar.css` 在 `body` 上把 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover` 绑定到 l1（基础表面）token，标准属性 `scrollbar-color` 与 `::-webkit-scrollbar-thumb` 规则都读取这一组变量。抬升表面（菜单、浮层、对话框）在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`；一次重新绑定即可为两种渲染同时换色。推理过程与实测计算值见[滚动条 Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md)。 + ## 模型体验 无。主题服务管理浏览器偏好；这里没有任何内容进入模型请求。 diff --git a/packages/client/ui-theme/src/styles/scrollbar.css b/packages/client/ui-theme/src/styles/scrollbar.css new file mode 100644 index 0000000000..4aea10efab --- /dev/null +++ b/packages/client/ui-theme/src/styles/scrollbar.css @@ -0,0 +1,66 @@ +/* Scrollbar skin: the sole consumer of the four --dsw-alias-scrollbar-* + * tokens. Without it every scrolling region renders the UA scrollbar, which + * ignores the theme — a light native bar over the dark palette. + * + * The rule sits on `body`, not `html`: design-platform.css declares the + * --dsw-alias-* tokens on `body` (and the dark overrides on + * `body[data-ds-dark-theme]`), and custom properties only inherit downward, + * so an `html` rule resolves them to the guaranteed-invalid value and + * `scrollbar-color` falls back to `auto`. + * + * `scrollbar-color` is an inherited property, so binding it once on `body` + * reaches every scroll container without enumerating module class names. + * `scrollbar-width` is NOT inherited, so it is applied to all elements. + * The WebKit pseudo-elements are not inherited either, hence the unscoped + * `::-webkit-scrollbar` rules. + * + * Surfaces pick their elevation by rebinding --dsh-scrollbar-thumb{,-hover}: + * the l1 pair here is the base-surface default, and an elevated surface + * (menu, popover, dialog) rebinds to the l2 pair on its own container. Both + * the standard properties and the WebKit pseudo-elements read the + * indirection, so one rebind reaches both renderings. */ + +body { + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l1); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l1); +} + +/* `scrollbar-color` and `scrollbar-width` are declared on every element + rather than inherited from `body`. Inheriting would pass down the COLOUR + already substituted at `body`, so a descendant rebinding + --dsh-scrollbar-thumb could not change it; re-declaring makes each element + substitute the variable as it sees it, which is what gives an elevated + surface a working rebind. `scrollbar-width` is not an inherited property + at all, so it needs the per-element declaration regardless. + + Track stays transparent so the thumb reads against whatever surface + scrolls under it; only the thumb carries a token colour. */ +body, +body * { + scrollbar-width: thin; + scrollbar-color: var(--dsh-scrollbar-thumb) transparent; +} + +::-webkit-scrollbar { + width: 8px; + height: 8px; +} + +::-webkit-scrollbar-track { + background: transparent; +} + +::-webkit-scrollbar-thumb { + border-radius: 4px; + background: var(--dsh-scrollbar-thumb); +} + +::-webkit-scrollbar-thumb:hover { + background: var(--dsh-scrollbar-thumb-hover); +} + +/* Both scrollbars meeting in a corner: no separate token, so the corner + matches the transparent track rather than the UA's opaque default. */ +::-webkit-scrollbar-corner { + background: transparent; +} diff --git a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts new file mode 100644 index 0000000000..a53a19eecf --- /dev/null +++ b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts @@ -0,0 +1,311 @@ +/** + * Scrollbar stylesheet contract, asserted against the CSS text on disk: every + * --dsw-alias-scrollbar-* token design-platform.css defines has a consumer, + * scrollbar.css binds the base-surface pair through the rebindable + * indirection, and elevated surfaces rebind that indirection in complete + * pairs. The expected token set is scanned out of design-platform.css, so + * adding, renaming, or dropping a scrollbar token moves these assertions with + * it. + */ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +/** One flattened CSS rule: its comma-separated selector parts and its declarations in source order. */ +interface CssRule { + selectors: string[] + declarations: [property: string, value: string][] +} + +const STYLES = new URL('../src/styles/', import.meta.url) +const PACKAGES_DIR = fileURLToPath(new URL('../../../', import.meta.url)) +const read = (name: string): string => readFileSync(fileURLToPath(new URL(name, STYLES)), 'utf8') + +const platformCss = read('design-platform.css') +const scrollbarCss = read('scrollbar.css') + +/** Body attribute selecting the dark palette; ui-layout's ThemePresenter sets it. */ +const DARK_ATTRIBUTE = '[data-ds-dark-theme]' +/** Alias tokens under test: the prefix the elevation pairs share. */ +const TOKEN_PREFIX = '--dsw-alias-scrollbar-' +/** Prefix of the rebindable indirection scrollbar.css owns. */ +const INDIRECTION_PREFIX = '--dsh-scrollbar-' + +/** + * Flatten a stylesheet into rules. Whitespace, declaration order, and trailing + * semicolons are normalized away; nesting and at-rules are not handled, which + * no sheet under test uses for scrollbar declarations. + * @param css - stylesheet text. + * @returns one entry per rule, in source order. + */ +function parseRules(css: string): CssRule[] { + const withoutComments = css.replace(/\/\*[\s\S]*?\*\//g, ' ') + const rules: CssRule[] = [] + // Destructuring defaults only satisfy noUncheckedIndexedAccess; both groups + // are unconditional in the pattern. + for (const [, selector = '', body = ''] of withoutComments.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { + const declarations = body + .split(';') + .map(part => part.trim()) + .filter(part => part.includes(':')) + .map((part): [string, string] => { + const colon = part.indexOf(':') + return [part.slice(0, colon).trim(), part.slice(colon + 1).trim()] + }) + rules.push({ selectors: selector.split(',').map(part => part.trim()), declarations }) + } + return rules +} + +/** + * Custom-property names a value reads. + * @param value - declaration value, possibly with nested var() calls. + * @returns every referenced custom-property name, in source order. + */ +function varReferences(value: string): string[] { + return [...value.matchAll(/var\(\s*(--[\w-]+)/g)].map(([, name = '']) => name) +} + +/** + * Every CSS file shipped as package source, excluding build output and + * installed dependencies. + * @returns absolute paths of the stylesheets under packages/. + */ +function packageStylesheets(): string[] { + const found: string[] = [] + const walk = (dir: string): void => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = join(dir, entry.name) + if (entry.isDirectory()) { + if (entry.name !== 'node_modules' && entry.name !== 'lib' && entry.name !== 'dist') walk(path) + } else if (entry.name.endsWith('.css')) found.push(path) + } + } + walk(PACKAGES_DIR) + return found +} + +/** + * Tokens a stylesheet reads through its rendering declarations, following its + * own custom-property definitions transitively so a token reached only through + * an indirection counts. The walk starts from the standard-property + * declarations, so a defined-but-unread indirection contributes nothing. + * @param rules - parsed rules of one stylesheet. + * @returns every `--dsw-*` token the sheet's rendering declarations depend on. + */ +function tokensRendered(rules: CssRule[]): Set { + const definitions = new Map() + const pending: string[] = [] + for (const rule of rules) { + for (const [property, value] of rule.declarations) { + if (property.startsWith('--')) definitions.set(property, value) + else pending.push(value) + } + } + const reached = new Set() + const visited = new Set() + while (pending.length > 0) { + for (const name of varReferences(pending.pop()!)) { + if (name.startsWith('--dsw-')) reached.add(name) + if (visited.has(name)) continue + visited.add(name) + const definition = definitions.get(name) + if (definition !== undefined) pending.push(definition) + } + } + return reached +} + +const platformRules = parseRules(platformCss) +const scrollbarRules = parseRules(scrollbarCss) +const sorted = (names: Iterable): string[] => [...names].sort() + +/** + * Scrollbar tokens defined by the rules whose selectors carry (or do not + * carry) the dark palette attribute. + * @param dark - true to scan the dark blocks, false to scan the light blocks. + * @returns the scrollbar token names defined there. + */ +function definedTokens(dark: boolean): Set { + const names = new Set() + for (const rule of platformRules) { + if (rule.selectors.every(selector => selector.includes(DARK_ATTRIBUTE)) !== dark) continue + for (const [property] of rule.declarations) { + if (property.startsWith(TOKEN_PREFIX)) names.add(property) + } + } + return names +} + +const lightTokens = definedTokens(false) +const darkTokens = definedTokens(true) +const allTokens = new Set([...lightTokens, ...darkTokens]) + +/** Every scrollbar token any package stylesheet references, mapped to the files referencing it. */ +const referencedTokens = new Map() +/** Every indirection property any package stylesheet outside ui-theme declares, mapped to its declaring rules. */ +const rebindRules: { file: string; rule: CssRule }[] = [] + +for (const file of packageStylesheets()) { + const rules = parseRules(readFileSync(file, 'utf8')) + for (const rule of rules) { + let rebinds = false + for (const [property, value] of rule.declarations) { + if (property.startsWith(INDIRECTION_PREFIX) && file !== fileURLToPath(new URL('scrollbar.css', STYLES))) rebinds = true + for (const token of varReferences(value)) { + if (!token.startsWith(TOKEN_PREFIX)) continue + referencedTokens.set(token, [...referencedTokens.get(token) ?? [], file]) + } + } + if (rebinds) rebindRules.push({ file, rule }) + } +} + +describe('design-platform.css scrollbar tokens', () => { + it('defines the same scrollbar token set in the light and the dark block', () => { + // A token present only in the light block silently keeps its light value + // under the dark palette, since the dark block only overrides. + expect(allTokens.size).toBeGreaterThan(0) + expect(sorted(lightTokens)).toEqual(sorted(allTokens)) + expect(sorted(darkTokens)).toEqual(sorted(allTokens)) + }) + + it('resolves every scrollbar token to a static scale value, not to another alias', () => { + // The alias layer is the only indirection in the token sheet: an alias + // pointing at a second alias makes the dark override order-dependent. + for (const rule of platformRules) { + for (const [property, value] of rule.declarations) { + if (!property.startsWith(TOKEN_PREFIX)) continue + for (const reference of varReferences(value)) { + expect(reference, `${property}: ${value}`).toMatch(/^--dsw-static-/) + } + } + } + }) +}) + +describe('scrollbar token consumers', () => { + it('every defined scrollbar token is referenced by some package stylesheet', () => { + // Before scrollbar.css existed these tokens had no consumer at all and + // every scroll container rendered the unthemed UA bar. A fifth token, or a + // rename on one side only, leaves the new name unreferenced here. + expect(sorted(referencedTokens.keys())).toEqual(sorted(allTokens)) + }) + + it('every referenced scrollbar token is defined in design-platform.css', () => { + // A dangling var() renders the UA default instead of failing loudly, so a + // rename has to move the reference and the definition together. + for (const [token, files] of referencedTokens) { + expect(allTokens, files.join(', ')).toContain(token) + } + }) +}) + +describe('scrollbar.css base-surface binding', () => { + const rendered = tokensRendered(scrollbarRules) + + it('renders the l1 pair through the rebindable indirection', () => { + // l1 is the base-surface default the indirection resolves to; the + // indirection only counts as bound when a rendering declaration reads it. + expect(rendered).toContain(`${TOKEN_PREFIX}bg-l1`) + expect(rendered).toContain(`${TOKEN_PREFIX}hover-l1`) + }) + + it('routes the standard property and the WebKit thumb through the same indirection', () => { + // A rebind on an elevated container has to move the Firefox and the WebKit + // rendering together, which only holds while both read the same variable. + const declaration = (property: string, selectorPart: string): string | undefined => scrollbarRules + .filter(rule => rule.selectors.includes(selectorPart)) + .flatMap(rule => rule.declarations) + .findLast(([name]) => name === property)?.[1] + const thumbColor = declaration('scrollbar-color', 'body') + expect(thumbColor).toBeDefined() + const indirection = varReferences(thumbColor!)[0] + expect(indirection).toBe(`${INDIRECTION_PREFIX}thumb`) + expect(varReferences(declaration('background', '::-webkit-scrollbar-thumb')!)).toEqual([indirection]) + }) +}) + +describe('scrollbar.css selectors', () => { + const scrollbarColorSelectors = scrollbarRules + .filter(rule => rule.declarations.some(([property]) => property === 'scrollbar-color')) + .flatMap(rule => rule.selectors) + + it('declares scrollbar-color only where the body-scoped tokens are visible', () => { + // design-platform.css defines the alias tokens on `body`, and custom + // properties inherit downward only: the same declaration on `html` or + // `:root` resolves to the guaranteed-invalid value, which computes + // scrollbar-color to `auto` and drops the theming entirely. + expect(scrollbarColorSelectors.length).toBeGreaterThan(0) + for (const selector of scrollbarColorSelectors) { + expect(selector, selector).toMatch(/^body\b/) + } + }) + + it('defines the indirection where the alias tokens are visible', () => { + const definesIndirection = ([property, value]: [string, string]): boolean => + property.startsWith(INDIRECTION_PREFIX) && value.includes(TOKEN_PREFIX) + const hosts = scrollbarRules + .filter(rule => rule.declarations.some(definesIndirection)) + .flatMap(rule => rule.selectors) + expect(hosts.length).toBeGreaterThan(0) + for (const selector of hosts) expect(selector, selector).toMatch(/^body\b/) + }) + + it('re-declares the scrollbar properties per element rather than inheriting them', () => { + // scrollbar-width is not an inherited property, and an inherited + // scrollbar-color carries the colour already substituted at `body`, which + // a descendant rebinding the indirection could no longer change. + expect(scrollbarColorSelectors).toContain('body *') + const widthSelectors = scrollbarRules + .filter(rule => rule.declarations.some(([property]) => property === 'scrollbar-width')) + .flatMap(rule => rule.selectors) + expect(widthSelectors).toContain('body *') + }) +}) + +describe('elevated surface rebinds', () => { + it('at least one surface rebinds the indirection', () => { + expect(rebindRules.length).toBeGreaterThan(0) + }) + + it('each rebinding rule sets the thumb and the hover variable together', () => { + // A surface rebinding only the resting colour keeps the l1 hover colour, + // so the elevation is wrong only while the pointer is over the thumb. + for (const { file, rule } of rebindRules) { + const properties = rule.declarations.map(([property]) => property).filter(property => property.startsWith(INDIRECTION_PREFIX)) + expect(sorted(properties), `${file} ${rule.selectors.join(', ')}`).toEqual([ + `${INDIRECTION_PREFIX}thumb-hover`, `${INDIRECTION_PREFIX}thumb`, + ].sort()) + } + }) + + it('each rebinding rule binds the indirection names scrollbar.css renders', () => { + // A misspelled property name declares an unused variable, and the surface + // silently keeps the base-surface colour. + const rendered = new Set( + scrollbarRules + .flatMap(rule => rule.declarations) + .filter(([property]) => !property.startsWith('--')) + .flatMap(([, value]) => varReferences(value)) + .filter(name => name.startsWith(INDIRECTION_PREFIX)), + ) + for (const { file, rule } of rebindRules) { + for (const [property] of rule.declarations) { + if (property.startsWith(INDIRECTION_PREFIX)) expect(rendered, `${file}: ${property}`).toContain(property) + } + } + }) + + it('every rebind targets the l2 elevation pair', () => { + for (const { file, rule } of rebindRules) { + for (const [property, value] of rule.declarations) { + if (!property.startsWith(INDIRECTION_PREFIX)) continue + for (const token of varReferences(value)) { + expect(token, `${file}: ${property}`).toMatch(/-l2$/) + } + } + } + }) +}) diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css index c03d511c92..2f7bc1fbc6 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.module.css @@ -207,6 +207,13 @@ min-height: 0; overflow-y: auto; padding-bottom: 12px; + /* Row trailing content (the relative time, and the hover action buttons + that replace it) sits flush against the row's 8px right padding, so an + overlaid scrollbar covers it. Reserving the gutter keeps the bar beside + the rows instead of on top of them; `stable` holds the reservation when + the list is short enough not to scroll, so expanding a group does not + shift every row left. */ + scrollbar-gutter: stable; } /* One workspace section: header row + expanded session run. Rows inside diff --git a/packages/client/ui-workspace/tests/browser-styles.spec.ts b/packages/client/ui-workspace/tests/browser-styles.spec.ts new file mode 100644 index 0000000000..d2ac07f0c2 --- /dev/null +++ b/packages/client/ui-workspace/tests/browser-styles.spec.ts @@ -0,0 +1,48 @@ +/** + * WorkspaceBrowser scroll-region style contract, asserted against the CSS text + * on disk: the session list reserves its scrollbar gutter so the scrollbar + * cannot overlay row trailing content, and reserves it whether or not the list + * currently overflows so expanding a group does not shift rows sideways. + */ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const css = readFileSync(fileURLToPath(new URL('../src/client/WorkspaceBrowser.module.css', import.meta.url)), 'utf8') + +/** + * Declarations of one class rule, keyed by property with whitespace collapsed. + * Declaration order and trailing semicolons are normalized away. + * @param className - local class name, without the leading dot. + * @returns the rule's declarations, or undefined when no such rule exists. + */ +function declarations(className: string): Map | undefined { + const withoutComments = css.replace(/\/\*[\s\S]*?\*\//g, ' ') + const match = new RegExp(String.raw`(^|[\s,}])\.${className}\s*\{([^{}]*)\}`).exec(withoutComments) + if (match === null) return undefined + const found = new Map() + // The body group is unconditional in the pattern; the fallback only satisfies + // noUncheckedIndexedAccess. + for (const part of (match[2] ?? '').split(';')) { + const colon = part.indexOf(':') + if (colon === -1) continue + found.set(part.slice(0, colon).trim(), part.slice(colon + 1).trim().replace(/\s+/g, ' ')) + } + return found +} + +describe('WorkspaceBrowser.module.css list', () => { + const list = declarations('list') + + it('is the scrolling region', () => { + expect(list).toBeDefined() + expect(list!.get('overflow-y')).toBe('auto') + }) + + it('reserves the scrollbar gutter unconditionally', () => { + // Row trailing content sits flush against the row's right padding, so an + // overlay scrollbar covers it. `stable` keeps the reservation when the list + // is short enough not to scroll, so expanding a group does not shift rows. + expect(list!.get('scrollbar-gutter')).toBe('stable') + }) +}) diff --git a/packages/client/web/src/base.css b/packages/client/web/src/base.css index b8449634eb..92d3d5383a 100644 --- a/packages/client/web/src/base.css +++ b/packages/client/web/src/base.css @@ -1,8 +1,10 @@ /* Shell-owned global base: full-height mount plus the theme token sheets. - * The four ui-theme sheets are the sole token source (--dsw-*); the shell - * links them here so tokens exist before any plugin CSS lands. */ + * The five ui-theme sheets are the sole token source (--dsw-*); the shell + * links them here so tokens exist before any plugin CSS lands. scrollbar.css + * follows design-platform.css because it reads that sheet's tokens. */ @import '@deepseek-ai/dsh-client-ui-theme/styles/base.css'; @import '@deepseek-ai/dsh-client-ui-theme/styles/design-platform.css'; +@import '@deepseek-ai/dsh-client-ui-theme/styles/scrollbar.css'; @import '@deepseek-ai/dsh-client-ui-theme/styles/gradient-shadow-text.css'; @import '@deepseek-ai/dsh-client-ui-theme/styles/shiki.css'; diff --git a/packages/client/web/tests/base-styles.spec.ts b/packages/client/web/tests/base-styles.spec.ts new file mode 100644 index 0000000000..d87921cede --- /dev/null +++ b/packages/client/web/tests/base-styles.spec.ts @@ -0,0 +1,58 @@ +/** + * Shell base sheet contract, asserted against the CSS text on disk: base.css is + * where the ui-theme token sheets enter the bundle, every sheet it names exists, + * and scrollbar.css follows design-platform.css because it reads that sheet's + * tokens. + */ +import { existsSync, readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const THEME_PACKAGE = '@deepseek-ai/dsh-client-ui-theme' +const baseCss = readFileSync(fileURLToPath(new URL('../src/base.css', import.meta.url)), 'utf8') + +/** + * Import specifiers of the sheet, in source order. Quote style and surrounding + * whitespace are normalized away. + * @param css - stylesheet text. + * @returns each `@import` target in the order the sheet lists it. + */ +function importOrder(css: string): string[] { + // The destructuring default only satisfies noUncheckedIndexedAccess; the + // group is unconditional in the pattern. + return [...css.matchAll(/@import\s+['"]([^'"]+)['"]/g)].map(([, specifier = '']) => specifier) +} + +/** + * Resolve a `/styles/` specifier to its path in the workspace. + * The theme package maps `./styles/*` to `./src/styles/*`, so the sheets stay + * on the source plane rather than needing a build. + * @param specifier - import specifier from base.css. + * @returns absolute path of the file the specifier names. + */ +function resolveThemeSheet(specifier: string): string { + const name = specifier.slice(`${THEME_PACKAGE}/styles/`.length) + return fileURLToPath(new URL(`../../ui-theme/src/styles/${name}`, import.meta.url)) +} + +const imports = importOrder(baseCss) + +describe('web shell base.css', () => { + it('imports every sheet from the theme package and each one exists', () => { + expect(imports.length).toBeGreaterThan(0) + for (const specifier of imports) { + expect(specifier.startsWith(`${THEME_PACKAGE}/styles/`), specifier).toBe(true) + expect(existsSync(resolveThemeSheet(specifier)), specifier).toBe(true) + } + }) + + it('imports the scrollbar sheet after the token sheet it reads', () => { + // Both sheets bind on `body`, so with scrollbar.css first the alias tokens + // would still resolve; the order encodes the dependency direction so a + // later specificity or selector change cannot silently invert it. + const platform = imports.indexOf(`${THEME_PACKAGE}/styles/design-platform.css`) + const scrollbar = imports.indexOf(`${THEME_PACKAGE}/styles/scrollbar.css`) + expect(platform).toBeGreaterThanOrEqual(0) + expect(scrollbar).toBeGreaterThan(platform) + }) +}) From a0319671810c0de85ab4971251655dae3330089d Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 11:06:03 +0800 Subject: [PATCH 2/8] test(web): register the sidebar scrollbar e2e on the host plane Every scaffold-importing e2e compiles on the host plane, so the new file goes in tsconfig.host.json's include list and apps/web/tsconfig.json's exclude list. Without both, tsc -p apps/web/tsconfig.json fails with TS6059/TS6307. --- apps/web/tsconfig.json | 1 + tsconfig.host.json | 1 + 2 files changed, 2 insertions(+) diff --git a/apps/web/tsconfig.json b/apps/web/tsconfig.json index 559e72618b..1b0f807d5f 100644 --- a/apps/web/tsconfig.json +++ b/apps/web/tsconfig.json @@ -32,6 +32,7 @@ "tests/workspace-management.e2e.ts", "tests/replay-round-trip.e2e.ts", "tests/seeded-history.e2e.ts", + "tests/sidebar-scrollbar.e2e.ts", "tests/code-mode-round.e2e.ts", "tests/cordis-tool-round.e2e.ts" ], diff --git a/tsconfig.host.json b/tsconfig.host.json index 9cf2a86bda..b00b752328 100644 --- a/tsconfig.host.json +++ b/tsconfig.host.json @@ -19,6 +19,7 @@ "apps/web/tests/workspace-management.e2e.ts", "apps/web/tests/replay-round-trip.e2e.ts", "apps/web/tests/seeded-history.e2e.ts", + "apps/web/tests/sidebar-scrollbar.e2e.ts", "apps/web/tests/code-mode-round.e2e.ts", "apps/web/tests/cordis-tool-round.e2e.ts", "apps/cli/tests/**/*.ts", From 474b88e362a80ab6b3938acc3283ec87b096b596 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 12:09:18 +0800 Subject: [PATCH 3/8] docs: record the plugin-bundle rebuild trap in the scrollbar note Verifying browser-visible plugin CSS needs a rebuild build:web does not perform: WorkspaceBrowser.module.css never reaches apps/web/dist, because ui-workspace loads as a runtime plugin with its CSS inlined into lib/client.js by that package's own bundle script. A negative control that reruns only build:web exercises a stale bundle and passes with the declaration removed, which reads as a vacuous test rather than an invalid control. No script in the web lane does this rebuild, so every scroll-region or plugin-CSS change hits the same trap. --- ...2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml | 4 ++-- .../2026-07-28-themed-scrollbars-and-reserved-gutter.md | 2 ++ .../2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index 5d9b727b2e..6b6257c3b6 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: 29aad9976610b02b42e0c69222504a84188e34d7 -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: 2c5c4eefee88680df35d1a069e6ab5de7884144f +2026-07-28-themed-scrollbars-and-reserved-gutter.md: 404d1c037774aa24484cfac9227ad1d8b4d816d2 +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: ffad52e8dd0e72ca17eae058ee5cbf56764b22af diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index 29aad99766..404d1c0377 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -55,3 +55,5 @@ Three unit specs read the CSS text on disk. `ui-theme/tests/scrollbar-styles.spe Confirmed in headless chromium on the built client by reading computed values, which is what distinguishes a working token chain from a syntactically valid one: a scroll container computes the l1 thumb color in each palette, and a container that rebinds the indirection computes the l2 color, proving the rebind reaches the computed value rather than only the custom property. Headless chromium draws overlay scrollbars, so a reserved gutter there does not shrink `clientWidth`. The reservation shows up as a non-zero `offsetWidth - clientWidth` band on the list; client-area geometry alone does not demonstrate it, and an assertion comparing the time element's right edge against the client-area edge holds with and without the reservation, so it would pass or fail on the platform's scrollbar style rather than on the declaration under test. + +Verifying browser-visible plugin CSS needs a rebuild `pnpm run build:web` does not perform. `WorkspaceBrowser.module.css` never reaches `apps/web/dist`: ui-workspace loads as a runtime plugin and its CSS is inlined into `packages/client/ui-workspace/lib/client.js`, built by that package's own `bundle` script. A negative control that reruns only `build:web` therefore exercises a stale bundle and passes with the declaration removed, which reads as a vacuous test rather than as an invalid control. Rebuild with `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`, confirm the artifact by grepping `lib/client.js` for the declaration, then `build:web`. No script in the web lane does this: `test:web` runs `build:web` alone, so every scroll-region or plugin-CSS change hits the same trap. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index 2c5c4eefee..ffad52e8dd 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -55,3 +55,5 @@ Status: implemented 在构建产物客户端上于 headless chromium 中读取计算值确认，这正是区分「token 链真正生效」与「语法合法」的手段：滚动容器在两套调色板下分别计算出 l1 的滑块颜色，而重新绑定间接变量的容器计算出 l2 的颜色，证明重新绑定作用到了计算值，而不只是作用到自定义属性上。 headless chromium 绘制的是覆盖式滚动条，因此其中预留空位不会缩小 `clientWidth`。该预留表现为列表上非零的 `offsetWidth - clientWidth` 条带；仅凭内容区几何无法证明它，而把时间元素右边缘与内容区右边缘做比较的断言，在有无预留的两种状态下都成立，因此它的通过或失败取决于平台的滚动条样式，而不是取决于被测的那条声明。 + +验证浏览器可见的插件 CSS 需要一次 `pnpm run build:web` 并不执行的重建。`WorkspaceBrowser.module.css` 从不进入 `apps/web/dist`：ui-workspace 以运行时插件方式加载，其 CSS 内联进 `packages/client/ui-workspace/lib/client.js`，由该包自己的 `bundle` 脚本构建。因此只重跑 `build:web` 的反向对照实际测的是旧产物，去掉声明后仍会通过，看起来像测试无效，实际是对照无效。正确做法是先 `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`，用 grep 在 `lib/client.js` 中确认该声明确实存在或消失，然后再 `build:web`。web 通道中没有任何脚本会做这一步：`test:web` 只运行 `build:web`，因此任何滚动区域或插件 CSS 的改动都会碰到同一个陷阱。 From c3e9690b2ea74909ffaba218ed70fc7cf9d666f8 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 15:02:20 +0800 Subject: [PATCH 4/8] fix(ui-theme): gate the standard scrollbar properties behind the missing WebKit pseudo-element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-`auto` `scrollbar-width` or `scrollbar-color` makes Chromium and Safari discard every `::-webkit-scrollbar*` rule for that element, including `::-webkit-scrollbar-thumb:hover`. Declaring both unconditionally left the hover tokens rendering nowhere: the engines implementing the hover pseudo-element are exactly the ones the standard properties silence, and Firefox has no hover pseudo-element to fall back on. Both hover tokens and all four elevated surfaces' hover rebinds were therefore dead code. Measured in chromium on probe elements with `scrollbar-gutter: stable`: an 8px `::-webkit-scrollbar` alone reserved a 30px band, and adding `scrollbar-width: thin` dropped it to the 10px `thin` reserves. The standard properties now sit inside `@supports not selector(::-webkit-scrollbar)`, so Firefox takes them and WebKit-based engines take the pseudo-elements. The WebKit rules stay ungated: an engine without those pseudo-elements drops them as unknown selectors, and gating them would hide them from an engine that implements them without `selector()` — the pre-16.4 Safari the ungated form serves correctly. Three unit assertions pin the split by source offset, which the existing at-rule-flattening parser cannot see. The web e2e now reads the path chromium actually takes: the `auto` standard properties as the gate's signature, the pseudo-element sizing and track, the indirection variables resolved per throwaway probe, and the hover declaration as cascade rule text — chromium folds the `:hover` rule into `getComputedStyle(el, '::-webkit-scrollbar-thumb')`, so no computed query separates the states. --- ...d-scrollbars-and-reserved-gutter.i18n.yaml | 4 +- ...8-themed-scrollbars-and-reserved-gutter.md | 19 ++- ...hemed-scrollbars-and-reserved-gutter.zh.md | 19 ++- apps/web/tests/sidebar-scrollbar.e2e.ts | 139 ++++++++++++------ packages/client/ui-theme/README.i18n.yaml | 4 +- packages/client/ui-theme/README.md | 4 +- packages/client/ui-theme/README.zh.md | 4 +- .../client/ui-theme/src/styles/scrollbar.css | 63 +++++--- .../ui-theme/tests/scrollbar-styles.spec.ts | 75 ++++++++++ 9 files changed, 251 insertions(+), 80 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index 6b6257c3b6..f841a008ea 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: 404d1c037774aa24484cfac9227ad1d8b4d816d2 -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: ffad52e8dd0e72ca17eae058ee5cbf56764b22af +2026-07-28-themed-scrollbars-and-reserved-gutter.md: 71d2e5b5156d3bc54968552aa2eef82fab2ff443 +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: fcae03440ff621d949f8158990e2d871590b7daa diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index 404d1c0377..71d2e5b515 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -18,7 +18,9 @@ The rules sit on `body`, not `html`. `design-platform.css` declares the `--dsw-a `scrollbar-width` and `scrollbar-color` are declared on `body, body *` rather than once at the top. Inheritance would pass down the color already substituted at `body`, so a descendant rebinding the indirection could not change its own scrollbar; re-declaring makes each element substitute the variable as it sees it. `scrollbar-width` is not an inherited property in the first place, so it needs the per-element declaration regardless. The `::-webkit-scrollbar*` pseudo-elements are likewise not inherited and are matched unscoped. -Both halves read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Four surfaces rebind today: the command popup, the slash menu, the model-select panel, and the settings panel. The last two declare it on the elevated panel rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. +The two renderings are mutually exclusive, and the exclusion is enforced rather than assumed. A non-`auto` `scrollbar-width` or `scrollbar-color` makes Chromium and Safari discard every `::-webkit-scrollbar*` rule for that element, `::-webkit-scrollbar-thumb:hover` included. Declaring both unconditionally therefore leaves the hover token rendering nowhere at all: the engines that implement the hover pseudo-element are exactly the ones the standard properties silence, and Firefox has no hover pseudo-element to fall back on. The standard properties consequently sit inside `@supports not selector(::-webkit-scrollbar)`, which is true only where the pseudo-element is unimplemented, so Firefox takes the standard path and WebKit-based engines take the pseudo-element path. The WebKit rules are not gated in turn: an engine without those pseudo-elements drops them as unknown selectors, so a gate would only restate what selector matching already does. An engine too old for the `selector()` function makes the condition invalid, which evaluates false and selects the pseudo-element path — the correct side for the pre-16.4 Safari that is the realistic case for that reading. + +Both paths read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Four surfaces rebind today: the command popup, the slash menu, the model-select panel, and the settings panel. The last two declare it on the elevated panel rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. The track and the corner stay transparent, so the thumb reads against whatever surface scrolls under it; only the thumb and its hover state carry a token color. @@ -34,6 +36,10 @@ The track and the corner stay transparent, so the thumb reads against whatever s **Declare the properties once and let them inherit.** Fewer matched elements, and it breaks the rebinding contract — inheritance carries the substituted color, not the variable reference, so an elevated surface could not retint its own scrollbar. It is also incomplete on its own terms, since `scrollbar-width` does not inherit. +**Declare the standard properties and the pseudo-elements unconditionally, without the `@supports` gate.** This is what the change originally shipped, and review caught it. Measured in chromium on probe elements with `scrollbar-gutter: stable` so the band is observable: an 8px `::-webkit-scrollbar` alone reserved a 30px band (the sheet's width plus the UA's buttons), and adding `scrollbar-width: thin` to the same element dropped it to the 10px `thin` reserves — the pseudo-element rules were being discarded, not merged. Every `::-webkit-scrollbar-thumb:hover` rule went with them, so both hover tokens and all four elevated surfaces' hover rebinds were dead code on the engine most users run. + +**Gate the WebKit rules too, behind `@supports selector(::-webkit-scrollbar)`.** Symmetrical to read, and wrong in one direction: it would hide the rules from an engine that implements the pseudo-elements but not `selector()`, which is the pre-16.4 Safari the ungated form serves correctly. Unknown selectors are already dropped, so the gate adds no protection to pay for that. + **Pad the rows instead of reserving the gutter (extra right padding on `.list`, or moving `.time` inward).** Rejected: padding applies whether or not a bar is present, so it costs horizontal room in the common short-list case, and it fixes exactly one container while leaving every other scrolling region's content under its bar. **`scrollbar-gutter: auto` on `.list`.** The reservation appears when the list overflows, which is when the bar exists. Rejected because the sidebar's lists grow and shrink as groups expand, so the reservation would appear and disappear under the user's cursor and shift the rows with it. @@ -42,17 +48,22 @@ The track and the corner stay transparent, so the thumb reads against whatever s - Every scroll container in the client draws the themed thumb: `rgb(229, 229, 229)` on a light base surface, `rgb(60, 60, 61)` on a dark one, and `rgb(84, 85, 87)` for a dark elevated surface that rebinds to the l2 pair. - The two renderings are separately specified, so a change to the thumb's geometry or hover behavior has to be made twice — once in `scrollbar-width`/`scrollbar-color`, once in the pseudo-elements. Routing both through the indirection pair confines that duplication to the properties Firefox and WebKit do not share. +- The hover tokens (`--dsw-alias-scrollbar-hover-l1`/`-l2`) render only on the pseudo-element path. Firefox states one thumb color through `scrollbar-color` and derives its own hover treatment, so a design change to the hover colors is visible in Chromium and Safari and not in Firefox. This is a limit of `scrollbar-color`, not of the sheet. - `body *` matches every element, for two properties whose effect the user agent already limits to elements that actually scroll. The cost is a broad selector; the alternative was a rebinding contract that does not work. - The workspace list is permanently narrower by the reserved band, at every list length. That is the trade the fix buys: stable row geometry instead of a timestamp that is legible only while the list is short. - There is no track token in the palette, so a design that later wants an opaque track needs a new alias token rather than a literal color in this sheet. ## Testing -Three unit specs read the CSS text on disk. `ui-theme/tests/scrollbar-styles.spec.ts` scans the scrollbar token set out of `design-platform.css` rather than hardcoding it, so adding, renaming, or dropping a token moves the assertions with it, and checks that every token has a consumer and that each elevated surface rebinds a complete pair. `web/tests/base-styles.spec.ts` pins the import order and the existence of every sheet `base.css` names. `ui-workspace/tests/browser-styles.spec.ts` pins the gutter reservation on `.list`. +Three unit specs read the CSS text on disk. `ui-theme/tests/scrollbar-styles.spec.ts` scans the scrollbar token set out of `design-platform.css` rather than hardcoding it, so adding, renaming, or dropping a token moves the assertions with it, and checks that every token has a consumer and that each elevated surface rebinds a complete pair. It also pins the path split by source offset: the standard properties inside the gate block, the `::-webkit-scrollbar*` rules and every read of the hover indirection outside it. That split needs an offset assertion because the spec's rule parser flattens through at-rules, so a gate deleted or a declaration moved across it leaves every other assertion in the file green. -`apps/web/tests/sidebar-scrollbar.e2e.ts` covers the two facts only a real engine reports: the reserved band width, and the substituted `scrollbar-color`. It needs no model calls — the list only has to overflow — so it seeds cold sessions from an existing committed fixture read-only. +`apps/web/tests/sidebar-scrollbar.e2e.ts` covers the facts only a real engine reports: the reserved band width, and which rendering path the engine took. It needs no model calls — the list only has to overflow — so it seeds cold sessions from an existing committed fixture read-only. -Confirmed in headless chromium on the built client by reading computed values, which is what distinguishes a working token chain from a syntactically valid one: a scroll container computes the l1 thumb color in each palette, and a container that rebinds the indirection computes the l2 color, proving the rebind reaches the computed value rather than only the custom property. +Confirmed in headless chromium on the built client by reading computed values, which is what distinguishes a working token chain from a syntactically valid one: a scroll container computes the l1 thumb color in each palette, and a container that rebinds the indirection computes the l2 color, proving the rebind reaches the computed value rather than only the custom property. Firefox was verified the same way for the standard path, including the l1-to-l2 rebind on `scrollbar-color`; headless Firefox reports `scrollbar-width: none` on every element, styled or not, which is a headless artifact rather than an effect of the sheet. + +Two chromium measurement limits shape what the e2e can assert. The gate makes chromium report `scrollbar-width` and `scrollbar-color` as `auto`, so the substituted `scrollbar-color` is no longer the observable — the e2e asserts the `auto` reading deliberately, since a concrete value there would mean the gate leaked and silenced the pseudo-elements. And `getComputedStyle(el, '::-webkit-scrollbar-thumb')` folds in the `::-webkit-scrollbar-thumb:hover` rule, so it reports the hover color at rest and pins neither state; proven by deleting the hover rule through `CSSStyleSheet.deleteRule` in the live page, which flipped that same query from the hover color to the resting one. The e2e therefore reads the resting and hover colors as the indirection variables resolve on the list — one throwaway probe element per variable, because `getComputedStyle` returns a live declaration and a reused probe reports only the last value read — and reads the hover declaration out of the cascade as rule text. + +The gate itself has a negative control at the level it operates on: removing the `@supports` wrapper from the sheet, rebuilding `build:web`, and rerunning the e2e turns the `scrollbar-width: auto` assertion red with `thin`, which is the suppression the gate exists to prevent. Headless chromium draws overlay scrollbars, so a reserved gutter there does not shrink `clientWidth`. The reservation shows up as a non-zero `offsetWidth - clientWidth` band on the list; client-area geometry alone does not demonstrate it, and an assertion comparing the time element's right edge against the client-area edge holds with and without the reservation, so it would pass or fail on the platform's scrollbar style rather than on the declaration under test. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index ffad52e8dd..fcae03440f 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -18,7 +18,9 @@ Status: implemented `scrollbar-width` 与 `scrollbar-color` 声明在 `body, body *` 上，而不是只在顶层声明一次。继承传下去的是已经在 `body` 处代入完成的颜色值，因此后代元素重新绑定这层间接变量也无法改变自己的滚动条；逐元素重新声明使每个元素按它自己看到的取值代入变量。`scrollbar-width` 本身就不是可继承属性，无论如何都需要逐元素声明。`::-webkit-scrollbar*` 伪元素同样不继承，因此以不加限定的选择器匹配。 -两侧都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有四处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板与设置面板。后两者把声明写在抬升面板上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 +两种渲染互斥，而这种互斥是被强制的，不是假定的。`scrollbar-width` 或 `scrollbar-color` 只要取非 `auto` 值，Chromium 与 Safari 就会丢弃该元素上的全部 `::-webkit-scrollbar*` 规则，`::-webkit-scrollbar-thumb:hover` 也在其中。因此无条件地同时声明会让 hover token 在任何地方都得不到渲染：实现了 hover 伪元素的引擎，恰恰就是被标准属性静音的那些，而 Firefox 没有 hover 伪元素可作退路。于是标准属性写在 `@supports not selector(::-webkit-scrollbar)` 之内，该条件只在伪元素未被实现处为真，因此 Firefox 走标准属性路径，WebKit 系引擎走伪元素路径。WebKit 规则不再反向加门禁：不实现这些伪元素的引擎会把它们当作未知选择器丢弃，因此加门禁只是重述选择器匹配本身已经做的事。对于旧到不支持 `selector()` 函数的引擎，该条件无效，从而求值为假并选中伪元素路径——对于这条判断下现实存在的 16.4 之前的 Safari，这正是正确的一侧。 + +两条路径都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有四处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板与设置面板。后两者把声明写在抬升面板上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 轨道与两条滚动条相交的角落保持透明，因此滑块是以其下滚动的任何表面为背景被看到；只有滑块及其 hover 状态带 token 颜色。 @@ -34,6 +36,10 @@ Status: implemented **只声明一次，靠继承下传。** 匹配的元素更少，但它破坏重新绑定契约——继承携带的是代入后的颜色，而不是变量引用，因此抬升表面无法给自己的滚动条换色。它本身也不完整，因为 `scrollbar-width` 不继承。 +**不加 `@supports` 门禁，无条件同时声明标准属性与伪元素。** 这正是本次变更最初提交的形态，被评审发现。在 chromium 中于带 `scrollbar-gutter: stable`（使条带可观测）的探针元素上实测：单独一条 8px 的 `::-webkit-scrollbar` 预留出 30px 条带（样式表指定的宽度加上浏览器自带的按钮），而给同一元素加上 `scrollbar-width: thin` 后降到 `thin` 所预留的 10px——说明伪元素规则是被丢弃，而不是被合并。全部 `::-webkit-scrollbar-thumb:hover` 规则随之失效，因此两个 hover token 与四处抬升表面的 hover 重新绑定，在多数用户实际使用的引擎上都是死代码。 + +**给 WebKit 规则也加门禁，写成 `@supports selector(::-webkit-scrollbar)`。** 读起来对称，但在一个方向上是错的：它会对「实现了伪元素但不支持 `selector()`」的引擎隐藏这些规则，而那正是不加门禁时能被正确服务的 16.4 之前的 Safari。未知选择器本就会被丢弃，因此这道门禁不提供任何能抵偿该代价的保护。 + **改用内边距而不是预留空位（给 `.list` 加右内边距，或把 `.time` 向内移）。** 之所以否决：内边距无论滚动条是否存在都生效，因此在常见的短列表情形下白白占用横向空间；而且它只修好一个容器，其余每个滚动区域的内容仍然压在滚动条之下。 **给 `.list` 用 `scrollbar-gutter: auto`。** 空位在列表溢出时出现，也就是滚动条存在的时候。之所以否决：侧边栏的列表会随分组展开与收起而伸缩，因此空位会在用户光标之下出现又消失，并带动行一起位移。 @@ -42,17 +48,22 @@ Status: implemented - 客户端的每个滚动容器都绘制带主题的滑块：亮色基础表面为 `rgb(229, 229, 229)`，暗色基础表面为 `rgb(60, 60, 61)`，重新绑定到 l2 的暗色抬升表面为 `rgb(84, 85, 87)`。 - 两种渲染分别指定，因此改动滑块的几何或 hover 行为需要改两处：一处在 `scrollbar-width`／`scrollbar-color`，一处在伪元素。让两者都经由这组间接变量，把这份重复限制在 Firefox 与 WebKit 不共用的那些属性上。 +- hover token（`--dsw-alias-scrollbar-hover-l1`／`-l2`）只在伪元素路径上渲染。Firefox 通过 `scrollbar-color` 只表述一个滑块颜色，其 hover 表现由引擎自行推导，因此对 hover 颜色的设计改动在 Chromium 与 Safari 上可见，在 Firefox 上不可见。这是 `scrollbar-color` 本身的限制，不是这张样式表的限制。 - `body *` 匹配所有元素，涉及的两个属性其效果本就被浏览器限制在实际会滚动的元素上。代价是一个覆盖面很宽的选择器；另一种选择是一个不生效的重新绑定契约。 - 工作区列表在任何列表长度下都永久少了预留空位那一条宽度。这正是该修复换来的代价：以稳定的行几何，换掉只在列表较短时才可读的时间戳。 - 调色板中没有轨道 token，因此日后若设计需要不透明轨道，要新增一个别名 token，而不是在这张样式表里写字面颜色。 ## 测试 -三份单元测试读取磁盘上的 CSS 文本。`ui-theme/tests/scrollbar-styles.spec.ts` 从 `design-platform.css` 中扫描出滚动条 token 集合，而不是把它写死，因此新增、重命名或删除 token 时断言会随之变化；它检查每个 token 都有消费方，且每处抬升表面重新绑定的都是完整的一对。`web/tests/base-styles.spec.ts` 锁定导入顺序，以及 `base.css` 列出的每张样式表确实存在。`ui-workspace/tests/browser-styles.spec.ts` 锁定 `.list` 上的空位预留。 +三份单元测试读取磁盘上的 CSS 文本。`ui-theme/tests/scrollbar-styles.spec.ts` 从 `design-platform.css` 中扫描出滚动条 token 集合，而不是把它写死，因此新增、重命名或删除 token 时断言会随之变化；它检查每个 token 都有消费方，且每处抬升表面重新绑定的都是完整的一对。它还以源码偏移量锁定两条路径的划分：标准属性在门禁块之内，`::-webkit-scrollbar*` 规则与每一处对 hover 间接变量的读取都在门禁块之外。这个划分必须用偏移量断言，因为该测试文件的规则解析器会把 at-rule 拉平，所以删掉门禁或把某条声明移到门禁另一侧，文件里其余全部断言仍然是绿的。 -`apps/web/tests/sidebar-scrollbar.e2e.ts` 覆盖只有真实渲染引擎才能报告的两个事实：预留条带的宽度，以及代入后的 `scrollbar-color`。它不需要任何模型调用——列表只要溢出即可——因此以只读方式复用一份既有的已提交 fixture（测试前置数据）来铺入冷会话。 +`apps/web/tests/sidebar-scrollbar.e2e.ts` 覆盖只有真实渲染引擎才能报告的事实：预留条带的宽度，以及引擎实际走的是哪条渲染路径。它不需要任何模型调用——列表只要溢出即可——因此以只读方式复用一份既有的已提交 fixture（测试前置数据）来铺入冷会话。 -在构建产物客户端上于 headless chromium 中读取计算值确认，这正是区分「token 链真正生效」与「语法合法」的手段：滚动容器在两套调色板下分别计算出 l1 的滑块颜色，而重新绑定间接变量的容器计算出 l2 的颜色，证明重新绑定作用到了计算值，而不只是作用到自定义属性上。 +在构建产物客户端上于 headless chromium 中读取计算值确认，这正是区分「token 链真正生效」与「语法合法」的手段：滚动容器在两套调色板下分别计算出 l1 的滑块颜色，而重新绑定间接变量的容器计算出 l2 的颜色，证明重新绑定作用到了计算值，而不只是作用到自定义属性上。Firefox 的标准属性路径以同样方式做了验证，包含 `scrollbar-color` 上从 l1 到 l2 的重新绑定；headless Firefox 对任何元素（无论是否被样式命中）都报告 `scrollbar-width: none`，这是 headless 的产物，不是这张样式表造成的。 + +chromium 上有两处测量限制决定了 e2e 能断言什么。门禁使 chromium 报告的 `scrollbar-width` 与 `scrollbar-color` 都是 `auto`，因此代入后的 `scrollbar-color` 不再是可观测量——e2e 特意断言这个 `auto` 读数，因为此处出现具体值就意味着门禁泄漏、伪元素被静音。另外，`getComputedStyle(el, '::-webkit-scrollbar-thumb')` 会把 `::-webkit-scrollbar-thumb:hover` 规则一并折算进去，因此它在静止态就报告 hover 颜色，两种状态都锁不住；这一点由在运行中的页面里用 `CSSStyleSheet.deleteRule` 删掉 hover 规则得证——同一查询随之从 hover 颜色翻转为静止态颜色。因此 e2e 改为读取那组间接变量在列表上代入后的静止态与 hover 颜色（每个变量用一个一次性探针元素，因为 `getComputedStyle` 返回的是活的声明对象，复用探针只会报告最后一次读到的值），并把 hover 声明当作规则文本从层叠中读出。 + +门禁本身在它起作用的层面有反向对照：把样式表中的 `@supports` 包裹去掉、重新 `build:web`、再跑 e2e，`scrollbar-width: auto` 那条断言会以 `thin` 变红，而这正是门禁存在所要阻止的那种静音。 headless chromium 绘制的是覆盖式滚动条，因此其中预留空位不会缩小 `clientWidth`。该预留表现为列表上非零的 `offsetWidth - clientWidth` 条带；仅凭内容区几何无法证明它，而把时间元素右边缘与内容区右边缘做比较的断言，在有无预留的两种状态下都成立，因此它的通过或失败取决于平台的滚动条样式，而不是取决于被测的那条声明。 diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts index f54c0f9b2b..c761e27463 100644 --- a/apps/web/tests/sidebar-scrollbar.e2e.ts +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -12,12 +12,27 @@ // content) and never launches a replay row. A stray stream would fail loud // with NO_ADAPTER. // -// Headless-chromium caveat, load-bearing for what is asserted below: chromium -// paints an OVERLAY scrollbar that consumes no layout width. Comparing the -// time element's right edge against the list's client-area right edge -// therefore holds with and without the reservation and proves nothing; the -// reserved band width is the only layout signal that distinguishes the two -// states. See the assertions for which one is the control. +// Headless-chromium caveats, load-bearing for what is asserted below. +// +// Chromium paints an OVERLAY scrollbar that consumes no layout width, so +// comparing the time element's right edge against the list's client-area right +// edge holds with and without the reservation and proves nothing; the reserved +// band width is the only layout signal that distinguishes the two states. See +// the assertions for which one is the control. +// +// Chromium also takes the `::-webkit-scrollbar*` path, not the standard +// properties: scrollbar.css gates `scrollbar-width`/`scrollbar-color` behind +// `@supports not selector(::-webkit-scrollbar)`, which is false here. The +// resolved standard properties therefore read `auto`, and that reading is +// asserted — a concrete value would mean the gate leaked and silenced the +// pseudo-element rules. What the theme test measures instead is the pair the +// pseudo-element rules read: the indirection variables as they resolve ON the +// list, plus the `::-webkit-scrollbar-thumb:hover` declaration as it stands in +// the cascade. The hover thumb colour is not observable any other way — +// chromium folds the `:hover` rule into `getComputedStyle(el, +// '::-webkit-scrollbar-thumb')`, so that query reports the hover colour at +// rest and cannot pin either state (measured by deleting the hover rule live: +// the same query flipped from the hover colour to the resting one). import { readFile } from 'node:fs/promises' import { fileURLToPath } from 'node:url' import type { Browser, Page } from 'playwright' @@ -35,14 +50,20 @@ const SEED_COUNT = 24 interface ListMetrics { /** Resolved `scrollbar-gutter`. */ gutter: string - /** Resolved `scrollbar-width`. */ + /** Resolved `::-webkit-scrollbar` width: the pseudo-element path's own sizing. */ width: string - /** Resolved `scrollbar-color` (thumb then track). */ - color: string - /** The thumb half of `scrollbar-color`, split off the track half. */ - thumb: string - /** `--dsw-alias-scrollbar-bg-l1` resolved on the list into the same colour serialization `scrollbar-color` reports. */ + /** Resolved `::-webkit-scrollbar-track` background. */ + track: string + /** Resolved `scrollbar-width`, expected `auto` because the gate excludes chromium. */ + standardWidth: string + /** Resolved `scrollbar-color`, expected `auto` for the same reason. */ + standardColor: string + /** `::-webkit-scrollbar-thumb:hover` background declarations found in the cascade, in sheet order. */ + hoverRules: string[] + /** `--dsh-scrollbar-thumb` resolved on the list, serialized as a colour. */ token: string + /** `--dsh-scrollbar-thumb-hover` resolved on the list, serialized the same way. */ + hoverToken: string /** True when the list actually scrolls. */ overflows: boolean /** Border-box width minus client width: the space the scrollbar takes out of the content area. */ @@ -66,27 +87,47 @@ function measureList(page: Page): Promise { if (list === null) throw new Error('sidebar session list not in the DOM') const time = list.querySelector('[class*="time"]') if (time === null) throw new Error('no row relative-time element in the sidebar list') - // The token needs the same serialization `scrollbar-color` reports: the - // palette sheet writes it in whatever notation it chose, so it is - // resolved through a probe element's `color`. The probe is appended to - // the list so `var()` substitution happens where the list sits in the - // cascade — the token reaching THIS element is the claim. - const probe = document.createElement('span') - list.append(probe) - probe.style.color = 'var(--dsw-alias-scrollbar-bg-l1)' - const token = getComputedStyle(probe).color - probe.remove() + // Each indirection variable is resolved through its own throwaway probe + // appended to the list: `var()` substitution then happens where the list + // sits in the cascade, which is the claim, and `color` normalizes whatever + // notation the palette sheet chose into one comparable serialization. A + // REUSED probe would report only the last value read — `getComputedStyle` + // returns a live declaration, so reassigning `style.color` retroactively + // changes every earlier read. + const resolve = (name: string): string => { + const probe = document.createElement('span') + probe.style.color = `var(${name})` + list.append(probe) + const value = getComputedStyle(probe).color + probe.remove() + return value + } + // The hover colour is read out of the cascade rather than computed: + // chromium reports the `:hover` background for the resting pseudo-element + // too (see the file header), so no computed query separates the states. + // Cross-origin sheets throw on `cssRules`; none is expected, and skipping + // them cannot mask the rule under test, which ships in the app's own CSS. + const hoverRules = [...document.styleSheets] + .flatMap((sheet) => { + try { + return [...sheet.cssRules] + } catch { + return [] + } + }) + .filter((rule): rule is CSSStyleRule => rule instanceof CSSStyleRule) + .filter(rule => rule.selectorText === '::-webkit-scrollbar-thumb:hover') + .map(rule => rule.style.getPropertyValue('background')) const style = getComputedStyle(list) - // `scrollbar-color` serializes as ` `; both halves are - // functional colours, so the split is on the space before the track's - // opening token, not on every space. - const thumb = style.scrollbarColor.replace(/\s+rgba?\([^)]*\)$/, '') return { gutter: style.scrollbarGutter, - width: style.scrollbarWidth, - color: style.scrollbarColor, - thumb, - token, + width: getComputedStyle(list, '::-webkit-scrollbar').width, + track: getComputedStyle(list, '::-webkit-scrollbar-track').backgroundColor, + standardWidth: style.scrollbarWidth, + standardColor: style.scrollbarColor, + hoverRules, + token: resolve('--dsh-scrollbar-thumb'), + hoverToken: resolve('--dsh-scrollbar-thumb-hover'), overflows: list.scrollHeight > list.clientHeight, band: list.getBoundingClientRect().width - list.clientWidth, clientRight: list.getBoundingClientRect().left + list.clientWidth, @@ -170,28 +211,38 @@ describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thum expect(tripwire.pageErrors).toEqual([]) }, 60_000) - it('resolves the themed thumb colour on the list in both palettes', async () => { + it('renders the themed thumb through the WebKit path in both palettes', async () => { onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-theme')) const light = await measureList(page) - // `thin`, not `auto`: the sheet's per-element declaration reached a - // container it never names. - expect(light.width).toBe('thin') - // A concrete colour, not `auto`, and byte-equal to the alias token - // resolved on this element: the indirection carried the token here rather - // than falling back to the UA thumb. - expect(light.color).not.toBe('auto') - expect(light.thumb).toBe(light.token) - // Transparent track, so the thumb reads against the scrolling surface. - expect(light.color.endsWith('rgba(0, 0, 0, 0)')).toBe(true) + // The gate's signature on this engine, and the reason it exists: chromium + // implements `::-webkit-scrollbar`, so the standard properties stay at + // their initial `auto`. A concrete value here would mean the gate leaked, + // which is exactly what makes chromium discard the pseudo-element rules — + // the hover token included. + expect(light.standardWidth).toBe('auto') + expect(light.standardColor).toBe('auto') + // The pseudo-element path is the one in force: the sheet's own 8px sizing + // and transparent track reached a container it never names. + expect(light.width).toBe('8px') + expect(light.track).toBe('rgba(0, 0, 0, 0)') + // The resting and the hover rule each read the rebindable indirection, and + // the two resolve to DIFFERENT colours on this list: the l1 pair arrived + // here intact rather than collapsing to one value or falling back. + expect(light.hoverRules).toEqual(['var(--dsh-scrollbar-thumb-hover)']) + expect(light.token).toMatch(/^rgba?\(/) + expect(light.hoverToken).not.toBe(light.token) // The dark palette declares different scrollbar tokens; driving the body // attribute pins the cascade the way lifecycle-chrome does (the Settings // gesture that sets it is owned there). await page.evaluate(() => { document.body.setAttribute('data-ds-dark-theme', '') }) const dark = await measureList(page) - expect(dark.thumb).toBe(dark.token) - expect(dark.thumb).not.toBe(light.thumb) + expect(dark.token).not.toBe(light.token) + expect(dark.hoverToken).not.toBe(dark.token) + expect(dark.hoverToken).not.toBe(light.hoverToken) await page.evaluate(() => { document.body.removeAttribute('data-ds-dark-theme') }) - expect((await measureList(page)).thumb).toBe(light.thumb) + const restored = await measureList(page) + expect(restored.token).toBe(light.token) + expect(restored.hoverToken).toBe(light.hoverToken) expect(tripwire.pageErrors).toEqual([]) }, 60_000) diff --git a/packages/client/ui-theme/README.i18n.yaml b/packages/client/ui-theme/README.i18n.yaml index cebdba55d0..cc5c4ec07b 100644 --- a/packages/client/ui-theme/README.i18n.yaml +++ b/packages/client/ui-theme/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/client/ui-theme/README.md -README.md: 9bf232d506c599a6302c04d5769b43993d84dbf6 -README.zh.md: 84dba38d751b74c13f4af42c40484995900dfc12 +README.md: a1ff7d840dae86f5da98de1208ecda3b8b62026b +README.zh.md: 49b52bcb1e07527e98c602086404228c5513091a diff --git a/packages/client/ui-theme/README.md b/packages/client/ui-theme/README.md index 9bf232d506..a1ff7d840d 100644 --- a/packages/client/ui-theme/README.md +++ b/packages/client/ui-theme/README.md @@ -6,7 +6,9 @@ Theme plugin: ThemeService over the --dsw-* token base stylesheets (static scale `src/styles/` holds five sheets, all imported by the web shell's `base.css`: `base.css`, `design-platform.css`, `scrollbar.css`, `gradient-shadow-text.css`, and `shiki.css`. `scrollbar.css` is the sole consumer of the `--dsw-alias-scrollbar-*` tokens and must follow `design-platform.css`, which declares them. -Scrollbar rebinding contract: `scrollbar.css` binds `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover` on `body` to the l1 (base-surface) tokens, and both the standard `scrollbar-color` and the `::-webkit-scrollbar-thumb` rules read that pair. An elevated surface (menu, popover, dialog) sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container; one rebind retints both renderings. Reasoning and the measured computed values: [the scrollbar Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md). +Scrollbar rebinding contract: `scrollbar.css` binds `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover` on `body` to the l1 (base-surface) tokens, and both rendering paths read that pair. An elevated surface (menu, popover, dialog) sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container; one rebind retints whichever path the engine took. + +The two paths are mutually exclusive by construction. `scrollbar-width`/`scrollbar-color` sit inside `@supports not selector(::-webkit-scrollbar)` because a non-`auto` value of either makes Chromium and Safari discard every `::-webkit-scrollbar*` rule for that element, `::-webkit-scrollbar-thumb:hover` included — declaring both unconditionally leaves `--dsh-scrollbar-thumb-hover` with no rendering anywhere. Firefox therefore takes the standard properties and WebKit-based engines take the pseudo-elements, so the hover token only ever renders through the pseudo-element path. Reasoning and the measured computed values: [the scrollbar Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md). ## Model Experience diff --git a/packages/client/ui-theme/README.zh.md b/packages/client/ui-theme/README.zh.md index 84dba38d75..49b52bcb1e 100644 --- a/packages/client/ui-theme/README.zh.md +++ b/packages/client/ui-theme/README.zh.md @@ -6,7 +6,9 @@ `src/styles/` 下有五张样式表，全部由 web 壳的 `base.css` 导入：`base.css`、`design-platform.css`、`scrollbar.css`、`gradient-shadow-text.css` 与 `shiki.css`。`scrollbar.css` 是 `--dsw-alias-scrollbar-*` token 的唯一消费方，必须排在声明这些 token 的 `design-platform.css` 之后。 -滚动条重新绑定契约：`scrollbar.css` 在 `body` 上把 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover` 绑定到 l1（基础表面）token，标准属性 `scrollbar-color` 与 `::-webkit-scrollbar-thumb` 规则都读取这一组变量。抬升表面（菜单、浮层、对话框）在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`；一次重新绑定即可为两种渲染同时换色。推理过程与实测计算值见[滚动条 Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md)。 +滚动条重新绑定契约：`scrollbar.css` 在 `body` 上把 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover` 绑定到 l1（基础表面）token，两条渲染路径都读取这一组变量。抬升表面（菜单、浮层、对话框）在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`；一次重新绑定即可为引擎实际走的那条路径换色。 + +两条路径在构造上互斥。`scrollbar-width`／`scrollbar-color` 写在 `@supports not selector(::-webkit-scrollbar)` 之内，因为这两个属性只要取非 `auto` 值，Chromium 与 Safari 就会丢弃该元素上的全部 `::-webkit-scrollbar*` 规则，`::-webkit-scrollbar-thumb:hover` 也在其中——若无条件地同时声明，`--dsh-scrollbar-thumb-hover` 在任何引擎上都不会被渲染。因此 Firefox 走标准属性，WebKit 系引擎走伪元素，hover token 只经由伪元素这条路径渲染。推理过程与实测计算值见[滚动条 Agent Note](../../../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md)。 ## 模型体验 diff --git a/packages/client/ui-theme/src/styles/scrollbar.css b/packages/client/ui-theme/src/styles/scrollbar.css index 4aea10efab..31b5a8aabe 100644 --- a/packages/client/ui-theme/src/styles/scrollbar.css +++ b/packages/client/ui-theme/src/styles/scrollbar.css @@ -2,50 +2,69 @@ * tokens. Without it every scrolling region renders the UA scrollbar, which * ignores the theme — a light native bar over the dark palette. * - * The rule sits on `body`, not `html`: design-platform.css declares the + * The rules sit on `body`, not `html`: design-platform.css declares the * --dsw-alias-* tokens on `body` (and the dark overrides on * `body[data-ds-dark-theme]`), and custom properties only inherit downward, * so an `html` rule resolves them to the guaranteed-invalid value and * `scrollbar-color` falls back to `auto`. * - * `scrollbar-color` is an inherited property, so binding it once on `body` - * reaches every scroll container without enumerating module class names. - * `scrollbar-width` is NOT inherited, so it is applied to all elements. - * The WebKit pseudo-elements are not inherited either, hence the unscoped - * `::-webkit-scrollbar` rules. - * * Surfaces pick their elevation by rebinding --dsh-scrollbar-thumb{,-hover}: * the l1 pair here is the base-surface default, and an elevated surface * (menu, popover, dialog) rebinds to the l2 pair on its own container. Both - * the standard properties and the WebKit pseudo-elements read the - * indirection, so one rebind reaches both renderings. */ + * rendering paths below read the indirection, so one rebind reaches whichever + * path the engine took. */ body { --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l1); --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l1); } -/* `scrollbar-color` and `scrollbar-width` are declared on every element - rather than inherited from `body`. Inheriting would pass down the COLOUR - already substituted at `body`, so a descendant rebinding - --dsh-scrollbar-thumb could not change it; re-declaring makes each element - substitute the variable as it sees it, which is what gives an elevated - surface a working rebind. `scrollbar-width` is not an inherited property - at all, so it needs the per-element declaration regardless. +/* The two paths are mutually exclusive, and the gate is load-bearing rather + than defensive. A non-`auto` `scrollbar-width` or `scrollbar-color` makes + Chromium and Safari drop every `::-webkit-scrollbar*` rule for that + element, including `::-webkit-scrollbar-thumb:hover` — measured in chromium + as an 8px `::-webkit-scrollbar` width taking effect on its own and being + ignored as soon as `scrollbar-width: thin` is added. Declaring both + unconditionally therefore leaves the hover tokens with no rendering at all, + because the engines that implement the hover pseudo-element are exactly the + ones the standard properties silence, and Firefox has no hover + pseudo-element to fall back on. - Track stays transparent so the thumb reads against whatever surface - scrolls under it; only the thumb carries a token colour. */ -body, -body * { - scrollbar-width: thin; - scrollbar-color: var(--dsh-scrollbar-thumb) transparent; + `not selector(::-webkit-scrollbar)` is true only where the pseudo-element + is unimplemented, so Firefox takes the standard path and WebKit-based + engines take the pseudo-element path. An engine too old for the + `selector()` function makes the condition invalid, which evaluates false + and selects the pseudo-element path — the correct side for the pre-16.4 + Safari that is the realistic case. */ +@supports not selector(::-webkit-scrollbar) { + /* Declared on every element rather than inherited from `body`. Inheriting + would pass down the COLOUR already substituted at `body`, so a descendant + rebinding --dsh-scrollbar-thumb could not change it; re-declaring makes + each element substitute the variable as it sees it, which is what gives + an elevated surface a working rebind. `scrollbar-width` is not an + inherited property at all, so it needs the per-element declaration + regardless. + + No hover counterpart exists on this path: `scrollbar-color` states one + thumb colour and the engine derives its own hover treatment. */ + body, + body * { + scrollbar-width: thin; + scrollbar-color: var(--dsh-scrollbar-thumb) transparent; + } } +/* Not gated in turn: an engine that does not implement these pseudo-elements + drops the rules as unknown selectors, so the gate would only restate what + selector matching already does. Not inherited either, hence the unscoped + selectors. */ ::-webkit-scrollbar { width: 8px; height: 8px; } +/* Track stays transparent so the thumb reads against whatever surface scrolls + under it; only the thumb carries a token colour. */ ::-webkit-scrollbar-track { background: transparent; } diff --git a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts index a53a19eecf..0e1a5c94c1 100644 --- a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts +++ b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts @@ -58,6 +58,27 @@ function parseRules(css: string): CssRule[] { return rules } +/** + * Half-open source span of one at-rule's block, excluding its prelude. + * @param css - stylesheet text. + * @param prelude - exact at-rule prelude to locate, without the opening brace. + * @returns the block's brace offsets, or undefined when the prelude is absent. + */ +function atRuleBlock(css: string, prelude: string): { start: number; end: number } | undefined { + const opening = css.indexOf(`${prelude} {`) + if (opening === -1) return undefined + const start = css.indexOf('{', opening) + let depth = 0 + for (let index = start; index < css.length; index += 1) { + if (css[index] === '{') depth += 1 + else if (css[index] === '}') { + depth -= 1 + if (depth === 0) return { start, end: index } + } + } + throw new Error(`unbalanced braces after ${prelude}`) +} + /** * Custom-property names a value reads. * @param value - declaration value, possibly with nested var() calls. @@ -265,6 +286,60 @@ describe('scrollbar.css selectors', () => { }) }) +describe('scrollbar.css rendering paths', () => { + /** The gate prelude, spelled exactly as the sheet must spell it for the split to exist. */ + const GATE = '@supports not selector(::-webkit-scrollbar)' + const withoutComments = scrollbarCss.replace(/\/\*[\s\S]*?\*\//g, ' ') + const gate = atRuleBlock(withoutComments, GATE) + /** Standard scrollbar properties, the ones whose non-`auto` values suppress the pseudo-elements. */ + const STANDARD_PROPERTIES = ['scrollbar-width', 'scrollbar-color'] + + it('gates the standard properties behind the absence of the WebKit pseudo-element', () => { + // A non-`auto` scrollbar-width or scrollbar-color makes Chromium and + // Safari discard every ::-webkit-scrollbar* rule for that element, + // ::-webkit-scrollbar-thumb:hover included. Declaring both paths + // unconditionally therefore renders the hover token nowhere: the engines + // implementing the hover pseudo-element are exactly the ones the standard + // properties silence, and Firefox has no hover pseudo-element at all. + expect(gate, GATE).toBeDefined() + for (const property of STANDARD_PROPERTIES) { + const offsets = [...withoutComments.matchAll(new RegExp(String.raw`(^|[;{\s])${property}\s*:`, 'g'))] + .map(match => match.index) + expect(offsets.length, property).toBeGreaterThan(0) + for (const offset of offsets) { + expect(offset, `${property} outside ${GATE}`).toBeGreaterThan(gate!.start) + expect(offset, `${property} outside ${GATE}`).toBeLessThan(gate!.end) + } + } + }) + + it('leaves the WebKit pseudo-element rules outside the gate', () => { + // Gating these in turn would only restate selector matching: an engine + // without the pseudo-elements drops the rules as unknown selectors. Inside + // the gate they would be dropped by the engines that do implement them, + // which is every engine that can render them. + const offsets = [...withoutComments.matchAll(/::-webkit-scrollbar/g)] + .map(match => match.index) + .filter(offset => withoutComments.slice(offset).search(/^[\w:-]*\s*[,{]/) === 0) + expect(offsets.length).toBeGreaterThan(0) + for (const offset of offsets) { + expect(offset > gate!.start && offset < gate!.end, `::-webkit-scrollbar rule inside ${GATE}`).toBe(false) + } + }) + + it('renders the hover token only through the pseudo-element path', () => { + // The standard path has no hover counterpart — scrollbar-color states one + // thumb colour and the engine derives its own hover treatment — so the + // hover indirection has to be read outside the gate or it renders nowhere. + const hoverOffsets = [...withoutComments.matchAll(new RegExp(String.raw`var\(\s*${INDIRECTION_PREFIX}thumb-hover`, 'g'))] + .map(match => match.index) + expect(hoverOffsets.length).toBeGreaterThan(0) + for (const offset of hoverOffsets) { + expect(offset > gate!.start && offset < gate!.end, 'hover indirection read inside the gate').toBe(false) + } + }) +}) + describe('elevated surface rebinds', () => { it('at least one surface rebinds the indirection', () => { expect(rebindRules.length).toBeGreaterThan(0) From 17419aa6b9f92971b635fd7fac4c1e24fa825a4e Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 17:22:09 +0800 Subject: [PATCH 5/8] test(web): assert the timestamp occlusion the sidebar gutter fixes The e2e measured the reserved band but never the symptom the change is named for. Headless chromium defaults to an overlay scrollbar, which is the configuration where a bar can cover row content at all, so the scenario already ran in the right mode: against clean master the band is 0 and the bar covers 7px of the relative time. Adds timeCoveredBy, the overlap between the relative time's right edge and the range the bar occupies, taking the bar's width from the sheet where it applies and from the UA's overlay width otherwise. Assuming 0 there would report no occlusion in precisely the state that has it. Keeps the band assertion rather than replacing it: the two catch different regressions. Removing only scrollbar-gutter leaves timeCoveredBy at 0, because the bar is then 8px and the row's right padding is also 8px, so it abuts the timestamp without covering it. Removing the pseudo-element width as well is what produces the overlap. Each was mutation-checked with the other assertions in its test silenced. Records in the note that the gutter and the ::-webkit-scrollbar width are jointly necessary against an overlay bar, measured by deleting each from the live cascade with the other in force: either alone drops the band from 8 to 0. --- ...d-scrollbars-and-reserved-gutter.i18n.yaml | 4 +- ...8-themed-scrollbars-and-reserved-gutter.md | 6 +- ...hemed-scrollbars-and-reserved-gutter.zh.md | 6 +- apps/web/tests/sidebar-scrollbar.e2e.ts | 72 ++++++++++++++++--- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index f841a008ea..f20ff46ec8 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: 71d2e5b5156d3bc54968552aa2eef82fab2ff443 -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: fcae03440ff621d949f8158990e2d871590b7daa +2026-07-28-themed-scrollbars-and-reserved-gutter.md: c52440bb057c5202ee90bcf53518fe62ba1379b7 +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: e234106c47eeaf418aac4a09ced0dd6c854a439c diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index 71d2e5b515..c52440bb05 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -26,6 +26,8 @@ The track and the corner stay transparent, so the thumb reads against whatever s `.list` declares `scrollbar-gutter: stable`, which keeps the bar beside the rows instead of on top of them. `stable` rather than `auto` because `auto` reserves the gutter only while the list actually overflows: expanding a workspace group would then shift every row horizontally at the moment it starts scrolling. The reservation is unconditional and the rows never move. +The gutter and the sheet's `::-webkit-scrollbar` width are jointly necessary against an overlay scrollbar, which is the configuration where the symptom exists at all. Measured on the running app by deleting each from the live cascade with the other left in force: either deletion alone takes the list's band from 8 to 0. The gutter states that space be reserved, and the pseudo-element width is what makes chromium treat the bar as occupying layout space rather than floating over the content. Neither half of this change is therefore optional for the reported bug, which is a second reason the two halves ship together. + ## Alternatives considered **Per-module `::-webkit-scrollbar` rules in each scrolling component sheet.** Rejected: the client has thirteen scrolling containers across nine packages, every one would carry the same block, and the fourteenth would ship unthemed with nothing failing. A skin driven by design tokens belongs in the package that owns the tokens. @@ -65,6 +67,8 @@ Two chromium measurement limits shape what the e2e can assert. The gate makes ch The gate itself has a negative control at the level it operates on: removing the `@supports` wrapper from the sheet, rebuilding `build:web`, and rerunning the e2e turns the `scrollbar-width: auto` assertion red with `thin`, which is the suppression the gate exists to prevent. -Headless chromium draws overlay scrollbars, so a reserved gutter there does not shrink `clientWidth`. The reservation shows up as a non-zero `offsetWidth - clientWidth` band on the list; client-area geometry alone does not demonstrate it, and an assertion comparing the time element's right edge against the client-area edge holds with and without the reservation, so it would pass or fail on the platform's scrollbar style rather than on the declaration under test. +Headless chromium draws overlay scrollbars, and that is the configuration in which the reported symptom exists, so the e2e reproduces the bug rather than approximating it: against clean master the list's band is 0 and the bar covers 7px of the relative time. A reserved gutter there does not shrink `clientWidth`, so an assertion comparing the time element's right edge against the client-area edge holds with and without the reservation and would pass or fail on the platform's scrollbar style rather than on the declaration under test. The two signals that do separate the states are the `offsetWidth - clientWidth` band and `timeCoveredBy`, the overlap measured against the bar's own width. + +Both are asserted because each catches a different regression, established by mutating one declaration at a time with the other assertions in that test silenced. Removing only the gutter leaves `timeCoveredBy` at 0 — the bar is then 8px and the row's right padding is also 8px, so it abuts the timestamp without covering it — and the band assertion is what fails. Removing the pseudo-element width as well, which is the actual master state, produces the overlap, and `timeCoveredBy` fails at 7. A headed run under xvfb cannot show the symptom in either state, because chromium paints a classic space-consuming bar there and `clientWidth` already excludes it. Verifying browser-visible plugin CSS needs a rebuild `pnpm run build:web` does not perform. `WorkspaceBrowser.module.css` never reaches `apps/web/dist`: ui-workspace loads as a runtime plugin and its CSS is inlined into `packages/client/ui-workspace/lib/client.js`, built by that package's own `bundle` script. A negative control that reruns only `build:web` therefore exercises a stale bundle and passes with the declaration removed, which reads as a vacuous test rather than as an invalid control. Rebuild with `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`, confirm the artifact by grepping `lib/client.js` for the declaration, then `build:web`. No script in the web lane does this: `test:web` runs `build:web` alone, so every scroll-region or plugin-CSS change hits the same trap. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index fcae03440f..e234106c47 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -26,6 +26,8 @@ Status: implemented `.list` 声明 `scrollbar-gutter: stable`，使滚动条位于行的旁边而非行的上方。取 `stable` 而非 `auto`，因为 `auto` 只在列表确实溢出时才预留空位：那样展开一个工作区分组时，所有行会在列表开始滚动的那一刻发生水平位移。`stable` 的预留是无条件的，行不会移动。 +面对覆盖式滚动条——也就是这个症状唯一存在的那种形态——空位声明与样式表里的 `::-webkit-scrollbar` 宽度是共同必要的。在运行中的应用上实测：保留其中一条、从活的层叠中删掉另一条，任意一次单独删除都会让列表的条带从 8 降到 0。空位声明表述的是「要预留空间」，而伪元素宽度才是让 chromium 把滚动条视为占据布局空间、而不是浮在内容之上的原因。因此对这个 bug 而言，本次变更的两半都不是可选项，这也是两半必须一起交付的第二个理由。 + ## 曾考虑的替代方案 **在每个滚动组件的样式表里各写一份 `::-webkit-scrollbar` 规则。** 之所以否决：客户端共有分布在九个包中的十三个滚动容器，每一个都要带上同一段规则，而第十四个会在没有任何门禁报错的情况下漏掉主题。由设计 token 驱动的皮肤应当归属于拥有这些 token 的包。 @@ -65,6 +67,8 @@ chromium 上有两处测量限制决定了 e2e 能断言什么。门禁使 chrom 门禁本身在它起作用的层面有反向对照：把样式表中的 `@supports` 包裹去掉、重新 `build:web`、再跑 e2e，`scrollbar-width: auto` 那条断言会以 `thin` 变红，而这正是门禁存在所要阻止的那种静音。 -headless chromium 绘制的是覆盖式滚动条，因此其中预留空位不会缩小 `clientWidth`。该预留表现为列表上非零的 `offsetWidth - clientWidth` 条带；仅凭内容区几何无法证明它，而把时间元素右边缘与内容区右边缘做比较的断言，在有无预留的两种状态下都成立，因此它的通过或失败取决于平台的滚动条样式，而不是取决于被测的那条声明。 +headless chromium 绘制的是覆盖式滚动条，而这恰好就是被报告症状存在的那种形态，因此这个 e2e 复现的是这个 bug 本身，而不是它的近似：在干净的 master 上，列表条带为 0，滚动条盖住相对时间 7px。其中预留空位不会缩小 `clientWidth`，因此把时间元素右边缘与内容区右边缘做比较的断言在有无预留的两种状态下都成立，它的通过或失败取决于平台的滚动条样式，而不是取决于被测的那条声明。真正能区分两种状态的两个量是 `offsetWidth - clientWidth` 条带，以及以滚动条自身宽度为基准量出的重叠量 `timeCoveredBy`。 + +两者都要断言，因为各自捕捉的是不同的回归；这一点通过每次只改动一条声明、并把同一个测试里的其余断言静音来确定。只删掉空位声明时 `timeCoveredBy` 仍为 0——此时滚动条是 8px，而行的右内边距也是 8px，于是它紧贴时间戳但并未盖住——失败的是条带那条断言。再把伪元素宽度也删掉（这才是 master 的真实状态）才会产生重叠，此时 `timeCoveredBy` 以 7 变红。在 xvfb 下的有头运行无论哪种状态都看不到这个症状，因为 chromium 在那里画的是经典占位滚动条，`clientWidth` 本来就已经把它排除了。 验证浏览器可见的插件 CSS 需要一次 `pnpm run build:web` 并不执行的重建。`WorkspaceBrowser.module.css` 从不进入 `apps/web/dist`：ui-workspace 以运行时插件方式加载，其 CSS 内联进 `packages/client/ui-workspace/lib/client.js`，由该包自己的 `bundle` 脚本构建。因此只重跑 `build:web` 的反向对照实际测的是旧产物，去掉声明后仍会通过，看起来像测试无效，实际是对照无效。正确做法是先 `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`，用 grep 在 `lib/client.js` 中确认该声明确实存在或消失，然后再 `build:web`。web 通道中没有任何脚本会做这一步：`test:web` 只运行 `build:web`，因此任何滚动区域或插件 CSS 的改动都会碰到同一个陷阱。 diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts index c761e27463..6dfebb374c 100644 --- a/apps/web/tests/sidebar-scrollbar.e2e.ts +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -14,11 +14,35 @@ // // Headless-chromium caveats, load-bearing for what is asserted below. // -// Chromium paints an OVERLAY scrollbar that consumes no layout width, so -// comparing the time element's right edge against the list's client-area right -// edge holds with and without the reservation and proves nothing; the reserved -// band width is the only layout signal that distinguishes the two states. See -// the assertions for which one is the control. +// Headless chromium defaults to an OVERLAY scrollbar: one drawn on top of the +// content, consuming no layout width unless something reserves space. That is +// the mode in which the reported symptom exists at all, so this environment +// reproduces it rather than merely approximating it — measured against clean +// master, where the list's band is 0 and the bar covers 7px of the relative +// time. (Under a classic space-consuming bar, `clientWidth` already excludes +// the bar and nothing can be covered; a headed run under xvfb behaves that way +// and cannot show the symptom.) +// +// The consequence for assertions: comparing the time element's right edge +// against the list's CLIENT-area right edge holds in both states and proves +// nothing, because with an overlay bar the client edge is the border edge. The +// two signals that do separate the states are the reserved band width and +// `timeCoveredBy`, which measures the overlap against the bar's own width. +// +// Both the `scrollbar-gutter: stable` reservation and the sheet's +// `::-webkit-scrollbar` width are needed for that band, and neither suffices: +// measured on the running app, deleting either one takes the band from 8 to 0 +// while the other stays in force. The gutter states that space be reserved; the +// pseudo-element width is what makes chromium treat the bar as occupying layout +// space in the first place. +// +// That conjunction is why `band` and `timeCoveredBy` are both asserted and +// neither replaces the other. Removing only the gutter leaves `timeCoveredBy` at +// 0, because the bar is then 8px wide and the row's right padding is also 8px, +// so it abuts the timestamp without covering it; `band` catches that case. +// Removing both — the actual master state — is what produces the reported +// overlap, and `timeCoveredBy` measures it at 7. Each was mutation-checked with +// the other assertions in its test silenced. // // Chromium also takes the `::-webkit-scrollbar*` path, not the standard // properties: scrollbar.css gates `scrollbar-width`/`scrollbar-color` behind @@ -74,6 +98,14 @@ interface ListMetrics { borderRight: number /** Right edge of the first row's relative-time element, the content the unreserved bar covered. */ timeRight: number + /** + * Pixels of the relative time the scrollbar paints over: how far its right + * edge reaches into the band the bar occupies, `[borderRight - barWidth, + * borderRight]`. This is the reported symptom as a number, and it is the one + * geometric signal that separates the two states in this environment — see + * the file header on why `clientWidth` comparisons cannot. + */ + timeCoveredBy: number } /** @@ -119,9 +151,11 @@ function measureList(page: Page): Promise { .filter(rule => rule.selectorText === '::-webkit-scrollbar-thumb:hover') .map(rule => rule.style.getPropertyValue('background')) const style = getComputedStyle(list) + const pseudoWidth = getComputedStyle(list, '::-webkit-scrollbar').width + const barWidth = pseudoWidth === 'auto' ? 15 : Number.parseFloat(pseudoWidth) return { gutter: style.scrollbarGutter, - width: getComputedStyle(list, '::-webkit-scrollbar').width, + width: pseudoWidth, track: getComputedStyle(list, '::-webkit-scrollbar-track').backgroundColor, standardWidth: style.scrollbarWidth, standardColor: style.scrollbarColor, @@ -133,6 +167,14 @@ function measureList(page: Page): Promise { clientRight: list.getBoundingClientRect().left + list.clientWidth, borderRight: list.getBoundingClientRect().right, timeRight: time.getBoundingClientRect().right, + // The bar is drawn in the rightmost `barWidth` of the border box, whether + // or not that space was reserved. Its width comes from the sheet where the + // sheet applies, and from the UA's own overlay bar otherwise — 15px is + // what this chromium paints, measured against master where the rule is + // absent. Taking the UA width as the fallback is what keeps the assertion + // honest: assuming 0 there would report no occlusion precisely in the + // state that has it. + timeCoveredBy: Math.max(0, time.getBoundingClientRect().right - (list.getBoundingClientRect().right - barWidth)), } }) } @@ -201,11 +243,19 @@ describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thum // drawn over it. Removing the declaration makes it exactly 0. The value // itself is not pinned — it tracks `scrollbar-width` and the platform. expect(metrics.band).toBeGreaterThan(0) - // With the band reserved, the row's relative time — flush against the - // row's right padding, the element the unreserved bar covered — ends - // inside the content area, clear of the bar. Alone this would be vacuous - // under chromium's overlay scrollbar (see the file header); it is - // meaningful only conjoined with the band assertion above. + // The reported symptom, stated directly: no part of the row's relative time + // lies under the bar. Measures 7 on clean master — the `h` of `1h` is the + // covered part. Unlike the client-edge comparison below it does not go + // vacuous under an overlay scrollbar, because it measures against the bar's + // own width rather than against a content edge the overlay bar does not + // move. It is not a replacement for the band assertion above; see the file + // header for which regression each one catches. + expect(metrics.timeCoveredBy).toBe(0) + // Corollaries of the reservation, kept because they pin where the band sits + // rather than only that it exists: the time ends inside the content area, + // and the content area ends before the border box. Each holds in both + // states on its own (see the file header) and is meaningful only alongside + // the two assertions above. expect(metrics.timeRight).toBeLessThanOrEqual(metrics.clientRight) expect(metrics.clientRight).toBeLessThan(metrics.borderRight) expect(tripwire.pageErrors).toEqual([]) From 4265ac876c6cd9eb43fc925210882b77751ecb37 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 17:42:57 +0800 Subject: [PATCH 6/8] fix(client): rebind the scrollbar indirection on three missed elevated surfaces Review found three scroll containers sitting on surfaces the rebinding contract covers, none of which rebound: ui-primitives' shared Menu card on --dsw-specific-menu (the surface PopupSelectView already rebinds for), and the composer input and question composer cards, both on --dsw-specific-input-major. Each rendered the l1 thumb, which differs from l2 only in the dark palette and only on that surface, so a light-palette screenshot and a code read both look correct. Adds the mechanical check that would have caught them instead of leaving it to inspection: a sheet that scrolls somewhere and paints a known elevated surface somewhere must rebind. The elevated set is derived from the sheets that already rebind, since a rebinding rule paints the surface whose elevation it declares, so a new elevated surface joins the set by rebinding rather than by anyone updating a list. Surface-level rather than element-level because the card and the descendant that scrolls are separate rules and CSS text does not say which contains which. Verified by reverting each of the three fixes in turn: the check names the sheet and the surface every time. Also commits snapshots/sidebar-scrollbar/geometry.expected.md, the resolved scrollbar style and geometry in both palettes. The aria goldens the other web scenarios commit cannot carry a CSS-only change, since it alters no DOM and no accessible name and leaves their trees byte-identical. Absolute coordinates stay out: they track font metrics and the laid-out sidebar width, so committing them would document the platform and force a per-platform re-record. --- ...d-scrollbars-and-reserved-gutter.i18n.yaml | 4 +- ...8-themed-scrollbars-and-reserved-gutter.md | 6 +- ...hemed-scrollbars-and-reserved-gutter.zh.md | 6 +- apps/web/tests/sidebar-scrollbar.e2e.ts | 74 ++++++++++++++++++- .../sidebar-scrollbar/geometry.expected.md | 33 +++++++++ .../src/client/skeleton/InputBar.module.css | 7 ++ .../client/ui-primitives/src/Menu.module.css | 7 ++ .../src/client/QuestionComposer.module.css | 7 ++ .../ui-theme/tests/scrollbar-styles.spec.ts | 65 +++++++++++++++- 9 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index f20ff46ec8..be67afa4f8 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: c52440bb057c5202ee90bcf53518fe62ba1379b7 -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: e234106c47eeaf418aac4a09ced0dd6c854a439c +2026-07-28-themed-scrollbars-and-reserved-gutter.md: dfa2dc69d46ea8ce9cf0ad621334b8999957e830 +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: 19381f5c1541d41359ba6f02e9448fe8342ee44c diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index c52440bb05..dfa2dc69d4 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -20,7 +20,9 @@ The rules sit on `body`, not `html`. `design-platform.css` declares the `--dsw-a The two renderings are mutually exclusive, and the exclusion is enforced rather than assumed. A non-`auto` `scrollbar-width` or `scrollbar-color` makes Chromium and Safari discard every `::-webkit-scrollbar*` rule for that element, `::-webkit-scrollbar-thumb:hover` included. Declaring both unconditionally therefore leaves the hover token rendering nowhere at all: the engines that implement the hover pseudo-element are exactly the ones the standard properties silence, and Firefox has no hover pseudo-element to fall back on. The standard properties consequently sit inside `@supports not selector(::-webkit-scrollbar)`, which is true only where the pseudo-element is unimplemented, so Firefox takes the standard path and WebKit-based engines take the pseudo-element path. The WebKit rules are not gated in turn: an engine without those pseudo-elements drops them as unknown selectors, so a gate would only restate what selector matching already does. An engine too old for the `selector()` function makes the condition invalid, which evaluates false and selects the pseudo-element path — the correct side for the pre-16.4 Safari that is the realistic case for that reading. -Both paths read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Four surfaces rebind today: the command popup, the slash menu, the model-select panel, and the settings panel. The last two declare it on the elevated panel rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. +Both paths read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Seven surfaces rebind today: the command popup, the slash menu, the model-select panel, the settings panel, the shared `ui-primitives` menu card, the composer input card, and the question composer card. Most declare it on the elevated card rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. + +The last three were missed in the first implementation and found in review, which is why the rebinding contract is now checked mechanically rather than by inspection: a sheet that scrolls somewhere and paints a known elevated surface somewhere must rebind. The elevated set is derived from the sheets that already rebind — a rebinding rule paints the surface whose elevation it declares — so it is self-maintaining rather than a list to update. The check is surface-level rather than element-level because the card and the descendant that scrolls are separate rules, and CSS text does not express which contains which. The track and the corner stay transparent, so the thumb reads against whatever surface scrolls under it; only the thumb and its hover state carry a token color. @@ -61,6 +63,8 @@ Three unit specs read the CSS text on disk. `ui-theme/tests/scrollbar-styles.spe `apps/web/tests/sidebar-scrollbar.e2e.ts` covers the facts only a real engine reports: the reserved band width, and which rendering path the engine took. It needs no model calls — the list only has to overflow — so it seeds cold sessions from an existing committed fixture read-only. +That scenario also commits a golden, `snapshots/sidebar-scrollbar/geometry.expected.md`, holding the resolved scrollbar style and geometry in both palettes. The aria goldens the other web scenarios commit cannot carry a CSS-only change: it alters no DOM and no accessible name, so their normalized trees are byte-identical with and without it. Recording the resolved values instead makes an unintended shift in thumb colour, band width, or rendering path a reviewable diff rather than a threshold someone has to reason about. Absolute coordinates are deliberately excluded — `timeRight` and the two edges depend on the sidebar's laid-out width and on font metrics, so committing them would produce a fixture that has to be re-recorded per platform and would document the platform rather than the change. What is recorded is the band, the overlap, and two orderings, each a difference or a comparison that survives any layout preserving the reservation. + Confirmed in headless chromium on the built client by reading computed values, which is what distinguishes a working token chain from a syntactically valid one: a scroll container computes the l1 thumb color in each palette, and a container that rebinds the indirection computes the l2 color, proving the rebind reaches the computed value rather than only the custom property. Firefox was verified the same way for the standard path, including the l1-to-l2 rebind on `scrollbar-color`; headless Firefox reports `scrollbar-width: none` on every element, styled or not, which is a headless artifact rather than an effect of the sheet. Two chromium measurement limits shape what the e2e can assert. The gate makes chromium report `scrollbar-width` and `scrollbar-color` as `auto`, so the substituted `scrollbar-color` is no longer the observable — the e2e asserts the `auto` reading deliberately, since a concrete value there would mean the gate leaked and silenced the pseudo-elements. And `getComputedStyle(el, '::-webkit-scrollbar-thumb')` folds in the `::-webkit-scrollbar-thumb:hover` rule, so it reports the hover color at rest and pins neither state; proven by deleting the hover rule through `CSSStyleSheet.deleteRule` in the live page, which flipped that same query from the hover color to the resting one. The e2e therefore reads the resting and hover colors as the indirection variables resolve on the list — one throwaway probe element per variable, because `getComputedStyle` returns a live declaration and a reused probe reports only the last value read — and reads the hover declaration out of the cascade as rule text. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index e234106c47..19381f5c15 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -20,7 +20,9 @@ Status: implemented 两种渲染互斥，而这种互斥是被强制的，不是假定的。`scrollbar-width` 或 `scrollbar-color` 只要取非 `auto` 值，Chromium 与 Safari 就会丢弃该元素上的全部 `::-webkit-scrollbar*` 规则，`::-webkit-scrollbar-thumb:hover` 也在其中。因此无条件地同时声明会让 hover token 在任何地方都得不到渲染：实现了 hover 伪元素的引擎，恰恰就是被标准属性静音的那些，而 Firefox 没有 hover 伪元素可作退路。于是标准属性写在 `@supports not selector(::-webkit-scrollbar)` 之内，该条件只在伪元素未被实现处为真，因此 Firefox 走标准属性路径，WebKit 系引擎走伪元素路径。WebKit 规则不再反向加门禁：不实现这些伪元素的引擎会把它们当作未知选择器丢弃，因此加门禁只是重述选择器匹配本身已经做的事。对于旧到不支持 `selector()` 函数的引擎，该条件无效，从而求值为假并选中伪元素路径——对于这条判断下现实存在的 16.4 之前的 Safari，这正是正确的一侧。 -两条路径都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有四处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板与设置面板。后两者把声明写在抬升面板上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 +两条路径都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有七处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板、设置面板、`ui-primitives` 共用菜单卡片、输入条卡片与提问组件卡片。多数把声明写在抬升卡片上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 + +后三处在最初的实现里被漏掉、由评审发现，因此重新绑定契约现在由机械检查把关，而不再依赖人工审阅：一张样式表只要在某处滚动、又在某处绘制已知的抬升表面，就必须重新绑定。抬升表面集合从已经做了重新绑定的样式表推导得出——重新绑定的那条规则正好绘制着它所声明抬升层级的那个表面——因此该集合自我维护，不是一份需要人去更新的清单。这项检查以表面为粒度而非以元素为粒度，因为卡片与真正滚动的后代元素是两条不同的规则，而 CSS 文本无法表达谁包含谁。 轨道与两条滚动条相交的角落保持透明，因此滑块是以其下滚动的任何表面为背景被看到；只有滑块及其 hover 状态带 token 颜色。 @@ -61,6 +63,8 @@ Status: implemented `apps/web/tests/sidebar-scrollbar.e2e.ts` 覆盖只有真实渲染引擎才能报告的事实：预留条带的宽度，以及引擎实际走的是哪条渲染路径。它不需要任何模型调用——列表只要溢出即可——因此以只读方式复用一份既有的已提交 fixture（测试前置数据）来铺入冷会话。 +这个场景还提交了一份 golden（期望产物）`snapshots/sidebar-scrollbar/geometry.expected.md`，记录两套调色板下解析后的滚动条样式与几何。其余 web 场景提交的 aria golden 承载不了纯 CSS 的改动：它不改变任何 DOM、也不改变任何无障碍名称，因此有无这次改动，它们规范化后的树都是逐字节相同的。改为记录解析后的取值，就让滑块颜色、条带宽度或渲染路径的意外变化成为可评审的 diff，而不是一条需要人去推敲的阈值断言。绝对坐标被特意排除——`timeRight` 与两条边缘取决于侧边栏排版后的宽度和字体度量，把它们提交进去会得到一份需要按平台重新录制的 fixture，那记录的是平台而不是这次改动。真正记录下来的是条带、重叠量与两个先后关系，每一项都是差值或比较，因此只要预留仍然成立，任何排版下都不变。 + 在构建产物客户端上于 headless chromium 中读取计算值确认，这正是区分「token 链真正生效」与「语法合法」的手段：滚动容器在两套调色板下分别计算出 l1 的滑块颜色，而重新绑定间接变量的容器计算出 l2 的颜色，证明重新绑定作用到了计算值，而不只是作用到自定义属性上。Firefox 的标准属性路径以同样方式做了验证，包含 `scrollbar-color` 上从 l1 到 l2 的重新绑定；headless Firefox 对任何元素（无论是否被样式命中）都报告 `scrollbar-width: none`，这是 headless 的产物，不是这张样式表造成的。 chromium 上有两处测量限制决定了 e2e 能断言什么。门禁使 chromium 报告的 `scrollbar-width` 与 `scrollbar-color` 都是 `auto`，因此代入后的 `scrollbar-color` 不再是可观测量——e2e 特意断言这个 `auto` 读数，因为此处出现具体值就意味着门禁泄漏、伪元素被静音。另外，`getComputedStyle(el, '::-webkit-scrollbar-thumb')` 会把 `::-webkit-scrollbar-thumb:hover` 规则一并折算进去，因此它在静止态就报告 hover 颜色，两种状态都锁不住；这一点由在运行中的页面里用 `CSSStyleSheet.deleteRule` 删掉 hover 规则得证——同一查询随之从 hover 颜色翻转为静止态颜色。因此 e2e 改为读取那组间接变量在列表上代入后的静止态与 hover 颜色（每个变量用一个一次性探针元素，因为 `getComputedStyle` 返回的是活的声明对象，复用探针只会报告最后一次读到的值），并把 hover 声明当作规则文本从层叠中读出。 diff --git a/apps/web/tests/sidebar-scrollbar.e2e.ts b/apps/web/tests/sidebar-scrollbar.e2e.ts index 6dfebb374c..763787f81f 100644 --- a/apps/web/tests/sidebar-scrollbar.e2e.ts +++ b/apps/web/tests/sidebar-scrollbar.e2e.ts @@ -59,13 +59,27 @@ // the same query flipped from the hover colour to the resting one). import { readFile } from 'node:fs/promises' import { fileURLToPath } from 'node:url' +import { join } from 'node:path' import type { Browser, Page } from 'playwright' import { chromium } from 'playwright' import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest' -import { launchWebScaffold, seedSession, watchConsole, webSnapshotMode, type WebScaffold } from './scaffold.ts' +import { + assertFixtureInventory, compareOrRefreshGolden, launchWebScaffold, seedSession, watchConsole, + webSnapshotMode, type WebScaffold, +} from './scaffold.ts' import { saveFailureShot } from './support.ts' const SEED = fileURLToPath(new URL('./snapshots/seeded-history/seed.jsonl', import.meta.url)) +const SNAPSHOT_DIR = fileURLToPath(new URL('./snapshots/sidebar-scrollbar', import.meta.url)) +/** + * Committed golden of the resolved scrollbar style and geometry, in both + * palettes. The aria goldens the other scenarios commit cannot carry this + * change: it alters no DOM and no accessible name, so their normalized trees are + * byte-identical with and without it. This one records the values instead, which + * makes an unintended shift in thumb colour, band width, or rendering path a + * reviewable diff rather than an assertion someone has to think about. + */ +const GEOMETRY_EXPECTED = join(SNAPSHOT_DIR, 'geometry.expected.md') const MODE = webSnapshotMode() /** Enough rows that the list overflows the 800px-tall viewport's sidebar; the scenario asserts the overflow rather than trusting it. */ const SEED_COUNT = 24 @@ -179,6 +193,48 @@ function measureList(page: Page): Promise { }) } +/** + * Render the golden body: the resolved scrollbar style of the list in each + * palette, plus the geometric relations the fix establishes. + * + * Absolute coordinates are deliberately absent. `timeRight`, `clientRight`, and + * `borderRight` depend on the sidebar's laid-out width and on font metrics, so + * committing them would make the golden fail on a machine whose fonts measure + * differently — a fixture that has to be re-recorded per platform documents the + * platform, not the change. What is recorded instead is the band, the overlap, + * and the two orderings, each of which is a difference or a comparison and so + * survives any layout that keeps the reservation. + * @param light - metrics measured under the light palette. + * @param dark - metrics measured under the dark palette. + * @returns the golden body, without a trailing newline. + */ +function renderGeometry(light: ListMetrics, dark: ListMetrics): string { + const palette = (name: string, metrics: ListMetrics): string[] => [ + `## ${name}`, + '', + `- scrollbar-gutter: ${metrics.gutter}`, + `- ::-webkit-scrollbar width: ${metrics.width}`, + `- ::-webkit-scrollbar-track background: ${metrics.track}`, + `- scrollbar-width: ${metrics.standardWidth}`, + `- scrollbar-color: ${metrics.standardColor}`, + `- ::-webkit-scrollbar-thumb:hover declarations: ${metrics.hoverRules.join(' | ')}`, + `- --dsh-scrollbar-thumb: ${metrics.token}`, + `- --dsh-scrollbar-thumb-hover: ${metrics.hoverToken}`, + `- list overflows: ${String(metrics.overflows)}`, + `- reserved band: ${String(metrics.band)}px`, + `- relative time covered by the bar: ${String(metrics.timeCoveredBy)}px`, + `- relative time ends inside the content area: ${String(metrics.timeRight <= metrics.clientRight)}`, + `- content area ends before the border box: ${String(metrics.clientRight < metrics.borderRight)}`, + '', + ] + return [ + '# Sidebar session list scrollbar', + '', + ...palette('Light palette', light), + ...palette('Dark palette', dark), + ].join('\n').trimEnd() +} + /** * Reveal the seeded rows: every seeded session is unattached, so they all sit * in the collapsed Ungrouped bucket. Converges on expanded rather than @@ -296,6 +352,22 @@ describe('web e2e: sidebar session list scrollbar (reserved gutter / themed thum expect(tripwire.pageErrors).toEqual([]) }, 60_000) + it('matches the committed scrollbar geometry golden in both palettes', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-sidebar-scrollbar-golden')) + const light = await measureList(page) + await page.evaluate(() => { document.body.setAttribute('data-ds-dark-theme', '') }) + const dark = await measureList(page) + await page.evaluate(() => { document.body.removeAttribute('data-ds-dark-theme') }) + await compareOrRefreshGolden(GEOMETRY_EXPECTED, renderGeometry(light, dark), MODE) + expect(tripwire.pageErrors).toEqual([]) + }, 60_000) + + it('commits exactly the fixtures it reads', async () => { + // The scenario borrows seeded-history's seed.jsonl rather than committing a + // second copy, so this directory holds the golden alone. + await assertFixtureInventory(SNAPSHOT_DIR, ['geometry.expected.md']) + }) + it.skipIf(MODE === 'record')('issued zero model calls and stayed clean', () => { expect(tripwire.warnings).toEqual([]) expect(tripwire.pageErrors).toEqual([]) diff --git a/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md b/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md new file mode 100644 index 0000000000..4349532ef8 --- /dev/null +++ b/apps/web/tests/snapshots/sidebar-scrollbar/geometry.expected.md @@ -0,0 +1,33 @@ +# Sidebar session list scrollbar + +## Light palette + +- scrollbar-gutter: stable +- ::-webkit-scrollbar width: 8px +- ::-webkit-scrollbar-track background: rgba(0, 0, 0, 0) +- scrollbar-width: auto +- scrollbar-color: auto +- ::-webkit-scrollbar-thumb:hover declarations: var(--dsh-scrollbar-thumb-hover) +- --dsh-scrollbar-thumb: rgb(229, 229, 229) +- --dsh-scrollbar-thumb-hover: rgb(212, 212, 212) +- list overflows: true +- reserved band: 8px +- relative time covered by the bar: 0px +- relative time ends inside the content area: true +- content area ends before the border box: true + +## Dark palette + +- scrollbar-gutter: stable +- ::-webkit-scrollbar width: 8px +- ::-webkit-scrollbar-track background: rgba(0, 0, 0, 0) +- scrollbar-width: auto +- scrollbar-color: auto +- ::-webkit-scrollbar-thumb:hover declarations: var(--dsh-scrollbar-thumb-hover) +- --dsh-scrollbar-thumb: rgb(60, 60, 61) +- --dsh-scrollbar-thumb-hover: rgb(84, 85, 87) +- list overflows: true +- reserved band: 8px +- relative time covered by the bar: 0px +- relative time ends inside the content area: true +- content area ends before the border box: true diff --git a/packages/client/ui-conversation/src/client/skeleton/InputBar.module.css b/packages/client/ui-conversation/src/client/skeleton/InputBar.module.css index 0cb3920f6e..1bada4391d 100644 --- a/packages/client/ui-conversation/src/client/skeleton/InputBar.module.css +++ b/packages/client/ui-conversation/src/client/skeleton/InputBar.module.css @@ -87,6 +87,13 @@ box-shadow: var(--dsw-shadow-lv2); font-size: 16px; line-height: 24px; + /* Elevated surface in dark, same as the menus: the textarea inside scrolls + once the composer hits its height cap, so the thumb takes the l2 pair. + Declared on the card because the elevation belongs to the surface, and the + custom properties inherit down to the textarea that actually scrolls (see + ui-theme styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } .accessory { diff --git a/packages/client/ui-primitives/src/Menu.module.css b/packages/client/ui-primitives/src/Menu.module.css index 8e16b252d3..873c54e179 100644 --- a/packages/client/ui-primitives/src/Menu.module.css +++ b/packages/client/ui-primitives/src/Menu.module.css @@ -17,6 +17,13 @@ border-radius: 12px; background: var(--dsw-specific-menu); box-shadow: var(--dsw-shadow-lv3); + /* Elevated surface: the scrollbar thumb takes the l2 elevation tokens. The + declaration sits on the card rather than on `.scrollable .viewport` + because the elevation is a property of this surface, and the custom + properties inherit down to whichever descendant actually scrolls (see + ui-theme styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } /* Primary card is 218 wide in the design across both hosts. */ diff --git a/packages/client/ui-question/src/client/QuestionComposer.module.css b/packages/client/ui-question/src/client/QuestionComposer.module.css index 19306bd552..be4fd5791b 100644 --- a/packages/client/ui-question/src/client/QuestionComposer.module.css +++ b/packages/client/ui-question/src/client/QuestionComposer.module.css @@ -19,6 +19,13 @@ background: var(--dsw-specific-input-major); box-shadow: var(--dsw-shadow-lv1-blur); color: var(--dsw-alias-label-primary); + /* Elevated surface in dark, same as the menus: the option list inside scrolls + once the card hits the cap above, so the thumb takes the l2 pair. Declared + on the card because the elevation belongs to the surface, and the custom + properties inherit down to `.options` (see ui-theme styles/scrollbar.css + for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } .card, diff --git a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts index 0e1a5c94c1..544c8a1bcb 100644 --- a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts +++ b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts @@ -167,22 +167,64 @@ const allTokens = new Set([...lightTokens, ...darkTokens]) const referencedTokens = new Map() /** Every indirection property any package stylesheet outside ui-theme declares, mapped to its declaring rules. */ const rebindRules: { file: string; rule: CssRule }[] = [] +/** + * What one stylesheet contributes to the elevated-surface question: which + * surface tokens its rules paint, whether any rule scrolls, and whether it + * rebinds. Kept per file rather than per rule because the elevated card and the + * descendant that actually scrolls are separate rules in the same sheet, and + * CSS text does not express which element contains which. + */ +interface SheetSurfaces { + /** Surface tokens named by `background`/`background-color` on a rebinding rule. */ + rebound: Set + /** Surface tokens named by `background`/`background-color` anywhere in the sheet. */ + painted: Set + /** True when some rule declares `overflow*: auto|scroll`. */ + scrolls: boolean + /** True when some rule rebinds the indirection. */ + rebinds: boolean +} +const sheetSurfaces = new Map() + +/** Properties whose `auto`/`scroll` value makes a rule a scroll container. */ +const OVERFLOW_PROPERTIES = ['overflow', 'overflow-x', 'overflow-y'] +/** Properties that paint a surface, and so identify the elevation a rule sits on. */ +const SURFACE_PROPERTIES = ['background', 'background-color'] for (const file of packageStylesheets()) { const rules = parseRules(readFileSync(file, 'utf8')) + const surfaces: SheetSurfaces = { rebound: new Set(), painted: new Set(), scrolls: false, rebinds: false } for (const rule of rules) { let rebinds = false + const ruleSurfaces: string[] = [] for (const [property, value] of rule.declarations) { if (property.startsWith(INDIRECTION_PREFIX) && file !== fileURLToPath(new URL('scrollbar.css', STYLES))) rebinds = true + if (OVERFLOW_PROPERTIES.includes(property) && /\b(?:auto|scroll)\b/.test(value)) surfaces.scrolls = true + if (SURFACE_PROPERTIES.includes(property)) ruleSurfaces.push(...varReferences(value)) for (const token of varReferences(value)) { if (!token.startsWith(TOKEN_PREFIX)) continue referencedTokens.set(token, [...referencedTokens.get(token) ?? [], file]) } } - if (rebinds) rebindRules.push({ file, rule }) + for (const token of ruleSurfaces) surfaces.painted.add(token) + if (rebinds) { + rebindRules.push({ file, rule }) + surfaces.rebinds = true + for (const token of ruleSurfaces) surfaces.rebound.add(token) + } } + sheetSurfaces.set(file, surfaces) } +/** + * Surface tokens known to be elevated, derived from the sheets that already + * rebind rather than listed here: a rebinding rule paints the surface whose + * elevation it is declaring. Deriving it means a new elevated surface joins the + * set by rebinding, and cannot be added to the palette without either rebinding + * or failing the check below. + */ +const elevatedSurfaces = new Set([...sheetSurfaces.values()].flatMap(surfaces => [...surfaces.rebound])) + describe('design-platform.css scrollbar tokens', () => { it('defines the same scrollbar token set in the light and the dark block', () => { // A token present only in the light block silently keeps its light value @@ -383,4 +425,25 @@ describe('elevated surface rebinds', () => { } } }) + + it('every sheet that scrolls on a known elevated surface rebinds', () => { + // The failure this closes: a scroll container on an elevated surface that + // nobody remembered to rebind renders the l1 thumb, which differs from l2 + // only in the dark palette and only for that one surface — invisible in + // review and in a light-palette screenshot. Three sheets shipped that way + // (ui-primitives Menu, InputBar, QuestionComposer) and review caught them + // by hand, which is what this replaces. + // + // Surface-level, not element-level: the elevated card and the descendant + // that scrolls are separate rules, and CSS text does not say which contains + // which. A sheet that both scrolls somewhere and paints a known elevated + // surface somewhere must rebind; the elevation would otherwise be a + // coincidence of two unrelated rules, which no sheet under test does. + expect(elevatedSurfaces.size).toBeGreaterThan(0) + for (const [file, surfaces] of sheetSurfaces) { + if (!surfaces.scrolls || surfaces.rebinds) continue + const elevated = [...surfaces.painted].filter(token => elevatedSurfaces.has(token)) + expect(elevated, `${file} scrolls on ${elevated.join(', ')} without rebinding`).toEqual([]) + } + }) }) From f035797755101907572aeae636c080a2f5493108 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 18:14:22 +0800 Subject: [PATCH 7/8] build(web): rebuild plugin bundles before the browser lane test:web ran build:web alone, which does not rebuild UI plugin client bundles. Plugin CSS reaches the browser through packages/client/*/lib/ client.js, not apps/web/dist, so a changed *.module.css served its previous bundle: the run exercised stale CSS and a removed declaration still passed. That is how I first mistook a valid gutter test for a vacuous one. Root build already covers packages/*/*, so running it first is enough; check-all already ordered build before build:web, so CI was never exposed. Only the local script was, which is where a stale-bundle pass is most likely to be believed. Verified against the situation it fixes: mutate the source, rebuild the bundle, restore the source, and the artifact is left without the declaration while the source has it. Under the old script the run tested that artifact; under the new one the artifact is rebuilt first (grep goes 0 to 1) and the scrollbar spec passes. The nine failing web files are the pre-existing aria-golden set from f2c004524, unchanged by this. --- ...26-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml | 4 ++-- .../2026-07-28-themed-scrollbars-and-reserved-gutter.md | 4 +++- .../2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md | 4 +++- docs/testing.i18n.yaml | 6 +++--- docs/testing.md | 2 +- docs/testing.zh.md | 2 +- package.json | 2 +- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index be67afa4f8..1b2380340f 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: dfa2dc69d46ea8ce9cf0ad621334b8999957e830 -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: 19381f5c1541d41359ba6f02e9448fe8342ee44c +2026-07-28-themed-scrollbars-and-reserved-gutter.md: 53f125c5cbe891608e909ac412f6c44d9aae5c3d +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: a3b595c8ddbadb8aacdebc2e5e2dc27ac5972383 diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index dfa2dc69d4..53f125c5cb 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -75,4 +75,6 @@ Headless chromium draws overlay scrollbars, and that is the configuration in whi Both are asserted because each catches a different regression, established by mutating one declaration at a time with the other assertions in that test silenced. Removing only the gutter leaves `timeCoveredBy` at 0 — the bar is then 8px and the row's right padding is also 8px, so it abuts the timestamp without covering it — and the band assertion is what fails. Removing the pseudo-element width as well, which is the actual master state, produces the overlap, and `timeCoveredBy` fails at 7. A headed run under xvfb cannot show the symptom in either state, because chromium paints a classic space-consuming bar there and `clientWidth` already excludes it. -Verifying browser-visible plugin CSS needs a rebuild `pnpm run build:web` does not perform. `WorkspaceBrowser.module.css` never reaches `apps/web/dist`: ui-workspace loads as a runtime plugin and its CSS is inlined into `packages/client/ui-workspace/lib/client.js`, built by that package's own `bundle` script. A negative control that reruns only `build:web` therefore exercises a stale bundle and passes with the declaration removed, which reads as a vacuous test rather than as an invalid control. Rebuild with `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`, confirm the artifact by grepping `lib/client.js` for the declaration, then `build:web`. No script in the web lane does this: `test:web` runs `build:web` alone, so every scroll-region or plugin-CSS change hits the same trap. +Verifying browser-visible plugin CSS needs a rebuild `pnpm run build:web` does not perform. `WorkspaceBrowser.module.css` never reaches `apps/web/dist`: ui-workspace loads as a runtime plugin and its CSS is inlined into `packages/client/ui-workspace/lib/client.js`, built by that package's own `bundle` script. A negative control that reruns only `build:web` therefore exercises a stale bundle and passes with the declaration removed, which reads as a vacuous test rather than as an invalid control. Rebuild with `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`, confirm the artifact by grepping `lib/client.js` for the declaration, then `build:web`. + +`test:web` ran `build:web` alone, so every scroll-region or plugin-CSS change hit that trap; it now runs `build` first, which covers `packages/*/*` and so rebuilds the plugin bundles. `check-all` already ordered `build` before `build:web`, so CI was never exposed — only the local script was, which is exactly where a stale-bundle pass is most likely to be believed. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index 19381f5c15..a3b595c8dd 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -75,4 +75,6 @@ headless chromium 绘制的是覆盖式滚动条，而这恰好就是被报告 两者都要断言，因为各自捕捉的是不同的回归；这一点通过每次只改动一条声明、并把同一个测试里的其余断言静音来确定。只删掉空位声明时 `timeCoveredBy` 仍为 0——此时滚动条是 8px，而行的右内边距也是 8px，于是它紧贴时间戳但并未盖住——失败的是条带那条断言。再把伪元素宽度也删掉（这才是 master 的真实状态）才会产生重叠，此时 `timeCoveredBy` 以 7 变红。在 xvfb 下的有头运行无论哪种状态都看不到这个症状，因为 chromium 在那里画的是经典占位滚动条，`clientWidth` 本来就已经把它排除了。 -验证浏览器可见的插件 CSS 需要一次 `pnpm run build:web` 并不执行的重建。`WorkspaceBrowser.module.css` 从不进入 `apps/web/dist`：ui-workspace 以运行时插件方式加载，其 CSS 内联进 `packages/client/ui-workspace/lib/client.js`，由该包自己的 `bundle` 脚本构建。因此只重跑 `build:web` 的反向对照实际测的是旧产物，去掉声明后仍会通过，看起来像测试无效，实际是对照无效。正确做法是先 `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`，用 grep 在 `lib/client.js` 中确认该声明确实存在或消失，然后再 `build:web`。web 通道中没有任何脚本会做这一步：`test:web` 只运行 `build:web`，因此任何滚动区域或插件 CSS 的改动都会碰到同一个陷阱。 +验证浏览器可见的插件 CSS 需要一次 `pnpm run build:web` 并不执行的重建。`WorkspaceBrowser.module.css` 从不进入 `apps/web/dist`：ui-workspace 以运行时插件方式加载，其 CSS 内联进 `packages/client/ui-workspace/lib/client.js`，由该包自己的 `bundle` 脚本构建。因此只重跑 `build:web` 的反向对照实际测的是旧产物，去掉声明后仍会通过，看起来像测试无效，实际是对照无效。正确做法是先 `pnpm --filter @deepseek-ai/dsh-client-ui-workspace run bundle`，用 grep 在 `lib/client.js` 中确认该声明确实存在或消失，然后再 `build:web`。 + +`test:web` 原先只运行 `build:web`，因此任何滚动区域或插件 CSS 的改动都会碰到这个陷阱；现在它先运行 `build`，而 `build` 覆盖 `packages/*/*`，从而会重建各插件产物。`check-all` 本来就把 `build` 排在 `build:web` 之前，所以 CI 从未受影响——受影响的只有本地脚本，而这恰恰是「产物过期却通过」最容易被当真的地方。 diff --git a/docs/testing.i18n.yaml b/docs/testing.i18n.yaml index 9c4c30c6c1..976f52c779 100644 --- a/docs/testing.i18n.yaml +++ b/docs/testing.i18n.yaml @@ -1,6 +1,6 @@ # Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: -# pnpm run verify-translation-pairing --write -testing.md: d898c86169c20a10ffc0c2d0ecb712965a55207a -testing.zh.md: 424b22b3049ba395763cd796f288353a95a94311 +# pnpm run verify-translation-pairing --write docs/testing.md +testing.md: 04bd7782fa4328b6b693f13f60f4e33b463f8a18 +testing.zh.md: 5712fd8ce7b0cd46ebeb237bdd12c6c572ebe3de diff --git a/docs/testing.md b/docs/testing.md index d898c86169..04bd7782fa 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -10,7 +10,7 @@ How this repo tests, tier by tier, and the rules that keep a green suite meaning - **Coverage gate** (`pnpm run test:coverage`): the gating run, per-file 100% on `packages/*/*/src`. An uncovered line is often dead code the gate is correctly flagging for deletion, not a missing test to bolt on. Line coverage is necessary, never sufficient — it proves lines ran, not that the feature works as shipped. - **Real-API e2e** (`pnpm run test:e2e`): with-key tests against live provider APIs — the DeepSeek model plus provider-specific smokes that gate on their own keys (`EXA_API_KEY`, `PERPLEXITY_API_KEY`, …); each suite self-skips without its key so keyless CI stays green ([real-API e2e Agent Note](../.agents/notes/implemented/testing/2026-06-19-real-api-e2e-ci.md)). - **Snapshot** (`pnpm run test:snapshot`): keyless expected outputs cover external behavior — transport contracts and presentation, while persisted logs pin assembled backend behavior. ACP boots the real automation-server example, replays a recorded session, and diffs normalized JSON-RPC plus the re-persisted log ([ACP snapshot Agent Note](../.agents/notes/implemented/testing/2026-06-19-acp-snapshot-tests.md)); headless pins `stream-json` through its real one-shot process. TUI journeys replay primary/child JSONL through the real loop and tools, then project ANSI into semantic terminal-state outputs; package snapshots retain transient states and a real PTY covers the process boundary ([TUI snapshot Agent Note](../.agents/notes/implemented/testing/2026-07-18-tui-terminal-state-snapshots.md)). Use `pnpm run test:snapshot:record` when a model transcript changes and `pnpm run test:snapshot:refresh` when replay input remains valid; review every JSONL and expected-output diff. One ACP scenario (`text-turn`) pins full system-prompt/tool-schema content; other fixtures tokenize it so an edit churns one line ([pinned-header Agent Note](../.agents/notes/archived/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)). -- **Web browser snapshot** (gate-exempt `pnpm run test:web`): real chromium over the in-process web composition replays recorded fixtures against conversation aria goldens (`apps/web/tests/snapshots/`); record/refresh semantics and the deferred CI browser decision: [web e2e lane Agent Note](../.agents/notes/implemented/testing/2026-07-24-web-gui-browser-e2e-lane.md). +- **Web browser snapshot** (gate-exempt `pnpm run test:web`): real chromium over the in-process web composition replays recorded fixtures against conversation aria goldens (`apps/web/tests/snapshots/`); record/refresh semantics and the deferred CI browser decision: [web e2e lane Agent Note](../.agents/notes/implemented/testing/2026-07-24-web-gui-browser-e2e-lane.md). [Runs `build` first](../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md): plugin CSS ships per plugin. Committed session-format JSONL uses the canonical packed-row layout, and the keyless snapshot gate discovers every such fixture by its `session` header. In-flight branches carrying older fixture edits merge current `master` and run the [temporary migrator](../scripts/migrate-packed-session-fixtures.ts) through `pnpm run migrate:packed-session-fixtures`; the [removal proposal](../.agents/notes/proposed/process/2026-07-26-remove-packed-session-fixture-migrator.md) retires that command and these links after all affected branches converge. diff --git a/docs/testing.zh.md b/docs/testing.zh.md index 424b22b304..5712fd8ce7 100644 --- a/docs/testing.zh.md +++ b/docs/testing.zh.md @@ -10,7 +10,7 @@ - **覆盖率门禁**（`pnpm run test:coverage`）：门禁级运行，对 `packages/*/*/src` 按文件 100% 覆盖。未覆盖的行往往是门禁正确标记出的死代码（应删除），而非需要补写的测试。行覆盖率是必要条件，但永远不是充分条件：它证明行被执行过，不证明功能按交付预期工作。 - **真实 API e2e**（`pnpm run test:e2e`）：带密钥测试调用真实提供方 API，包括 DeepSeek 模型以及各提供方特有的冒烟测试；这些测试各自由自己的密钥控制（`EXA_API_KEY`、`PERPLEXITY_API_KEY` 等），缺少密钥时套件会自动跳过，使 keyless CI 保持绿色（[真实 API e2e Agent Note](../.agents/notes/implemented/testing/2026-06-19-real-api-e2e-ci.md)）。 - **快照**（`pnpm run test:snapshot`）：无密钥预期输出覆盖对外行为（传输契约与呈现），持久化日志则固定组装后的后端行为。ACP 启动真实的自动化服务器示例、回放录制会话，并对归一化 JSON-RPC 与重新持久化的日志执行 diff（[ACP 快照 Agent Note](../.agents/notes/implemented/testing/2026-06-19-acp-snapshot-tests.md)）；headless 通过真实单次运行进程固定 `stream-json`。TUI 旅程通过真实循环与工具回放主会话与子会话 JSONL，再将 ANSI 投影为语义化终端状态输出；包级快照保留瞬态状态，真实 PTY 覆盖进程边界（[TUI 快照 Agent Note](../.agents/notes/implemented/testing/2026-07-18-tui-terminal-state-snapshots.md)）。当模型 transcript（文本记录）发生变化时使用 `pnpm run test:snapshot:record`，回放输入仍然有效时使用 `pnpm run test:snapshot:refresh`；请审查每一处 JSONL 与预期输出差异。一个 ACP 场景（`text-turn`）固定完整的系统提示词与工具 schema 内容；其他 fixture（测试前置数据）将其 token 化，因此修改只会扰动一行（[pinned-header Agent Note](../.agents/notes/archived/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)）。 -- **Web 浏览器快照**（豁免门禁的 `pnpm run test:web`）：真实 chromium 在进程内 web 组装之上回放已录制 fixture，与会话区 aria 预期输出比对（`apps/web/tests/snapshots/`）；`DSH_SNAPSHOT=record`/`refresh` 的语义与暂缓的 CI 浏览器决策见 [web e2e 车道 Agent Note](../.agents/notes/implemented/testing/2026-07-24-web-gui-browser-e2e-lane.md)。 +- **Web 浏览器快照**（豁免门禁的 `pnpm run test:web`）：真实 chromium 在进程内 web 组装之上回放已录制 fixture，与会话区 aria 预期输出比对（`apps/web/tests/snapshots/`）；`DSH_SNAPSHOT=record`/`refresh` 的语义与暂缓的 CI 浏览器决策见 [web e2e 车道 Agent Note](../.agents/notes/implemented/testing/2026-07-24-web-gui-browser-e2e-lane.md)。[先跑 `build`](../.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md)：插件 CSS 按插件分别发布。 签入仓库的会话格式 JSONL 使用规范打包行布局，无密钥快照门禁会通过 `session` header 发现每一份此类 fixture。仍携带旧版 fixture 改动的在途分支应合并当前 `master`，并通过 `pnpm run migrate:packed-session-fixtures` 运行[临时迁移器](../scripts/migrate-packed-session-fixtures.ts)；待所有受影响分支收敛后，[移除提案](../.agents/notes/proposed/process/2026-07-26-remove-packed-session-fixture-migrator.md)会移除该命令及这些链接。 diff --git a/package.json b/package.json index cb6a6270ed..a0fc4f780f 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "test:snapshot:record": "DSH_SNAPSHOT=record vitest run --config vitest.snapshot.config.ts --update", "test:snapshot:refresh": "DSH_SNAPSHOT=refresh vitest run --config vitest.snapshot.config.ts", "migrate:packed-session-fixtures": "tsx scripts/migrate-packed-session-fixtures.ts", - "test:web": "npm run build:web && vitest run --config vitest.web.config.ts", + "test:web": "npm run build && npm run build:web && vitest run --config vitest.web.config.ts", "test:gui": "vitest run packages/client packages/host", "check:all": "tsx scripts/run-gates.ts check-all", "check:ci": "tsx scripts/run-gates.ts ci-primary", From 0401c3c6c78088f628c03b288d4871fab4ba5080 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 28 Jul 2026 18:31:52 +0800 Subject: [PATCH 8/8] test(ui-theme): resolve elevated surfaces from the palette, not from rebinds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check added last commit derived its elevated set from the sheets that already rebind, which cannot catch the omission it targets: such a set only confirms what someone already remembered, and a surface nobody has rebound yet defines itself as unelevated. Review found the case that proves it — TodoPanel scrolls in .list on a --dsw-specific-tip card, the same dark rung as the menu surface, unrebound and with the derived check green. Resolves the set from the palette's own dark elevation ladder instead: the surface tokens whose dark value lands on bg-layer-2 or bg-layer-3, which is the step the l1/l2 split encodes. A new palette token on an elevated rung is in scope the moment it is defined. Scope is by token family rather than geometry: only --dsw-alias-bg-* and --dsw-specific-* name a surface. The button, interactive, and markdown families reach the same rungs while naming a control or an inline span that no scroll container renders a bar against, and shape cannot separate them since a floating button carries a radius, a shadow, and a fixed size — ChatView's .toBottom pill was the false positive that showed this. Adds the missing TodoPanel rebind. Mutation-checked all four rebinds in turn: each is named with its surface. The palette anchoring has its own control — narrowing the family pattern turns it red on --dsw-specific-menu. --- ...d-scrollbars-and-reserved-gutter.i18n.yaml | 4 +- ...8-themed-scrollbars-and-reserved-gutter.md | 8 +- ...hemed-scrollbars-and-reserved-gutter.zh.md | 8 +- .../src/client/skeleton/TodoPanel.module.css | 7 ++ .../ui-theme/tests/scrollbar-styles.spec.ts | 115 +++++++++++++----- 5 files changed, 107 insertions(+), 35 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml index 1b2380340f..a205f48f54 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md -2026-07-28-themed-scrollbars-and-reserved-gutter.md: 53f125c5cbe891608e909ac412f6c44d9aae5c3d -2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: a3b595c8ddbadb8aacdebc2e5e2dc27ac5972383 +2026-07-28-themed-scrollbars-and-reserved-gutter.md: 38228c868bb00210118e8110feb722fb81d0d56c +2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md: 9fafe1faa9303b5e2e23a1b3064904f71494026d diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md index 53f125c5cb..38228c868b 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.md @@ -20,9 +20,13 @@ The rules sit on `body`, not `html`. `design-platform.css` declares the `--dsw-a The two renderings are mutually exclusive, and the exclusion is enforced rather than assumed. A non-`auto` `scrollbar-width` or `scrollbar-color` makes Chromium and Safari discard every `::-webkit-scrollbar*` rule for that element, `::-webkit-scrollbar-thumb:hover` included. Declaring both unconditionally therefore leaves the hover token rendering nowhere at all: the engines that implement the hover pseudo-element are exactly the ones the standard properties silence, and Firefox has no hover pseudo-element to fall back on. The standard properties consequently sit inside `@supports not selector(::-webkit-scrollbar)`, which is true only where the pseudo-element is unimplemented, so Firefox takes the standard path and WebKit-based engines take the pseudo-element path. The WebKit rules are not gated in turn: an engine without those pseudo-elements drops them as unknown selectors, so a gate would only restate what selector matching already does. An engine too old for the `selector()` function makes the condition invalid, which evaluates false and selects the pseudo-element path — the correct side for the pre-16.4 Safari that is the realistic case for that reading. -Both paths read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Seven surfaces rebind today: the command popup, the slash menu, the model-select panel, the settings panel, the shared `ui-primitives` menu card, the composer input card, and the question composer card. Most declare it on the elevated card rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. +Both paths read one indirection pair, `--dsh-scrollbar-thumb` and `--dsh-scrollbar-thumb-hover`, bound on `body` to the l1 (base-surface) tokens. **This is the rebinding contract, and it is the part the CSS alone does not state**: an elevated surface sets `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` and `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)` on its own container, and that one rebind reaches the standard properties and the WebKit pseudo-elements together. The pair is rebound as a pair; rebinding the resting thumb alone leaves the hover state on the base-surface token. Eight surfaces rebind today: the command popup, the slash menu, the model-select panel, the settings panel, the shared `ui-primitives` menu card, the composer input card, the question composer card, and the todo panel. Most declare it on the elevated card rather than on the scrolling descendant, because the elevation is a property of the surface and custom properties inherit down to whichever child actually scrolls. -The last three were missed in the first implementation and found in review, which is why the rebinding contract is now checked mechanically rather than by inspection: a sheet that scrolls somewhere and paints a known elevated surface somewhere must rebind. The elevated set is derived from the sheets that already rebind — a rebinding rule paints the surface whose elevation it declares — so it is self-maintaining rather than a list to update. The check is surface-level rather than element-level because the card and the descendant that scrolls are separate rules, and CSS text does not express which contains which. +The last four were missed in the first implementation and found in review, which is why the rebinding contract is now checked mechanically rather than by inspection: a sheet that scrolls somewhere and paints an elevated surface somewhere must rebind. + +The elevated set is resolved from the palette's own dark elevation ladder — the surface tokens whose dark value lands on `bg-layer-2` or `bg-layer-3`, which is the step the l1/l2 split encodes. Deriving it instead from the sheets that already rebind was the first attempt and is unsound: such a set can only confirm what someone already remembered, and a surface nobody has rebound yet — exactly the case the check exists for — defines itself as unelevated. `--dsw-specific-tip` proved it, resolving to the menu surface's rung while the todo panel scrolled on it unrebound and the derived check stayed green. + +Scope is by token family, not by geometry: only `--dsw-alias-bg-*` and `--dsw-specific-*` name a surface. `--dsw-alias-button-*`, `--dsw-alias-interactive-*`, and `--dsw-alias-markdown-*` reach the same rungs while naming a control or an inline span that no scroll container renders its bar against. Shape cannot make that call, since a floating button legitimately carries a radius, a shadow, and a fixed size. The check is per sheet rather than per rule because the card and the descendant that scrolls are separate rules, and CSS text does not express which contains which. The track and the corner stay transparent, so the thumb reads against whatever surface scrolls under it; only the thumb and its hover state carry a token color. diff --git a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md index a3b595c8dd..9fafe1faa9 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-28-themed-scrollbars-and-reserved-gutter.zh.md @@ -20,9 +20,13 @@ Status: implemented 两种渲染互斥，而这种互斥是被强制的，不是假定的。`scrollbar-width` 或 `scrollbar-color` 只要取非 `auto` 值，Chromium 与 Safari 就会丢弃该元素上的全部 `::-webkit-scrollbar*` 规则，`::-webkit-scrollbar-thumb:hover` 也在其中。因此无条件地同时声明会让 hover token 在任何地方都得不到渲染：实现了 hover 伪元素的引擎，恰恰就是被标准属性静音的那些，而 Firefox 没有 hover 伪元素可作退路。于是标准属性写在 `@supports not selector(::-webkit-scrollbar)` 之内，该条件只在伪元素未被实现处为真，因此 Firefox 走标准属性路径，WebKit 系引擎走伪元素路径。WebKit 规则不再反向加门禁：不实现这些伪元素的引擎会把它们当作未知选择器丢弃，因此加门禁只是重述选择器匹配本身已经做的事。对于旧到不支持 `selector()` 函数的引擎，该条件无效，从而求值为假并选中伪元素路径——对于这条判断下现实存在的 16.4 之前的 Safari，这正是正确的一侧。 -两条路径都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有七处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板、设置面板、`ui-primitives` 共用菜单卡片、输入条卡片与提问组件卡片。多数把声明写在抬升卡片上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 +两条路径都读取同一组间接变量 `--dsh-scrollbar-thumb` 与 `--dsh-scrollbar-thumb-hover`，它们在 `body` 上绑定到 l1（基础表面）token。**这就是重新绑定契约，也是单看 CSS 无法得知的部分**：抬升表面在自己的容器上设置 `--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2)` 与 `--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2)`，这一次重新绑定同时作用于标准属性和 WebKit 伪元素。这组变量必须成对重新绑定；只改静止态滑块会让 hover 状态仍留在基础表面的 token 上。目前有八处抬升表面做了重新绑定：命令浮层、斜杠菜单、模型选择面板、设置面板、`ui-primitives` 共用菜单卡片、输入条卡片、提问组件卡片与待办面板。多数把声明写在抬升卡片上而非滚动的后代元素上，因为抬升层级是这个表面的属性，而自定义属性会继承到真正滚动的那个子元素。 -后三处在最初的实现里被漏掉、由评审发现，因此重新绑定契约现在由机械检查把关，而不再依赖人工审阅：一张样式表只要在某处滚动、又在某处绘制已知的抬升表面，就必须重新绑定。抬升表面集合从已经做了重新绑定的样式表推导得出——重新绑定的那条规则正好绘制着它所声明抬升层级的那个表面——因此该集合自我维护，不是一份需要人去更新的清单。这项检查以表面为粒度而非以元素为粒度，因为卡片与真正滚动的后代元素是两条不同的规则，而 CSS 文本无法表达谁包含谁。 +后四处在最初的实现里被漏掉、由评审发现，因此重新绑定契约现在由机械检查把关，而不再依赖人工审阅：一张样式表只要在某处滚动、又在某处绘制抬升表面，就必须重新绑定。 + +抬升表面集合是从调色板自身的暗色抬升阶梯解析出来的——暗色取值落在 `bg-layer-2` 或 `bg-layer-3` 上的那些表面 token，而这一档正是 l1/l2 之分所编码的层级差。最初的做法是从已经做了重新绑定的样式表反向推导，那是不成立的：这样得到的集合只能确认别人已经记得的部分，而尚无人重新绑定的表面——恰恰就是这项检查存在的理由——会把自己定义成「非抬升」。`--dsw-specific-tip` 证明了这一点：它解析到与菜单表面相同的那一档，待办面板在它上面滚动却没有重新绑定，而推导式的检查依然是绿的。 + +判定范围依据 token 家族而非几何形状：只有 `--dsw-alias-bg-*` 与 `--dsw-specific-*` 表述的是表面。`--dsw-alias-button-*`、`--dsw-alias-interactive-*` 与 `--dsw-alias-markdown-*` 会落到相同档位，但它们表述的是控件或行内片段，没有任何滚动容器会把滚动条画在它们之上。形状无法做这个判断，因为悬浮按钮本来就会带圆角、阴影和固定尺寸。这项检查以样式表为粒度而非以规则为粒度，因为卡片与真正滚动的后代元素是两条不同的规则，而 CSS 文本无法表达谁包含谁。 轨道与两条滚动条相交的角落保持透明，因此滑块是以其下滚动的任何表面为背景被看到；只有滑块及其 hover 状态带 token 颜色。 diff --git a/packages/client/ui-conversation/src/client/skeleton/TodoPanel.module.css b/packages/client/ui-conversation/src/client/skeleton/TodoPanel.module.css index c6b50a2c77..7b506b5553 100644 --- a/packages/client/ui-conversation/src/client/skeleton/TodoPanel.module.css +++ b/packages/client/ui-conversation/src/client/skeleton/TodoPanel.module.css @@ -11,6 +11,13 @@ border: 1px solid var(--dsw-alias-border-l1); border-radius: 14px; background: var(--dsw-specific-tip); + /* Elevated surface: `--dsw-specific-tip` is the same dark rung as the menu + surface, and `.list` scrolls inside this card, so the thumb takes the l2 + elevation tokens. Declared here because the elevation belongs to the + surface, and the custom properties inherit down to `.list` (see ui-theme + styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); } .body { diff --git a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts index 544c8a1bcb..6eab29451b 100644 --- a/packages/client/ui-theme/tests/scrollbar-styles.spec.ts +++ b/packages/client/ui-theme/tests/scrollbar-styles.spec.ts @@ -169,16 +169,14 @@ const referencedTokens = new Map() const rebindRules: { file: string; rule: CssRule }[] = [] /** * What one stylesheet contributes to the elevated-surface question: which - * surface tokens its rules paint, whether any rule scrolls, and whether it + * elevated surfaces it paints, whether any rule scrolls, and whether it * rebinds. Kept per file rather than per rule because the elevated card and the * descendant that actually scrolls are separate rules in the same sheet, and - * CSS text does not express which element contains which. + * CSS text does not express which contains which. */ interface SheetSurfaces { - /** Surface tokens named by `background`/`background-color` on a rebinding rule. */ - rebound: Set - /** Surface tokens named by `background`/`background-color` anywhere in the sheet. */ - painted: Set + /** Elevated surface tokens this sheet paints anywhere. */ + elevated: Set /** True when some rule declares `overflow*: auto|scroll`. */ scrolls: boolean /** True when some rule rebinds the indirection. */ @@ -190,10 +188,57 @@ const sheetSurfaces = new Map() const OVERFLOW_PROPERTIES = ['overflow', 'overflow-x', 'overflow-y'] /** Properties that paint a surface, and so identify the elevation a rule sits on. */ const SURFACE_PROPERTIES = ['background', 'background-color'] +/** + * Token families that name a SURFACE — a background an element is drawn on, and + * so something a scrollbar can sit against. `--dsw-alias-button-*`, + * `--dsw-alias-interactive-*`, and `--dsw-alias-markdown-*` reach the same dark + * elevation rungs while naming a control or an inline span, which no scroll + * container renders its bar against (ChatView's floating `.toBottom` pill, + * CodeBlock's banner). Family, not geometry: a floating button legitimately + * carries a radius, a shadow, and a fixed size, so shape cannot separate them. + */ +const SURFACE_TOKEN_PATTERN = /^--dsw-(?:alias-bg-|specific-)/ + +/** + * The palette's own dark elevation ladder, resolved from `design-platform.css`: + * `bg-layer-2` and `bg-layer-3` are the rungs above the base surfaces, and the + * l1/l2 scrollbar split encodes exactly that step. Reading it from the palette + * rather than from the sheets that happen to rebind is what lets the check flag + * a surface NOBODY has rebound yet. + * @returns surface tokens whose dark value sits on an elevated rung. + */ +function elevatedRungs(): Set { + const definitions = new Map() + for (const rule of platformRules) { + // Dark declarations come later in the sheet and overwrite the light ones, + // which is the palette this distinction exists in. + for (const [property, value] of rule.declarations) definitions.set(property, value) + } + const resolve = (name: string): string => { + const seen = new Set() + let current = name + while (definitions.has(current) && !seen.has(current)) { + seen.add(current) + const value = definitions.get(current)! + const [reference] = varReferences(value) + if (reference === undefined) return value + current = reference + } + return current + } + const rungs = new Set([resolve('--dsw-alias-bg-layer-2'), resolve('--dsw-alias-bg-layer-3')]) + const tokens = new Set() + for (const name of definitions.keys()) { + if (SURFACE_TOKEN_PATTERN.test(name) && rungs.has(resolve(name))) tokens.add(name) + } + return tokens +} + +const elevatedSurfaces = elevatedRungs() for (const file of packageStylesheets()) { const rules = parseRules(readFileSync(file, 'utf8')) - const surfaces: SheetSurfaces = { rebound: new Set(), painted: new Set(), scrolls: false, rebinds: false } + const surfaces: SheetSurfaces = { elevated: new Set(), scrolls: false, rebinds: false } for (const rule of rules) { let rebinds = false const ruleSurfaces: string[] = [] @@ -206,25 +251,17 @@ for (const file of packageStylesheets()) { referencedTokens.set(token, [...referencedTokens.get(token) ?? [], file]) } } - for (const token of ruleSurfaces) surfaces.painted.add(token) + for (const token of ruleSurfaces) { + if (elevatedSurfaces.has(token)) surfaces.elevated.add(token) + } if (rebinds) { rebindRules.push({ file, rule }) surfaces.rebinds = true - for (const token of ruleSurfaces) surfaces.rebound.add(token) } } sheetSurfaces.set(file, surfaces) } -/** - * Surface tokens known to be elevated, derived from the sheets that already - * rebind rather than listed here: a rebinding rule paints the surface whose - * elevation it is declaring. Deriving it means a new elevated surface joins the - * set by rebinding, and cannot be added to the palette without either rebinding - * or failing the check below. - */ -const elevatedSurfaces = new Set([...sheetSurfaces.values()].flatMap(surfaces => [...surfaces.rebound])) - describe('design-platform.css scrollbar tokens', () => { it('defines the same scrollbar token set in the light and the dark block', () => { // A token present only in the light block silently keeps its light value @@ -426,24 +463,44 @@ describe('elevated surface rebinds', () => { } }) - it('every sheet that scrolls on a known elevated surface rebinds', () => { + it('resolves the elevated surface set from the palette ladder', () => { + // The set has to come from the palette, not from the sheets that happen to + // rebind: derived from rebinds it can only confirm what someone already + // remembered, and a surface nobody has rebound yet — the case the check + // exists for — would define itself as unelevated. Anchoring it here means a + // new palette token on an elevated rung is in scope the moment it is + // defined. `--dsw-specific-tip` is the regression that proved the point: it + // resolves to the same dark rung as the menu surface, and the Todo panel + // scrolled on it unrebound while a rebind-derived set stayed green. + expect(elevatedSurfaces).toContain('--dsw-alias-bg-layer-2') + expect(elevatedSurfaces).toContain('--dsw-alias-bg-layer-3') + expect(elevatedSurfaces).toContain('--dsw-specific-menu') + expect(elevatedSurfaces).toContain('--dsw-specific-input-major') + expect(elevatedSurfaces).toContain('--dsw-specific-tip') + // Base surfaces stay out, or every scroll container would be in scope and + // the check would say nothing. + expect(elevatedSurfaces).not.toContain('--dsw-alias-bg-base') + expect(elevatedSurfaces).not.toContain('--dsw-alias-bg-layer-1') + }) + + it('every sheet that scrolls on an elevated surface rebinds', () => { // The failure this closes: a scroll container on an elevated surface that // nobody remembered to rebind renders the l1 thumb, which differs from l2 - // only in the dark palette and only for that one surface — invisible in - // review and in a light-palette screenshot. Three sheets shipped that way - // (ui-primitives Menu, InputBar, QuestionComposer) and review caught them - // by hand, which is what this replaces. + // only in the dark palette and only for that one surface — invisible both in + // review and in a light-palette screenshot. Four sheets shipped that way + // (ui-primitives Menu, InputBar, QuestionComposer, TodoPanel) and review + // caught them by hand, which is what this replaces. // // Surface-level, not element-level: the elevated card and the descendant // that scrolls are separate rules, and CSS text does not say which contains - // which. A sheet that both scrolls somewhere and paints a known elevated - // surface somewhere must rebind; the elevation would otherwise be a - // coincidence of two unrelated rules, which no sheet under test does. - expect(elevatedSurfaces.size).toBeGreaterThan(0) + // which. What keeps that from over-reporting is the token FAMILY: only + // `--dsw-alias-bg-*` and `--dsw-specific-*` name a surface, so a floating + // button or an inline code span reaching the same rung is out of scope + // (ChatView's `.toBottom`, CodeBlock's banner). Geometry cannot make that + // call — a floating button carries a radius, a shadow, and a fixed size. for (const [file, surfaces] of sheetSurfaces) { if (!surfaces.scrolls || surfaces.rebinds) continue - const elevated = [...surfaces.painted].filter(token => elevatedSurfaces.has(token)) - expect(elevated, `${file} scrolls on ${elevated.join(', ')} without rebinding`).toEqual([]) + expect([...surfaces.elevated], `${file} scrolls on an elevated surface without rebinding`).toEqual([]) } }) })