diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.i18n.yaml index 15e96b5788..bd961e16dc 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.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-31-composer-glyph-layer-tracks-the-textarea.md -2026-07-31-composer-glyph-layer-tracks-the-textarea.md: b9249d59f12cd7bcb2267ce46168e9049517d307 -2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md: 68d8cc29e30d25fa25f79617e1e7d9e28e4edff8 +2026-07-31-composer-glyph-layer-tracks-the-textarea.md: ca5bc407f85199970854ec4853acc0843addae79 +2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md: b662cb35563770a1cab5df006ccc33a27e2ef4de diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.md b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.md index b9249d59f1..ca5bc407f8 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.md @@ -16,12 +16,11 @@ The defect is therefore exactly as old as the cap, and it hid behind the resting ## Decision -`InputBar` mirrors the textarea's `scrollTop` onto the backdrop, from two places: +`InputBar` mirrors the textarea's `scrollTop` onto the backdrop from one `scroll` listener, registered beside the existing wheel-chaining listener in the same effect (the textarea is never unmounted — the inert state renders the same element disabled). -- A `scroll` listener on the textarea, registered beside the existing wheel-chaining listener in the same effect (the textarea is never unmounted — the inert state renders the same element disabled). This covers every gesture and every caret-driven scroll the browser performs. -- A layout effect keyed on the committed draft. An edit reflows both layers without necessarily moving the textarea — no `scroll` event fires while the caret stays in view — and a shrinking draft clamps each layer independently. +One listener is the whole coupling, because every way the box moves ends in a `scroll` event on the textarea. A gesture scrolls it; an edit scrolls the caret into view; a draft that shrinks past the current offset clamps it. The clamp case is the one that looks like it needs separate handling and does not: the two layers share an extent — measured equal in chromium for plain, soft-wrapped, and unbreakable-run drafts — so they clamp to the same maximum, and the textarea's clamp fires the `scroll` that mirrors it. -Both write the textarea's offset to the backdrop, never the reverse: the textarea is the authority because it owns the caret, and the caret is what the browser scrolls to. +The mirror is one-directional: the textarea is the authority because it owns the caret, and the caret is what the browser scrolls to. ## Alternatives considered @@ -33,21 +32,23 @@ Both write the textarea's offset to the backdrop, never the reverse: the textare **Scroll the backdrop from the existing wheel handler instead of a `scroll` listener.** The handler already runs on every wheel over the textarea, so it looks like the natural place. Rejected because it covers only one of the ways the box scrolls: typing at the end, `End`, arrow keys, drag-selection past the edge, and scrollbar drags all move the textarea without a wheel event. Listening to `scroll` is listening to the thing itself rather than to one of its causes. -**Sync in the `onChange` handler rather than a layout effect.** Rejected because it fires before React commits the new draft to the backdrop, so it would mirror against the previous layout. The layout effect runs after the commit and before paint, which is where the two layers are simultaneously current. +**Add a second mirror in a layout effect keyed on the committed draft.** This shipped in the first version of the change, on the theory that an edit reflows both layers without necessarily moving the textarea, and that a shrinking draft clamps each layer independently. Both premises are false, and it was removed after mutation-testing each hook alone against the built client: with only the layout effect disabled the browser scenario stays green, while disabling only the `scroll` listener fails it. Typing scrolls the caret into view, which is an ordinary `scroll`; a shrinking draft clamps both layers to the same maximum because their extents are equal, and the textarea's clamp fires `scroll` too. The specific hazard the effect was imagined to cover — React replacing the backdrop's children when the decoration set changes shape, resetting its offset — does not occur: measured in chromium, replacing every child of an `overflow: hidden` box preserves `scrollTop` (300 stays 300), and the only replacement that zeroes it is one that shrinks the content below the offset, which is the clamp case already covered. + +**Sync in the `onChange` handler.** Rejected for the same reason plus one of its own: it fires before React commits the new draft to the backdrop, so it would mirror against the previous layout. ## Consequences - A draft past the cap scrolls its glyphs. Measured in the browser scenario: after a wheel gesture over a 40-line draft the last line sits inside the visible box and the first has scrolled out above it; before, the last line stayed a full draft-height below the box while the textarea's own offset had moved. -- The coupling is one-directional and cheap — two assignments of one number, no measurement, no layout read beyond `scrollTop` — so it adds nothing to the typing path's cost. +- The coupling is one-directional and cheap — one assignment of one number, no measurement, no layout read beyond `scrollTop` — so it adds nothing to the typing path's cost. - Chips, claim-token highlights, and text-ref marks stay aligned with their glyphs while scrolled, because they are positioned inside the backdrop and move with it. Nothing about the decoration walk changes. - The composer's two-layer design keeps this hazard: any future layer added beside the backdrop needs the same mirroring. The e2e scenario asserts the relation the user cares about (which line is on screen) rather than the mechanism, so it holds whatever the layer count becomes. ## Testing -The unit spec in [input-bar.spec.tsx](../../../../packages/client/ui-conversation/tests/input-bar.spec.tsx) proves the mirroring paths run: it stubs both offsets, because jsdom reports `scrollHeight === clientHeight` for every element and never scrolls one, and asserts the backdrop follows a textarea `scroll` and a committed edit. Reverting the `ref` makes it fail. +The unit spec in [input-bar.spec.tsx](../../../../packages/client/ui-conversation/tests/input-bar.spec.tsx) proves the mirroring path runs: it stubs both offsets, because jsdom reports `scrollHeight === clientHeight` for every element and never scrolls one, and asserts the backdrop follows the textarea to a new offset and back to the top. Reverting the `ref` makes it fail. The user-visible fact needs a real engine, so [composer-draft-scroll.e2e.ts](../../../../apps/web/tests/composer-draft-scroll.e2e.ts) measures it in chromium against the built client: a 40-line draft in a fresh workspace's blank composer, zero model calls, with a DOM Range over the backdrop's own text reporting where the first and last lines sit relative to the visible box. A vacuity guard asserts the draft actually overflows the capped box first. -Confirmed both directions against the built client. With the mirroring reverted and the packages rebuilt, the wheel case fails on the layer offsets, the edit case fails with it, and the golden diff reads `last draft line is on screen: false` while `textarea moved: true` — the reported symptom stated as a fixture. The resting-state case passes in both builds, which is the point: it is the state that hid the defect. +Confirmed both directions against the built client. With the mirroring reverted and the packages rebuilt, the wheel case fails on the layer offsets, the typing case fails with it, and the golden diff reads `last draft line is on screen: false` while `textarea moved: true` — the reported symptom stated as a fixture. The resting-state case passes in both builds, which is the point: it is the state that hid the defect. Note that the composer ships inside a client-module bundle, so `pnpm run build:web` alone does not pick up a change to `InputBar.tsx` — the package build must run for the browser lane to see it, and a scenario run against a stale `lib/` asserts against an older client than the tree. diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md index 68d8cc29e3..b662cb3556 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-composer-glyph-layer-tracks-the-textarea.zh.md @@ -16,12 +16,11 @@ Status: implemented ## 决策 -`InputBar` 把 textarea 的 `scrollTop` 镜像到 backdrop 上,来自两处: +`InputBar` 通过一个 `scroll` 监听把 textarea 的 `scrollTop` 镜像到 backdrop 上,该监听与既有的滚轮接力监听注册在同一个 effect 中(textarea 从不卸载——失效状态渲染的是同一个元素的 disabled 形态)。 -- textarea 上的 `scroll` 监听,与既有的滚轮接力监听注册在同一个 effect 中(textarea 从不卸载——失效状态渲染的是同一个元素的 disabled 形态)。它覆盖所有手势,以及浏览器因光标而执行的每一次滚动。 -- 一个以已提交草稿为 key 的 layout effect。一次编辑会让两层重排,却不一定让 textarea 移动——光标仍在可见范围内时不会触发 `scroll` 事件——而草稿变短时两层各自独立地被钳位。 +一个监听即构成完整耦合,因为这个盒子移动的每一种方式最终都会在 textarea 上产生 `scroll` 事件:手势使它滚动;编辑会把光标滚入可见范围;草稿缩短到当前偏移之下时它会被钳位。看似需要单独处理、实则不需要的正是钳位这一种:两层共享同一滚动范围——在 chromium 中对纯文本、软折行与不可断长串三类草稿实测均相等——因此它们会钳位到同一个最大值,而 textarea 的钳位本身就会触发那次完成镜像的 `scroll`。 -两者都把 textarea 的偏移写给 backdrop,绝不反向:textarea 是权威方,因为它持有光标,而浏览器滚动的目标正是光标。 +该镜像是单向的:textarea 是权威方,因为它持有光标,而浏览器滚动的目标正是光标。 ## 曾考虑的替代方案 @@ -33,21 +32,23 @@ Status: implemented **在既有的滚轮处理函数里滚动 backdrop,而不是新增 `scroll` 监听。** 该处理函数本就在 textarea 上的每次滚轮时运行,看似是自然的落点。之所以否决,是它只覆盖了盒子滚动的其中一种成因:在末尾输入、`End`、方向键、拖选越过边缘、拖动滚动条,都会在没有滚轮事件的情况下移动 textarea。监听 `scroll` 是在监听事情本身,而不是它的某一个成因。 -**在 `onChange` 处理函数里同步,而不用 layout effect。** 之所以否决,是它在 React 把新草稿提交到 backdrop 之前触发,因而会按上一次的布局做镜像。layout effect 在提交之后、绘制之前运行,那正是两层同时处于最新状态的时刻。 +**再加一个以已提交草稿为 key 的 layout effect 作为第二道镜像。** 该改动的第一版确实带着它,理由是:一次编辑会让两层重排却不一定让 textarea 移动,且草稿变短时两层各自独立地被钳位。这两个前提都不成立,因此在针对构建产物客户端逐个变异测试每个 hook 之后将其移除:仅禁用 layout effect 时浏览器场景全绿,而仅禁用 `scroll` 监听则会失败。输入会把光标滚入可见范围,那就是一次普通的 `scroll`;草稿变短时两层因范围相等而钳位到同一个最大值,且 textarea 的钳位同样会触发 `scroll`。该 effect 本想覆盖的那个具体隐患——React 在装饰集合形状变化时替换 backdrop 的全部子节点,从而重置其偏移——并不会发生:在 chromium 中实测,替换一个 `overflow: hidden` 盒子的全部子节点会保留 `scrollTop`(300 仍为 300),唯一会将其归零的替换是把内容缩短到偏移之下,而那正是已被覆盖的钳位情形。 + +**在 `onChange` 处理函数里同步。** 除上述同样的理由外还有其自身的问题:它在 React 把新草稿提交到 backdrop 之前触发,因而会按上一次的布局做镜像。 ## 后果 - 超过上限的草稿会滚动其字形。浏览器场景实测:在 40 行草稿上做一次滚轮手势后,最后一行位于可见盒子之内,第一行已滚出上方;此前最后一行仍停在盒子下方整整一个草稿高度处,而 textarea 自身的偏移已经移动了。 -- 该耦合是单向且廉价的——两次对一个数字的赋值,没有测量,除 `scrollTop` 外没有额外的布局读取——因此不会给输入路径增加开销。 +- 该耦合是单向且廉价的——一次对一个数字的赋值,没有测量,除 `scrollTop` 外没有额外的布局读取——因此不会给输入路径增加开销。 - chip、claim token 高亮与文本引用标记在滚动时始终与其字形对齐,因为它们定位在 backdrop 内部并随之移动。装饰扫描本身没有任何改动。 - composer 的双层设计保留了这一隐患:日后在 backdrop 旁新增的任何一层都需要同样的镜像。e2e 场景断言的是用户真正关心的关系(哪一行在屏幕上),而非实现机制,因此无论层数变成多少它都成立。 ## 验证 -[input-bar.spec.tsx](../../../../packages/client/ui-conversation/tests/input-bar.spec.tsx) 中的单元用例证明镜像路径确实执行:它对两侧偏移都做了桩替换——因为 jsdom 对任何元素都报告 `scrollHeight === clientHeight` 且从不滚动任何元素——并断言 backdrop 跟随 textarea 的 `scroll` 与一次已提交的编辑。撤掉那个 `ref` 会让它失败。 +[input-bar.spec.tsx](../../../../packages/client/ui-conversation/tests/input-bar.spec.tsx) 中的单元用例证明镜像路径确实执行:它对两侧偏移都做了桩替换——因为 jsdom 对任何元素都报告 `scrollHeight === clientHeight` 且从不滚动任何元素——并断言 backdrop 既跟随 textarea 到新的偏移,也跟随它回到顶部。撤掉那个 `ref` 会让它失败。 用户可见的事实需要真实引擎,因此 [composer-draft-scroll.e2e.ts](../../../../apps/web/tests/composer-draft-scroll.e2e.ts) 在 chromium 中针对构建产物客户端测量它:在全新工作区空白会话的 composer 中放入 40 行草稿,零模型调用,用一个跨越 backdrop 自身文本的 DOM Range 报告首行与末行相对于可见盒子的位置。一个防空转守卫会先断言草稿确实溢出了设有上限的盒子。 -已双向确认。撤掉镜像并重新构建各包后,滚轮用例在两层偏移上失败,编辑用例随之失败,golden 差异读作 `last draft line is on screen: false` 而 `textarea moved: true`——即以 fixture(测试前置数据)形式陈述的原始现象。静止状态用例在两种构建下都通过,这正是要点所在:它就是掩盖了该缺陷的那个状态。 +已双向确认。撤掉镜像并重新构建各包后,滚轮用例在两层偏移上失败,输入用例随之失败,golden 差异读作 `last draft line is on screen: false` 而 `textarea moved: true`——即以 fixture(测试前置数据)形式陈述的原始现象。静止状态用例在两种构建下都通过,这正是要点所在:它就是掩盖了该缺陷的那个状态。 注意 composer 随客户端模块 bundle 一同发布,因此仅运行 `pnpm run build:web` 不会纳入对 `InputBar.tsx` 的改动——必须运行包构建,浏览器测试通道才能看到它;针对陈旧 `lib/` 运行的场景,断言的是比当前工作树更旧的客户端。 diff --git a/apps/web/tests/composer-draft-scroll.e2e.ts b/apps/web/tests/composer-draft-scroll.e2e.ts index bb414e7e8f..cc3e5b46bb 100644 --- a/apps/web/tests/composer-draft-scroll.e2e.ts +++ b/apps/web/tests/composer-draft-scroll.e2e.ts @@ -230,12 +230,12 @@ describe('web e2e: composer draft scrolling', () => { expect(tripwire.pageErrors).toEqual([]) }, 60_000) - it('an edit re-mirrors the layers without a scroll event', async () => { + it('typing at the end of a scrolled draft keeps the layers together', async () => { onTestFailed(() => saveFailureShot(page, 'web-e2e-composer-draft-scroll-edit')) - // Typing at the caret — parked at the draft's end by the wheel gesture — - // reflows both layers. The textarea need not scroll (the caret stays in - // view), so nothing fires the `scroll` listener and only the draft-keyed - // mirror keeps the layers together. + // The other way the box moves. Typing at the caret — parked at the draft's + // end by the wheel gesture — scrolls it into view, which is a `scroll` like + // any other; this pins that an edit is not a separate case needing its own + // mirror, which is why one listener is the whole implementation. const input = page.locator('textarea:enabled').first() await input.press('End') await input.pressSequentially(' tail') diff --git a/packages/client/ui-conversation/src/client/skeleton/InputBar.tsx b/packages/client/ui-conversation/src/client/skeleton/InputBar.tsx index a5e217235f..354a4a5f20 100644 --- a/packages/client/ui-conversation/src/client/skeleton/InputBar.tsx +++ b/packages/client/ui-conversation/src/client/skeleton/InputBar.tsx @@ -6,7 +6,7 @@ * region-slot content) ride the owner props. Session facts * (running/removed/promptError) are self-selected via useSession. */ -import { useEffect, useLayoutEffect, useRef } from 'react' +import { useEffect, useRef } from 'react' import type { ChangeEvent, KeyboardEvent, MouseEvent, ReactNode } from 'react' import clsx from 'clsx' import { IconPlusOutline16 } from '@deepseek-ai/dsh-client-ui-primitives' @@ -104,7 +104,10 @@ export function InputBar({ // scroll — the backdrop paints every visible glyph (the textarea's own text // is transparent) but is clipped, not scrolled, so it does not follow the // textarea on its own: without this mirror a draft past the cap moves the - // caret while the words stay frozen in place. + // caret while the words stay frozen in place. Every way the box moves ends + // in a `scroll` event, edits included (the caret is scrolled into view), and + // the layers share an extent, so a draft that shrinks past the offset clamps + // both to the same maximum — one listener covers the coupling. useEffect(() => { const el = inputRef.current if (el === null) return @@ -129,16 +132,6 @@ export function InputBar({ } }, []) - // Draft edits reflow both layers without necessarily moving the textarea - // (no scroll event fires when the caret stays in view), and a shrinking - // draft clamps each layer independently. Re-mirror after every committed - // draft so the glyphs never lag the caret by an edit. - useLayoutEffect(() => { - const el = inputRef.current - const backdropEl = backdropRef.current - if (el !== null && backdropEl !== null) backdropEl.scrollTop = el.scrollTop - }, [draft]) - const onKeyDown = (e: KeyboardEvent): void => { // Absent machine (no session): the textarea is disabled so events cannot // fire; the guard narrows the faces for the paths below. diff --git a/packages/client/ui-conversation/tests/input-bar.spec.tsx b/packages/client/ui-conversation/tests/input-bar.spec.tsx index 6ce98d403a..c5ca9fcf9a 100644 --- a/packages/client/ui-conversation/tests/input-bar.spec.tsx +++ b/packages/client/ui-conversation/tests/input-bar.spec.tsx @@ -297,11 +297,11 @@ describe('running and lock semantics (queue cut 1)', () => { textarea.scrollTop = 120 fireEvent.scroll(textarea) expect(backdrop.scrollTop).toBe(120) - // Editing re-mirrors without a scroll event (the caret can stay in view). - backdrop.scrollTop = 0 - textarea.scrollTop = 96 - fireEvent.change(textarea, { target: { value: 'line\n'.repeat(39) } }) - expect(backdrop.scrollTop).toBe(96) + // Every later move tracks too, including back to the top — a one-shot + // mirror would leave the glyphs parked at the first offset it saw. + textarea.scrollTop = 0 + fireEvent.scroll(textarea) + expect(backdrop.scrollTop).toBe(0) }) it('disabled state shows the unavailable placeholder; custom placeholder wins', () => {