Fix long session timeline scrolling - #6526
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: The PR describes a real, observable UI issue — long session timelines expose a native gray scrollbar, clip tooltip details, and make earliest turns hard to select. The author tested with a specific long session ( Direction: Aligned. This is a focused UI usability fix for the web-shell timeline — keeping it usable for dense sessions. No direction concerns. Size: Not applicable (no core paths touched; all changes are in Approach: The scope feels right — 3 files, all focused on the timeline component. CSS changes for scrollbar hiding and centering, TSX changes for scroll management and floating detail layer. No unrelated changes visible. The 71 new test lines provide regression coverage. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:PR 描述了一个真实可观测的 UI 问题——长时间轴暴露灰色原生滚动条、裁剪详情 tooltip、最早轮次难以选中。作者用具体长会话( 方向:对齐。这是一个针对 web-shell 时间轴的 UI 可用性修复,保持密集会话下的可用性。无方向顾虑。 规模:不适用(未触及核心路径,所有改动在 方案:范围合理——3 个文件,都聚焦在时间轴组件。CSS 隐藏滚动条并居中,TSX 管理滚动和浮动详情层。未见无关改动。71 行新测试提供回归覆盖。进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe changes are clean and well-focused. Three things stand out as good decisions:
No critical blockers. No security issues. The One observation (not a blocker): the tooltip clamping logic adjusts TestingUnit tests — 38/38 pass Build — web-shell builds successfully Real-scenario (tmux) — Not applicable. This is a browser-rendered React component (web shell UI). Cannot be tested via CLI tmux. The author tested manually with Chrome on macOS using a real 60+ turn session ( 中文说明代码审查改动干净且聚焦。三个好的设计决策:
无关键阻塞问题。无安全问题。 测试单元测试 — 38/38 通过 — Qwen Code · qwen3.7-max |
|
This is a solid, focused UI fix. The author identified real usability issues with the long session timeline (native scrollbar leaking through, tooltip clipping, hard-to-reach first entries) and solved them with clean, well-scoped changes. The viewport wrapper with hidden scrollbar + mask-image fade is the right approach. Moving the tooltip to a single floating element outside the scroll container correctly solves the clipping problem. The Tests pass (38/38), build succeeds, and the diff carries no unrelated changes. The only gap is Windows/Linux browser verification, which the author flagged honestly. Approving. ✅ 中文说明这是一个扎实、聚焦的 UI 修复。作者识别了长时间轴的真实可用性问题(原生滚动条泄漏、tooltip 裁剪、首个 entry 难以选中),用干净且范围合理的改动解决了它们。 Viewport 包装器配合隐藏滚动条 + mask-image 渐变是正确方法。将 tooltip 移到滚动容器外的单个浮动元素正确解决了裁剪问题。 测试通过(38/38),构建成功,diff 无无关改动。唯一的缺口是 Windows/Linux 浏览器验证,作者已诚实标注。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
MessageList.tsx:1829 |
Unused panel variable in clamping useLayoutEffect — declared and null-checked but never referenced after the guard |
Remove the variable; check panelRef.current directly in the guard |
MessageList.tsx:1772-1777 |
buildTooltip can return null when panelRef.current is null, but syncTooltip and showTooltip pass the result to setTooltip without null guard, silently clearing tooltip state |
Add ?? current fallback in syncTooltip and null guard in showTooltip |
MessageList.tsx:1750 |
findTooltipAnchor uses querySelectorAll + Array.from().find() (O(n)) instead of querySelector('[data-turn-id="..."]') (O(1)) |
Replace with direct querySelector using entry.id |
MessageList.tsx:1819 |
Resize listener without debounce — each resize triggers getComputedStyle + getBoundingClientRect + setState |
Throttle with requestAnimationFrame |
MessageList.tsx:1831 |
Tooltip clamping only adjusts top, no horizontal clamping — left: rect.right + 8 can overflow viewport on narrow screens |
Add horizontal clamp or flip logic when rect.right + tooltipWidth > window.innerWidth |
MessageList.module.css |
z-index: 999 is below project convention (DialogShell: 1000, WebShellSidebar: 1000+, ChatEditor popover: 1000) |
Use z-index: calc(var(--web-shell-tooltip-z-index, 1000) + 1) |
MessageList.dom.test.tsx |
onMouseLeave on <nav> and per-button onMouseEnter/onMouseLeave never tested — all tooltip tests use focus events only |
Add mouse event dispatch tests for show/hide |
MessageList.dom.test.tsx |
No test asserts role="tooltip" on the portal element — a11y contract unverified |
Add expect(detail?.getAttribute('role')).toBe('tooltip') |
MessageList.dom.test.tsx |
useLayoutEffect cleanup (cancelAnimationFrame + clearTimeout + programmaticScrollRef=false) untested with rapid currentIndex changes |
Add test that re-renders without await nextFrame() between index changes |
— qwen3.7-max via Qwen Code /review
Runtime verification of #6526 (real daemon + browser, 81-turn session)Verdict: the three reported bugs are real and this PR fixes all of them. I found 3 issues in the fix itself — one of which I'd fix before merging, because it defeats one of the PR's own stated goals on any viewport shorter than ~970px. How I verified (worktree, A/B method, no mocked DOM)
What the PR fixes — confirmed
Gates: Finding 1 —
|
| viewport | clipBottom | current-turn marker visible | last marker visible |
|---|---|---|---|
| 1920×1080 | −54px | ✅ | ✅ |
| 1680×1000 | −14px | ✅ | ✅ |
| 1680×972 | 0px | ✅ | ✅ |
| 1512×860 (14" MBP viewport) | +56px | ✅ | ❌ |
| 1680×800 | +56px | ❌ | ❌ |
| 1440×790 (13" MBP viewport) | +56px | ❌ | ❌ |
The threshold is ~972px. Below it the rail is cut; below ~860px the current-turn marker is never painted — which contradicts the PR's own goal ("the current marker is kept visible"). It gets worse as the composer grows with a multi-line prompt. This is an incomplete fix rather than a fresh regression (main clips 178.5px off both ends at 1680×1000), but it's avoidable.
Suggested fix — revert the layer to top: 50%. Now that the rail has a bounded height, a sticky top: 50% resolves against .list's content box and centres the rail on the transcript. Verified on top of this PR:
panelCenterY == listCenterYexactly at 700 / 800 / 900 / 1000pxclipTopandclipBottomboth negative (slack) at all four heights- current + last markers visible at all four heights
Worth reconsidering max-height: min(640px, max(220px, calc(100vh - 220px))) (:209) for the same reason — it is sized against 100vh, not against the scroller it lives in.
Finding 2 — the portalled tooltip loses the app font and renders in Times New Roman
createPortal(…, document.body) (MessageList.tsx:1881-1909) moves the tooltip outside .app, and .app { font-family: var(--font-sans) } is the only thing that sets the app font. Nothing in .sessionTimelineDetails* declares font-family, so the tooltip inherits the UA default.
Measured on the rendered tooltip: font-family: "Times New Roman"; font-size: 16px vs the app's -apple-system, BlinkMacSystemFont, "Segoe UI", system-ui, sans-serif; 14px. (You can see it in the "AFTER" panel of the first screenshot above.)
Note SESSION_TIMELINE_TOOLTIP_THEME_VARS (:1686) already copies --font-sans onto the tooltip's inline style — but no rule consumes it, so the copy is dead today. One line makes it live:
.sessionTimelineDetails {
font-family: var(--font-sans, system-ui, sans-serif);
}Verified: computed font becomes the app font.
Two related nits on that same var list:
--web-shell-popover-z-indexis only ever consumed (always with the1000fallback) and never defined anywhere in the repo, socalc(var(…, 1000) + 2)always resolves to1002today. Copying it is defensive but currently inert — fine to keep, just noting it isn't doing anything.- The hand-maintained list is a trap: any
var()added to the tooltip subtree later will silently break. Portalling into the theme root instead ofdocument.bodywould remove the whole mechanism. I checked the.appelement:transform: none,filter: none,contain: none,container-type: normal— it does not establish a containing block forposition: fixed, and itsoverflow: hiddendoes not clip fixed descendants. It also carries the_themeDark_/_themeLight_class. So a portal into it inherits every var and the font, andposition: fixedstill behaves exactly as it does today.
Finding 3 — the programmaticScrollRef guard orphans the tooltip for keyboard users
Commit fdbd9ab added a guard so the auto-recentring scroll doesn't dismiss a visible tooltip. Measured behaviour:
- Pointer: the guard has no effect. When the rail scrolls under a stationary cursor, Chromium re-fires boundary events, so React's
onMouseLeave/onMouseEnterre-anchor the tooltip to whatever marker is now under the pointer (measured tooltip↔marker offset: 0px). Correct either way. - Keyboard focus: there are no boundary events. Focus marker
user-7, then let a new turn arrive → the rail recenters (scrollTop 0 → 642), the tooltip stays at its capturedtop: 270.5, and its own marker moves toy = −371.5. The tooltip ends up 642px from the turn it describes, floating next to an unrelated marker, witharia-describedbystill pointing at it. The same staleness shows up on window resize (1000 → 700: tooltip stays at235.6, its marker moves to−618).
So the guard converts "tooltip disappears" (harmless) into "tooltip points at the wrong turn" (misleading). Suggestion: on programmatic recentre, re-measure from the anchor element (call showTooltip(entry, anchorEl) again) or hide the tooltip when its anchor leaves the rail viewport.
Not covered
- Firefox / Safari. Only Chromium is installed here, so the
scrollbar-width: none(Gecko) path is verified by computed style only, and the WebKit pseudo-element path only under Blink. - Scrollbar pixels. Headless Chromium uses overlay scrollbars —
offsetWidth − clientWidthis0in both arms — so no screenshot can show the scrollbar appearing/disappearing. The computed-style A/B above is the proof; your macOS Chrome check covers the visual. - The
@media (max-width: 720px)rule wasn't exercised independently: the rail is already hidden at that width by the existingcontainer ≥ 1160pxgate.
Nice work — the portal + bounded viewport is the right shape for this. Finding 1 is a one-word change and I'd take it before merge; Findings 2 and 3 are small and can land here or as a follow-up.
中文版
#6526 的真实运行时验证(真实 daemon + 浏览器,81 轮长会话)
结论:PR 描述的三个问题都真实存在,本 PR 都修好了。但我在修复本身里发现了 3 个问题,其中第 1 个建议合并前处理——在视口高度低于约 970px 时,它恰好破坏了本 PR 自己声明的目标之一。
验证方式:worktree 检出 PR head fdbd9ab1c(与 main 的 merge-base 为 8296ce9e5,main 之后没有改动这两个源文件,PR 状态 MERGEABLE)。真实 qwen serve daemon + 真实 vite dev server + 无头 Chromium(Playwright),暗色主题。通过 daemon REST 接口用 mock OpenAI 兼容 provider 灌了一个 81 轮的会话,再用 /session/<id>?token= 打开。A/B 只替换 MessageList.tsx 和 MessageList.module.css 两个文件,其余(daemon、transcript、node_modules、浏览器)完全一致,因此下面每一处差异都可归因于本 PR。候选修复也用同样方式验证:在 PR 之上打一行改动再测一次。
已确认修好的部分
| 声明 | 证据(1680×1000,81 轮) |
|---|---|
| 新增 rail viewport 隐藏原生滚动条 | .sessionTimelineViewport computed:scrollbar-width: none、::-webkit-scrollbar { display: none; width: 0px }。把两段覆盖规则从同一构建里删掉后变成 scrollbar-width: thin、::-webkit-scrollbar { display: inline; width: 8px }(继承自 .app *::-webkit-scrollbar)。双类名提高优先级的写法是必需的。 |
| rail 内部滚动 | scrollHeight 1267 / clientHeight 640,scrollTop 自动设为 627(即最大值)以居中当前轮次 |
| 首/末 marker 可达 | rail scrollTop → 0 后 marker 0 出现;点击它使正文 scrollTop 14775 → 0,"Q1 …" 进入视野。在 main 上 marker 0 位于 y = -178.5,在正文裁剪区之外,根本无法 hover 或点击。 |
| rail 滚动不带动正文 | 在 rail 上滚轮:rail 657 → 257 → 0,正文 scrollTop 始终为 14775,即使继续过量滚动 3000px(overscroll-behavior: contain 生效) |
| 详情浮层不再被裁剪 | main 上最靠上的可见 marker 的 tooltip 被裁掉 31.9px(标题整行消失)。PR:单个 role="tooltip" 节点 portal 到 <body>,position: fixed,z-index: 1002,在首/中/末 marker 以及 700px 视口高度下均 0px 溢出。 |
| DOM 节点数 | [data-testid=session-timeline-detail]:81 → 1 |
| 无障碍 | aria-describedby 精确挂在 hover/focus 的那个 marker 上,blur 后移除 |
| 减弱动画 | prefers-reduced-motion: reduce 下 animation-name: none |
质量门禁:MessageList.dom.test.tsx 40/40 通过。反证(revert-proof):保留 PR 的测试文件、把两个源文件回退到 merge-base → 5 个测试失败(4 个新增 + 1 个被修改的),说明新测试确实有效而非空转。web-shell 全量 client 测试:75 文件 / 1192 测试通过。tsc -p tsconfig.lib.json --noEmit 在两侧产生完全相同的 218 个报错(都是 worktree 里 sibling dist 未构建造成的环境噪音,MessageList.* 零报错)。Prettier 3.6.1 检查通过。
问题 1 — top: 50vh 让 rail 溢出正文容器;视口高度低于约 860px 时当前轮次 marker 完全不可见
MessageList.module.css:185 把 sticky layer 锚在 top: 50vh,也就是相对浏览器视口居中。但 rail 的裁剪矩形是 .list(正文滚动容器),它的底边在窗口底部上方约 154px(输入框占位)。于是 rail 底部被裁掉。
clipBottom = panel.bottom − list.bottom(为正即被裁),宽 1680px,81 轮:
| 视口 | clipBottom | 当前轮 marker 可见 | 末尾 marker 可见 |
|---|---|---|---|
| 1920×1080 | −54px | ✅ | ✅ |
| 1680×1000 | −14px | ✅ | ✅ |
| 1680×972 | 0px | ✅ | ✅ |
| 1512×860(14" MBP 视口) | +56px | ✅ | ❌ |
| 1680×800 | +56px | ❌ | ❌ |
| 1440×790(13" MBP 视口) | +56px | ❌ | ❌ |
临界点约 972px。低于它 rail 被裁;低于约 860px 当前轮次的 marker 根本不会被绘制——这与 PR 自己声明的"当前 marker 保持可见"相矛盾。多行输入让输入框变高时会更严重。这属于修复不彻底而非新引入的回归(main 在 1680×1000 下上下各裁掉 178.5px),但完全可以避免。
建议修复 —— 把 layer 改回 top: 50%。 既然 rail 现在有了高度上限,sticky 的 top: 50% 会相对 .list 的 content box 解析,从而相对正文居中。已在本 PR 之上验证:
- 700 / 800 / 900 / 1000px 下
panelCenterY == listCenterY完全相等 - 四种高度下
clipTop与clipBottom均为负(有余量) - 四种高度下当前 marker 与末尾 marker 都可见
同理建议重新考虑 max-height: min(640px, max(220px, calc(100vh - 220px)))(:209)——它是按 100vh 而不是按它所在的滚动容器来计算的。
问题 2 — portal 出去的 tooltip 丢失了 app 字体,退化成 Times New Roman
createPortal(…, document.body)(MessageList.tsx:1881-1909)把 tooltip 移出了 .app,而 .app { font-family: var(--font-sans) } 是唯一设置 app 字体的地方。.sessionTimelineDetails* 里没有任何 font-family 声明,于是 tooltip 继承了浏览器默认字体。
实测渲染出来的 tooltip:font-family: "Times New Roman"; font-size: 16px,而 app 是 -apple-system, BlinkMacSystemFont, "Segoe UI", system-ui, sans-serif; 14px。(在上面第一张截图的 "AFTER" 面板里就能看出来。)
注意 SESSION_TIMELINE_TOOLTIP_THEME_VARS(:1686)已经把 --font-sans 复制到了 tooltip 的行内样式上——但没有任何规则消费它,所以这次复制目前是死代码。一行即可让它生效:
.sessionTimelineDetails {
font-family: var(--font-sans, system-ui, sans-serif);
}已验证:computed 字体变回 app 字体。
关于同一份变量清单的两点补充:
--web-shell-popover-z-index在整个代码库里只被读取(且始终带1000兜底)、从未被定义过,所以calc(var(…, 1000) + 2)目前恒为1002。复制它属于防御性写法,但当前不起作用——保留无妨,只是说明它现在没有实际效果。- 这份手工维护的清单本身是个坑:以后往 tooltip 子树里加任何
var()都会静默失效。把 portal 目标从document.body换成主题根节点即可彻底去掉这套机制。 我检查过.app元素:transform: none、filter: none、contain: none、container-type: normal——它不会为position: fixed建立包含块,其overflow: hidden也不裁剪 fixed 后代;同时它带着_themeDark_/_themeLight_类名。所以 portal 到它上面能继承全部变量和字体,而position: fixed的行为与现在完全一致。
问题 3 — programmaticScrollRef 守卫让键盘用户看到"孤儿" tooltip
fdbd9ab 加了守卫,使自动居中的滚动不会关掉已显示的 tooltip。实测行为:
- 鼠标:守卫其实没起作用。当 rail 在静止的光标下滚动时,Chromium 会重新派发边界事件,React 的
onMouseLeave/onMouseEnter会把 tooltip 重新锚定到光标下的新 marker(实测 tooltip 与 marker 偏移 0px)。两种情况下结果都正确。 - 键盘焦点:没有边界事件。聚焦 marker
user-7,随后新一轮到达 → rail 重新居中(scrollTop 0 → 642),tooltip 停在原先记录的top: 270.5,而它自己的 marker 移动到y = −371.5。tooltip 与它所描述的轮次相距 642px,悬浮在一个毫不相干的 marker 旁边,而aria-describedby仍然指向它。窗口 resize 时也有同样的失效(1000 → 700:tooltip 停在235.6,marker 移到−618)。
所以这个守卫把"tooltip 消失"(无害)变成了"tooltip 指向错误的轮次"(有误导性)。建议:在程序化重新居中时,用锚点元素重新测量(再调一次 showTooltip(entry, anchorEl)),或在锚点离开 rail viewport 时隐藏 tooltip。
未覆盖的部分
- Firefox / Safari:本机只装了 Chromium,因此
scrollbar-width: none(Gecko)路径只做了 computed style 层面的验证,WebKit 伪元素路径也只在 Blink 下验证。 - 滚动条像素:无头 Chromium 使用 overlay 滚动条——两侧
offsetWidth − clientWidth都是0——所以任何截图都无法展示滚动条的出现/消失。上面的 computed style A/B 就是证据;你在 macOS Chrome 上的目视检查覆盖了视觉部分。 @media (max-width: 720px)规则没有被独立验证:在该宽度下现有的container ≥ 1160px门槛已经把 rail 隐藏了。
整体做得不错——portal + 有界 viewport 是正确的方向。问题 1 只需改一个词,建议合并前处理;问题 2、3 影响较小,在本 PR 里改或后续跟进都可以。
|
Thanks for the detailed verification. For Finding 1, I am intentionally keeping the current behavior. The session timeline is designed to be visually centered against the page/viewport, not centered inside the transcript body. Centering it against the transcript area makes the rail sit too high once the composer takes space at the bottom, which is the behavior this PR is deliberately avoiding. The small bottom overlap at shorter viewport heights is an acceptable tradeoff for this design. The rail itself is scrollable, so users can still scroll the timeline viewport to reach and select later markers. This matches the intended Codex-style interaction: the timeline remains visually centered in the user's view, while dense marker lists are navigated within the rail. So I do not think Finding 1 is a bug that needs to be fixed in this PR. Findings 2 and 3 were valid and have been addressed in |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Maintainer verification: local build + real-browser A/BI built this branch locally and drove the real Verdict: the behavioural core is correct and genuinely fixes the reported bugs. One CSS change ( ✅ Gates
✅ Confirmed fixes (measured, not eyeballed)
🔴 Should fix before merge:
|
| viewport × composer | main (50%) |
this PR (50vh) |
this PR + top: 50% |
|---|---|---|---|
| 900 × 132 | 0 | +78 | 0 |
| 900 × 200 | 0 | +112 | 0 |
| 900 × 260 | 0 | +142 | 0 |
| 900 × 360 | 0 | +192 | 0 |
| 800 × 180 | 0 | +102 | 0 |
| 700 × 132 | 0 | +78 | 0 |
| 700 × 260 | 0 | +142 | 0 |
Rail bottom clipped by the chat scrollport (px) / is the current turn's marker clickable:
| viewport × composer | main |
this PR | this PR + top: 50% |
|---|---|---|---|
| 900 × 132 | 210 / no | 14 / yes | 0 / yes |
| 900 × 200 | 244 / no | 82 / no | 0 / yes |
| 900 × 260 | 274 / no | 142 / no | 0 / yes |
| 900 × 360 | 324 / no | 242 / no | 50 / no |
| 800 × 180 | 284 / no | 82 / no | 0 / yes |
| 700 × 132 | 310 / no | 34 / no | 0 / yes |
| 700 × 260 | 374 / no | 162 / no | 20 / yes |
So the marker for the current turn — the one the new auto-centring code deliberately scrolls into view — stops being clickable in 6 of the 7 window/composer sizes I tested. Reverting only that one property, keeping every other line of this PR, restores exact centring and makes the last marker usable in 6/7. This also explains why it looked fine in your macOS Chrome check: at a tall viewport with an idle composer (the 900 × 132 row) it's the one configuration that still works.
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;Optional hardening: max-height: min(640px, max(220px, calc(100vh - 220px))) is window-relative for the same reason. With top: 50% restored, the only remaining clipped case is a 900px viewport with a fully-expanded 360px composer (50px). Sizing that cap against the chat scrollport instead of 100vh would close it.
🟡 Minor: the tooltip paints above modal dialogs
--web-shell-popover-z-index has no default definition, so calc(var(--web-shell-popover-z-index, 1000) + 2) = 1002. DialogShell is also createPortal(…, document.body) with .backdrop { z-index: 1000 }. Verified with a pixel probe: with an opaque z-1000 overlay covering the screen, the pixel at the tooltip's centre is the tooltip's own background. Usually harmless — opening a dialog moves focus and onBlur hides the tooltip — but a dialog raised while the pointer merely rests on a marker (e.g. an incoming tool-approval) will render under it.
🟡 Nit: the tooltip container's color is black
The portal escapes .sessionTimelinePanel { color: var(--muted-foreground) }, and color isn't in SESSION_TIMELINE_TOOLTIP_THEME_VARS. Every text child sets its own colour, so nothing renders black today — but .sessionTimelineDetails computes color: rgb(0, 0, 0) against a #171717 background. Adding color: var(--foreground) to the rule makes it safe for anyone who later adds a bare text node.
📋 Note on coverage
None of the 41 jsdom tests can observe top, max-height, the mask, or the scrollbar — jsdom has no layout engine. The five new tests pin the JavaScript behaviour well, but the layout half of this PR is invisible to CI, which is exactly how the 50vh issue got through. Worth keeping in mind for future timeline changes.
中文版
维护者验证:本地构建 + 真实浏览器 A/B 对比
我在本地构建了这个分支,并在真实 Chromium 中运行真实的 MessageList 组件(Playwright),用 80 轮长会话把 PR 和它的 merge base(8296ce9e5)并排对比。测试环境挂载了真实组件,并复刻了应用真实的 DOM/CSS 结构(.app.themeDark > .appShell > .content > .list 加一个真实的 .footer 输入框盒子);主题变量故意不放在 :root 上——这与真实应用一致,因为 --background/--foreground 是作用在 .app.themeDark 这个包裹元素上的。
结论:行为层面的核心改动是正确的,确实修复了所描述的问题。但其中一处 CSS 改动(top: 50vh)是净负面的,建议合并前回退。
✅ 各项检查
| 检查项 | 结果 |
|---|---|
npm run build --workspace=packages/web-shell(含 tsc -p tsconfig.lib.json) |
通过 |
npx vitest run(整个 web-shell 包) |
1265 通过 / 79 个文件 |
MessageList.dom.test.tsx |
41 通过(PR 描述里写的是 38) |
对 3 个改动文件跑 eslint |
0 错误 |
| 浏览器运行期间的 console / page 报错 | 0 |
✅ 已确认修复的问题(都是实测,不是肉眼估计)
-
最早/最新的轮次此前确实点不到,现在可以了。 在
main上,80 个 marker 里只有 52 个能命中自身;marker 0–13 和 66–79 落在聊天滚动容器之外。在时间轴上滚滚轮会让聊天正文滚动 −5000px,而且永远滚不出那些 marker。在这个 PR 上,时间轴内部滚动(scrollTop612→0),聊天滚动增量为 0(overscroll-behavior: contain生效),marker 0 变为可命中且 tooltip 正常显示。 -
Tooltip 裁剪问题是真实存在的。 在
main上,最顶部可用 marker 的 tooltip 会被.list的overflow-y: auto裁掉 33.4px——整个标题行被切没了,底部同理。在这个 PR 上,所有 marker 的裁剪量都是 0px:position: fixed,portal 到document.body,z-index: 1002。 -
隐藏滚动条的 CSS 是有效的(有"牙齿")。 时间轴 viewport 的计算样式为:
scrollbar-width: none、scrollbar-color: transparent transparent、::-webkit-scrollbar { display: none; width: 0; height: 0 }。把这个 PR 新增的那三条规则精确删掉之后,该元素就会回落到应用全局的.app *规则 →scrollbar-width: thin、::-webkit-scrollbar { display: inline; width: 8px },gutter 从0 变成 10px,并且时间轴右侧条带里会画出 1882 个#3a3a3a像素(--scrollbar-thumb)。所以双写类名 +!important确实是为了压过.app *,必要。(这里的 headless Chromium 根本画不出滚动条——我用一个强制 20px 绿色滚动条的对照组验证了这一点,之后改成在 Xvfb 下用有头模式来做像素级检查。) -
主题能穿过 portal 边界。 tooltip 现在位于
document.body,在.app.themeDark之外,所以themeVars的复制是关键的——而且它确实生效了:标题rgb(250,250,250),详情#808598 @ 86%,背景#171717。 -
在按住 hover 的同时滚动聊天,tooltip 会正确重新锚定——它跟随的是光标下方真实的那个 marker(第 52 轮),而不是过期的那个。
-
DOM / 无障碍收益: 空闲时的 detail 节点从 80 → 0;80 个
aria-hidden="true"的块被一个role="tooltip"+aria-describedby取代。 -
没有回归:低于 1160px 容器阈值时时间轴依然不渲染(在 1000px 和 600px 下都验证过),少于 4 个条目的会话也一样;4 条目的会话正常渲染、不可滚动、4 个 marker 全部可用。
-
5 个新增测试是有效的。 我对已合入的源码做了 4 处独立变异,每一处都被恰好对应的那个测试抓到(每次回退后都恢复 41/41):
变异 失败的测试 把 tooltip 改成内联渲染,去掉 createPortalrenders timeline details as one body-level tooltip…handleViewportScroll→ 空函数…hides them on user scroll+hides timeline details when the user scrolls…isTooltipAnchorVisible→ 恒为truehides timeline details when the focused marker moves out of view去掉 viewport.scrollTop = …这一句自动居中写入keeps timeline details during current-turn centering…
🔴 合并前建议修复:top: 50% → top: 50vh 会让时间轴偏离居中并被裁掉底部
.sessionTimelineLayer 是 .list 内部的 position: sticky 元素,而 .list 就是聊天滚动容器。百分比形式的 top 是相对该滚动容器解析的,所以旧的 top: 50% 恰好把时间轴放在聊天区域的正中心。而 50vh 是相对浏览器视口解析的,视口比聊天区域高出一个输入框的高度——于是时间轴被往下推了大约半个输入框。
输入框并不矮:ChatEditor.module.css 里静止高度是 height: 140px,且 --chat-editor-input-max-height: 300px,因此 150–360px 的 footer 高度属于正常范围。
时间轴中心相对聊天区域中心的偏移(px,+ 表示偏低;有头 Chromium,80 轮,宽 1440px):
| 视口 × 输入框 | main(50%) |
本 PR(50vh) |
本 PR + top: 50% |
|---|---|---|---|
| 900 × 132 | 0 | +78 | 0 |
| 900 × 200 | 0 | +112 | 0 |
| 900 × 260 | 0 | +142 | 0 |
| 900 × 360 | 0 | +192 | 0 |
| 800 × 180 | 0 | +102 | 0 |
| 700 × 132 | 0 | +78 | 0 |
| 700 × 260 | 0 | +142 | 0 |
时间轴底部被聊天滚动容器裁掉的像素数 / 当前轮次的 marker 是否可点击:
| 视口 × 输入框 | main |
本 PR | 本 PR + top: 50% |
|---|---|---|---|
| 900 × 132 | 210 / 否 | 14 / 是 | 0 / 是 |
| 900 × 200 | 244 / 否 | 82 / 否 | 0 / 是 |
| 900 × 260 | 274 / 否 | 142 / 否 | 0 / 是 |
| 900 × 360 | 324 / 否 | 242 / 否 | 50 / 否 |
| 800 × 180 | 284 / 否 | 82 / 否 | 0 / 是 |
| 700 × 132 | 310 / 否 | 34 / 否 | 0 / 是 |
| 700 × 260 | 374 / 否 | 162 / 否 | 20 / 是 |
也就是说,当前轮次的 marker——正是新增的自动居中逻辑特意要滚动到可见的那一个——在我测试的 7 种视口/输入框组合里有 6 种变得不可点击。只回退这一个属性、保留本 PR 其余全部改动,就能恢复精确居中,并让最后一个 marker 在 7 种里的 6 种下可用。这也解释了为什么你在 macOS Chrome 上看起来是正常的:高视口 + 静止输入框(即 900 × 132 那一行)恰好是唯一还能正常工作的配置。
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;可选的进一步加固:max-height: min(640px, max(220px, calc(100vh - 220px))) 出于同样的原因也是相对窗口的。恢复 top: 50% 之后,唯一还会被裁剪的场景是 900px 视口 + 完全展开的 360px 输入框(裁掉 50px)。把这个上限改成相对聊天滚动容器而非 100vh 来计算,就能彻底堵上。
🟡 次要:tooltip 会盖在模态对话框之上
--web-shell-popover-z-index 没有任何默认定义,所以 calc(var(--web-shell-popover-z-index, 1000) + 2) = 1002。而 DialogShell 同样是 createPortal(…, document.body),且 .backdrop { z-index: 1000 }。我用像素探针验证过:铺一个不透明的 z-1000 遮罩盖满屏幕后,tooltip 中心位置的像素仍然是 tooltip 自身的背景色。通常没什么影响——打开对话框会移动焦点,onBlur 会把 tooltip 收起——但如果只是鼠标停在某个 marker 上时弹出了对话框(例如收到一个工具审批请求),tooltip 就会浮在对话框上方。
🟡 吹毛求疵:tooltip 容器的 color 是黑色
portal 逃出了 .sessionTimelinePanel { color: var(--muted-foreground) } 的作用域,而 color 并不在 SESSION_TIMELINE_TOOLTIP_THEME_VARS 里。由于每个文字子元素都各自设置了颜色,所以目前不会渲染出黑字——但 .sessionTimelineDetails 的计算值是 color: rgb(0, 0, 0),而背景是 #171717。在该规则里补一条 color: var(--foreground),就能避免以后有人往里加裸文本节点时踩坑。
📋 关于测试覆盖的说明
41 个 jsdom 测试没有一个能观察到 top、max-height、遮罩或滚动条——jsdom 没有布局引擎。新增的 5 个测试把 JavaScript 行为钉得很牢,但这个 PR 的布局部分对 CI 是完全不可见的,50vh 这个问题正是这样漏过去的。后续改动时间轴时值得留意这一点。
|
Thanks for the extra verification. I addressed the two independent tooltip hardening points in
Verification after that change:
For the |
|
Addressed the actionable parts in What changed:
Verification:
I did not add exhaustive tests for every early-return branch in the centering effect. Those branches are defensive no-op paths rather than distinct user-visible behaviors, and covering all of them would add a lot of brittle jsdom scaffolding without changing the product behavior this PR is fixing. |
Re-verification of
|
| Item | Before | Now (38cc27834) |
|---|---|---|
Tooltip container color |
rgb(0, 0, 0) on a #171717 background |
rgb(250, 250, 250) — color: var(--foreground) resolves correctly through the portal |
Tooltip vs modal DialogShell (.backdrop, z-index: 1000) |
painted above the modal | below ✅ |
Tooltip vs .slashPanel / .atPanel (z-index: 1000) |
above | below ✅ |
Tooltip vs .slashDetail (z-index: 1001) |
above | below ✅ |
| Tooltip vs ordinary chat content | above | still above ✅ (checked with a z-index: 998 control: tooltip wins) |
Paint order was measured with an opaque full-screen overlay portaled to document.body at each z-index and sampling the pixel at the tooltip's centre (headed Chromium under Xvfb).
Everything I confirmed last round still holds on the new head: tooltip clipping 0px, portaled to document.body, hides on rail scroll, marker 1 reachable by wheeling the rail with chat scroll delta 0. Gates: 1265 tests pass (79 files), MessageList.dom.test.tsx 41 pass, npm run build --workspace=packages/web-shell passes, 0 page errors.
🔴 Still open: top: 50vh
Re-measured on 38cc27834, 80 turns, 1440px wide, headed Chromium. computed top is 450px at a 900px viewport, 400px at 800px, 350px at 700px — i.e. still 50vh, still resolved against the browser viewport rather than the chat scrollport it is position: sticky inside. The numbers are byte-for-byte the same as my previous run:
| viewport × composer | rail centre offset from chat centremain / this PR / this PR + top: 50% |
rail bottom clipped (px)main / PR / PR + 50% |
current-turn marker clickablemain / PR / PR + 50% |
|---|---|---|---|
| 900 × 132 | 0 / +78 / 0 | 210 / 14 / 0 | no / yes / yes |
| 900 × 200 | 0 / +112 / 0 | 244 / 82 / 0 | no / no / yes |
| 900 × 260 | 0 / +142 / 0 | 274 / 142 / 0 | no / no / yes |
| 900 × 360 | 0 / +192 / 0 | 324 / 242 / 50 | no / no / no |
| 800 × 180 | 0 / +102 / 0 | 284 / 82 / 0 | no / no / yes |
| 700 × 132 | 0 / +78 / 0 | 310 / 34 / 0 | no / no / yes |
| 700 × 260 | 0 / +142 / 0 | 374 / 162 / 20 | no / no / yes |
The current turn's marker — the one the new auto-centring code deliberately scrolls into view — is still unclickable in 6 of the 7 window/composer sizes. Changing only this one property, keeping every other line of the PR, restores exact centring and makes it clickable in 6/7:
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;Was this one intentionally left out, or just missed? If there's a reason 50vh is needed I'd like to hear it — but as measured, 50% is strictly better in every configuration I tried.
🟡 New nit from the z-index change
--web-shell-popover-z-index is still listed in SESSION_TIMELINE_TOOLTIP_THEME_VARS (MessageList.tsx:1692) but the CSS no longer reads it, so it's now dead. It was already a no-op in practice: nothing defines that variable, so getComputedStyle(panel).getPropertyValue('--web-shell-popover-z-index') returns '', and setProperty(name, '') removes the property — I confirmed the rendered tooltip carries no inline value for it. Worth dropping from the array.
Related, and only if you care about embedders: z-index: 999 is now hard-coded, so unlike every other popover in web-shell the tooltip no longer tracks an embedder's --web-shell-popover-z-index override. That's harmless today precisely because DialogShell.backdrop also hard-codes 1000, so I wouldn't hold the PR on it — just flagging the asymmetry.
中文版
对 38cc27834 的复验
用同样的本地构建 + 真实浏览器 A/B 重新验证了更新后的 head。新提交只改了 CSS(MessageList.module.css 中的 2 行)。
两个次要问题都已修复并验证通过。阻塞项——top: 50vh——没有改动,复测结果完全一致。
✅ 已修复并确认
| 项目 | 之前 | 现在(38cc27834) |
|---|---|---|
Tooltip 容器 color |
#171717 背景上的 rgb(0, 0, 0) |
rgb(250, 250, 250)——color: var(--foreground) 能正确穿过 portal 解析 |
Tooltip vs 模态框 DialogShell(.backdrop,z-index: 1000) |
画在模态框之上 | 之下 ✅ |
Tooltip vs .slashPanel / .atPanel(z-index: 1000) |
之上 | 之下 ✅ |
Tooltip vs .slashDetail(z-index: 1001) |
之上 | 之下 ✅ |
| Tooltip vs 普通聊天内容 | 之上 | 仍在之上 ✅(用 z-index: 998 的对照组验证:tooltip 胜出) |
绘制顺序的测量方式:在每个 z-index 上向 document.body portal 一个不透明的全屏遮罩,然后采样 tooltip 中心点的像素(Xvfb 下的有头 Chromium)。
上一轮确认的结论在新 head 上依然成立:tooltip 裁剪量 0px、portal 到 document.body、时间轴滚动时会隐藏、滚动时间轴滚轮可以到达第 1 个 marker 且聊天滚动增量为 0。各项检查:1265 个测试通过(79 个文件),MessageList.dom.test.tsx 41 个通过,npm run build --workspace=packages/web-shell 通过,0 个页面报错。
🔴 仍未解决:top: 50vh
在 38cc27834 上重新测量,80 轮、宽 1440px、有头 Chromium。computed top 在 900px 视口下是 450px,800px 下是 400px,700px 下是 350px——也就是说仍然是 50vh,仍然是相对浏览器视口解析的,而不是它作为 position: sticky 所处的那个聊天滚动容器。数据和我上一轮跑出来的完全一致:
| 视口 × 输入框 | 时间轴中心相对聊天中心的偏移main / 本 PR / 本 PR + top: 50% |
时间轴底部被裁掉的像素main / PR / PR + 50% |
当前轮次 marker 是否可点击main / PR / PR + 50% |
|---|---|---|---|
| 900 × 132 | 0 / +78 / 0 | 210 / 14 / 0 | 否 / 是 / 是 |
| 900 × 200 | 0 / +112 / 0 | 244 / 82 / 0 | 否 / 否 / 是 |
| 900 × 260 | 0 / +142 / 0 | 274 / 142 / 0 | 否 / 否 / 是 |
| 900 × 360 | 0 / +192 / 0 | 324 / 242 / 50 | 否 / 否 / 否 |
| 800 × 180 | 0 / +102 / 0 | 284 / 82 / 0 | 否 / 否 / 是 |
| 700 × 132 | 0 / +78 / 0 | 310 / 34 / 0 | 否 / 否 / 是 |
| 700 × 260 | 0 / +142 / 0 | 374 / 162 / 20 | 否 / 否 / 是 |
当前轮次的 marker——正是新增的自动居中逻辑特意要滚动到可见的那一个——在 7 种视口/输入框组合里仍有 6 种不可点击。只改这一个属性、保留本 PR 其余全部改动,就能恢复精确居中,并让它在 7 种里的 6 种下可点击:
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;这一处是有意保留的,还是漏掉了?如果确实有必须用 50vh 的理由,我很想听听——但就实测数据而言,在我试过的每一种配置下 50% 都严格更优。
🟡 z-index 改动带来的一个新的小问题
--web-shell-popover-z-index 仍然留在 SESSION_TIMELINE_TOOLTIP_THEME_VARS 里(MessageList.tsx:1692),但 CSS 已经不再读取它,所以它现在是死代码。其实它本来就是个空操作:没有任何地方定义这个变量,因此 getComputedStyle(panel).getPropertyValue('--web-shell-popover-z-index') 返回 '',而 setProperty(name, '') 会把该属性移除——我确认了渲染出来的 tooltip 上并没有携带它的内联值。建议从数组里去掉。
相关的一点,仅在你关心嵌入方(embedder)时才有意义:z-index: 999 现在是硬编码的,因此与 web-shell 里其他所有 popover 不同,tooltip 不再跟随嵌入方对 --web-shell-popover-z-index 的覆盖。这在今天是无害的,恰恰因为 DialogShell.backdrop 同样硬编码了 1000,所以我不会因此卡住这个 PR——只是把这个不对称点提出来。
|
Thanks for re-checking. I removed the now-unused Verification:
On |
Re-verification of
|
| Build | Rail recentred | Spurious hides (tooltip vanished while its anchor was still visible) |
|---|---|---|
shipped — setTimeout(0) |
40/40 | 27 / 40 |
shipped — setTimeout(0) (rerun) |
40/40 | 32 / 40 |
shipped — setTimeout(0) (A/B/A, restored) |
40/40 | 12 / 40 |
mutated back to requestAnimationFrame |
40/40 | 0 / 40 |
mutated back to requestAnimationFrame (rerun) |
40/40 | 0 / 40 |
mutated back to requestAnimationFrame (rerun) |
40/40 | 0 / 40 |
0/120 with rAF, 71/120 with setTimeout(0). The spread across the three shipped runs (27/32/12) is exactly what you'd expect from a race.
Concretely: a keyboard user tabs to a timeline marker to read its tooltip, and the tooltip flickers out on every recentring while the assistant streams (follow-mode keeps changing currentIndex). aria-describedby drops with it. Pointer hover is affected too whenever the rail shifts by less than half a marker pitch, so the pointer stays on the same marker and no mouseleave masks it.
The PR's own test is literally named keeps timeline details during current-turn centering …, so this is a regression against stated intent, not a judgement call.
Suggested fix — revert that hunk. If the motivation was that rAF never fires in a background tab (leaving the flag stuck true), a belt-and-braces version keeps the correct ordering:
const frame = requestAnimationFrame(() => {
programmaticScrollRef.current = false;
syncTooltip();
});
const fallback = window.setTimeout(() => {
programmaticScrollRef.current = false;
}, 100);
return () => {
cancelAnimationFrame(frame);
window.clearTimeout(fallback);
programmaticScrollRef.current = false;
};⚠️ Why CI didn't catch it
keeps timeline details during current-turn centering but hides them on user scroll passes with both primitives — I ran the file four times with rAF restored: 42/42 every time. jsdom never dispatches a scroll event in response to a scrollTop write, so the test hand-dispatches one and can only ever assert "a scroll after the flag reset hides it". The ordering under test is the one thing it cannot observe. The nextFrame() → nextTask() edit in the same commit was therefore not required by a real failure; it just re-tuned the test's wait to the new primitive.
If you want this covered, the assertion has to be that the scroll event produced by the component's own scrollTop write does not hide the tooltip — which needs a real browser, not jsdom.
🔴 Still open: top: 50vh
MessageList.module.css is byte-for-byte unchanged across both new commits (git diff 38cc27834..14f2e75c6 -- …module.css → 0 lines). Re-measured on 14f2e75c6 anyway, headed Chromium, 80 turns, 1440px wide — identical to both previous rounds:
| viewport × composer | computed top |
rail centre offset from chat centre | rail bottom clipped | current-turn marker clickable | usable markers |
|---|---|---|---|---|---|
| 900 × 132 | 450px |
+78 | 14 | yes | 41/80 |
| 900 × 200 | 450px |
+112 | 82 | no | 37/80 |
| 900 × 260 | 450px |
+142 | 142 | no | 33/80 |
| 800 × 180 | 400px |
+102 | 82 | no | 33/80 |
| 700 × 132 | 350px |
+78 | 34 | no | 29/80 |
| 700 × 260 | 350px |
+142 | 162 | no | 21/80 |
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;This is the third round I've raised it and it hasn't been touched or answered. If there's a reason 50vh is needed, please say so and I'll drop it — but as measured, 50% is strictly better in every configuration, and it's a one-property change.
中文版
对 14f2e75c6 的复验
自上一轮以来新增两个提交:20ce1e193(harden timeline tooltip recentering)和 14f2e75c6(drop unused timeline tooltip var)。我重新跑了本地构建和真实浏览器 A/B。
结论:死变量那个小问题已修复。top: 50vh 仍未改动。而且 20ce1e193 引入了一个新的回归——把 requestAnimationFrame 换成 setTimeout(…, 0),恰好破坏了 programmaticScrollRef 这个守卫本来要保护的行为。
各项检查:1266 个测试通过(79 个文件,新增的 clamp 测试 +1),MessageList.dom.test.tsx 42 个通过,构建通过,eslint 0 错误,0 个页面报错。
✅ 已修复
--web-shell-popover-z-index 已从 SESSION_TIMELINE_TOOLTIP_THEME_VARS 中移除,已确认。
新增的 clamps timeline details to the viewport edge 测试是有效的——把 clamping 的 useLayoutEffect 禁用后,恰好只有这一个测试失败。
🔴 新回归:20ce1e193 中把 requestAnimationFrame 换成了 setTimeout(…, 0)
- const frame = requestAnimationFrame(() => {
+ const timer = window.setTimeout(() => {
programmaticScrollRef.current = false;
syncTooltip();
- });
+ }, 0);这个守卫的全部意义在于:当我们自己滚动时间轴去把当前轮次居中时,不要把由此产生的 scroll 事件当作用户滚动而隐藏 tooltip。 而这只有在浏览器派发那个 scroll 事件时标志位仍为 true 才成立。
由命令式 scrollTop 赋值触发的 scroll 事件,是在 "update the rendering" 的 scroll steps 阶段派发的——它在定时器任务之后、在 requestAnimationFrame 回调之前。所以 rAF 永远安全,而 setTimeout(0) 是一场竞态。我在 Chromium 里直接测了(对 el.scrollTop = N 同时挂上三种回调,跑 5 次):
scroll -> rAF -> setTimeout0 (2/5) 两种写法下守卫都成立
setTimeout0 -> scroll -> rAF (3/5) scroll 到达时守卫已被清除 <-- bug
scroll 在 5/5 次中都早于 rAF。setTimeout(0) 在 3/5 次中早于 scroll。而当这次赋值是从渲染阶段的回调内部发起时,坏顺序是确定性发生的。
在真实组件上的端到端验证。 80 轮会话,用键盘焦点(focusin,完全不涉及鼠标)把 tooltip 固定在第 36 个 marker 上,然后在正文上滚 40 次滚轮。每一次都真实触发了时间轴重新居中,并且被聚焦的 marker 全程都留在时间轴 viewport 内——所以每一次隐藏都是虚假的:
| 构建 | 时间轴重新居中 | 虚假隐藏(锚点仍可见时 tooltip 却消失) |
|---|---|---|
已提交 — setTimeout(0) |
40/40 | 27 / 40 |
已提交 — setTimeout(0)(重跑) |
40/40 | 32 / 40 |
已提交 — setTimeout(0)(A/B/A 复原后) |
40/40 | 12 / 40 |
改回 requestAnimationFrame |
40/40 | 0 / 40 |
改回 requestAnimationFrame(重跑) |
40/40 | 0 / 40 |
改回 requestAnimationFrame(重跑) |
40/40 | 0 / 40 |
rAF 下 0/120,setTimeout(0) 下 71/120。已提交版本三次跑出 27/32/12 的离散度,正是竞态该有的样子。
具体场景:键盘用户 Tab 到某个时间轴 marker 去读它的 tooltip,而模型正在流式输出(follow 模式不断改变 currentIndex),于是每一次重新居中都会让 tooltip 闪掉,aria-describedby 也随之消失。鼠标 hover 同样会受影响——只要时间轴位移小于半个 marker 间距,指针仍停在同一个 marker 上,就不会有 mouseleave 来掩盖这个问题。
这个 PR 自己的测试就叫 keeps timeline details during current-turn centering …,所以这是对既定意图的回归,不是见仁见智的问题。
建议的修复——回退这一段。如果当初的动机是"后台标签页里 rAF 不会触发,导致标志位永远卡在 true",那么下面这个双保险写法能在保持正确顺序的前提下解决它:
const frame = requestAnimationFrame(() => {
programmaticScrollRef.current = false;
syncTooltip();
});
const fallback = window.setTimeout(() => {
programmaticScrollRef.current = false;
}, 100);
return () => {
cancelAnimationFrame(frame);
window.clearTimeout(fallback);
programmaticScrollRef.current = false;
};⚠️ 为什么 CI 没抓到
keeps timeline details during current-turn centering but hides them on user scroll 这个测试在两种写法下都通过——我把 rAF 改回去后连跑了 4 次该文件:每次都是 42/42。jsdom 根本不会因为 scrollTop 赋值而派发 scroll 事件,所以测试只能手工派发一个,也就只能断言"标志位重置之后的 scroll 会隐藏 tooltip"。而被测的那个顺序,恰恰是它唯一观察不到的东西。因此同一提交里 nextFrame() → nextTask() 的改动并不是因为真的失败了,只是把测试的等待时机重新对齐到新的原语上。
如果想覆盖这一点,断言必须是"由组件自己那次 scrollTop 赋值所产生的 scroll 事件不会隐藏 tooltip"——这需要真实浏览器,jsdom 做不到。
🔴 仍未解决:top: 50vh
MessageList.module.css 在这两个新提交中逐字节未变(git diff 38cc27834..14f2e75c6 -- …module.css → 0 行)。我还是在 14f2e75c6 上重新测了一遍,有头 Chromium,80 轮,宽 1440px——与前两轮完全一致:
| 视口 × 输入框 | computed top |
时间轴中心相对聊天中心的偏移 | 底部被裁掉 | 当前轮次 marker 可点击 | 可用 marker |
|---|---|---|---|---|---|
| 900 × 132 | 450px |
+78 | 14 | 是 | 41/80 |
| 900 × 200 | 450px |
+112 | 82 | 否 | 37/80 |
| 900 × 260 | 450px |
+142 | 142 | 否 | 33/80 |
| 800 × 180 | 400px |
+102 | 82 | 否 | 33/80 |
| 700 × 132 | 350px |
+78 | 34 | 否 | 29/80 |
| 700 × 260 | 350px |
+142 | 162 | 否 | 21/80 |
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;这已经是我第三轮提出这一点,既没有改动也没有回复。如果确实有必须用 50vh 的理由,请说明,我就不再坚持——但就实测数据而言,在每一种配置下 50% 都严格更优,而且这只是一个属性的改动。
|
Thanks for re-running this. The Fixed in
Verification on the committed tree:
On |
Re-verification of
|
| Build | Rail recentred | Spurious hides |
|---|---|---|
14f2e75c6 — setTimeout(0) |
40/40 | 27 / 32 / 12 out of 40 (three runs) |
3a9e75b21 — rAF + 100 ms fallback |
40/40 | 0 / 0 / 0 (three runs) |
I also checked the two things a guard like this can get wrong:
It doesn't get stuck. After a programmatic recentring the tooltip survives, and a subsequent genuine user scroll of the rail still hides it (survivesRecentring: true, userRailScrollStillHides: true). So the flag is cleared, not merely pinned.
The 100 ms fallback doesn't re-open the race. This was my worry about the shape I suggested: if the next frame lands more than 100 ms after the scrollTop write, the fallback would clear the flag before the scroll event arrives. I tested it directly — same schedule as the component (scrollTop = N, rAF, setTimeout(…, 100)), then a synchronous main-thread burn of 0 / 50 / 90 / 150 / 300 ms in the same task:
blockMs | observed order (5 runs each)
0 | scroll -> rAF-clear -> fallback100 guard holds 5/5
50 | scroll -> rAF-clear -> fallback100 guard holds 5/5
90 | scroll -> rAF-clear -> fallback100 guard holds 5/5
150 | scroll -> rAF-clear -> fallback100 guard holds 5/5
300 | scroll -> rAF-clear -> fallback100 guard holds 5/5
scroll always lands first — 25/25. The reason is that Chromium dispatches the scroll event from an imperative write within 0.4–0.6 ms (I measured 16.6 ms once, at a frame boundary), so it is never anywhere near the 100 ms fallback even under heavy jank. My concern was unfounded; the fallback is pure insurance for the case where rAF never fires at all.
Everything from earlier rounds still holds on this head: tooltip portaled to document.body, position: fixed, z-index: 999, container color: rgb(250,250,250), 0 px clipping, role="tooltip" + aria-describedby, paints below a z-1000 modal backdrop, rail scrollbar hidden (scrollbar-width: none, ::-webkit-scrollbar { display: none }, gutter 0) while remaining scrollable, and marker 1 reachable by wheeling the rail with chat scroll delta 0.
⚠️ The fix is correct but still untested
I re-applied setTimeout(0) on top of this head and ran MessageList.dom.test.tsx three times: 42/42 every time. jsdom never dispatches a scroll event in response to a scrollTop write, so no jsdom test can distinguish the correct ordering from the broken one. The nextTask() → nextFrame() revert restores symmetry with the implementation but adds no protection — a future relapse would land silently. Not a blocker, just worth knowing that this line is load-bearing and unguarded.
🔴 Still open: top: 50vh
No CSS changed in this commit. Re-measured on 3a9e75b21, headed Chromium, 80 turns, 1440 px wide — the same numbers for the fourth round, now shown next to what the one-line change gives:
| viewport × composer | computed top |
offset from chat centre | rail bottom clipped | current marker clickable | usable | → with top: 50% |
|---|---|---|---|---|---|---|
| 900 × 132 | 450px |
+78 | 14 | yes | 41/80 | off 0, clip 0, 41/80 |
| 900 × 200 | 450px |
+112 | 82 | no | 37/80 | off 0, clip 0, 41/80 |
| 900 × 260 | 450px |
+142 | 142 | no | 33/80 | off 0, clip 0, 41/80 |
| 800 × 180 | 400px |
+102 | 82 | no | 33/80 | off 0, clip 0, 37/80 |
| 700 × 132 | 350px |
+78 | 34 | no | 29/80 | off 0, clip 0, 30/80 |
| 700 × 260 | 350px |
+142 | 162 | no | 21/80 | off 0, clip 20, 29/80 |
With top: 50% the current turn's marker is clickable in 6/6 configurations instead of 1/6, the rail sits exactly at the chat area's centre, and up to 8 more markers become reachable.
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;This is the fourth round I've raised it. Everything else in this PR is now verified and good — this one property is all that stands between it and my approval. If you'd prefer, I'm happy to push the one-line change myself; just say the word.
中文版
对 3a9e75b21 的复验
新增一个提交:keep timeline programmatic scroll guard through frame。我重新跑了本地构建和真实浏览器 A/B。
滚动守卫的回归已修复,而且我验证了这个修复是稳健的——包括我自己当初提的方案里担心的那个失效路径。top: 50vh 是唯一剩下的未决项。
各项检查:1266 个测试通过(79 个文件),MessageList.dom.test.tsx 42 个通过,构建通过,eslint 0 错误,0 个页面报错。
✅ 滚动守卫:已修复并经过压力测试
与上一轮相同的驱动方式——80 轮会话,用键盘焦点把 tooltip 固定在某个 marker 上,在正文上滚 40 次滚轮,每一次都真实触发时间轴重新居中且锚点全程可见:
| 构建 | 时间轴重新居中 | 虚假隐藏 |
|---|---|---|
14f2e75c6 — setTimeout(0) |
40/40 | 40 次里 27 / 32 / 12(三次运行) |
3a9e75b21 — rAF + 100 ms 兜底 |
40/40 | 0 / 0 / 0(三次运行) |
我还检查了这类守卫最容易出的两种问题:
它不会卡死。 程序化重新居中之后 tooltip 依然存在,而随后一次真正的用户滚动时间轴仍然会把它隐藏(survivesRecentring: true,userRailScrollStillHides: true)。说明标志位是被清除了,而不是被钉死。
100 ms 的兜底不会重新打开竞态。 这正是我对自己提的那个写法的担忧:如果下一帧比 scrollTop 赋值晚了 100 ms 以上,兜底就会在 scroll 事件到达之前清除标志位。我直接测了——用与组件完全相同的调度(scrollTop = N、rAF、setTimeout(…, 100)),然后在同一个任务里同步烧掉主线程 0 / 50 / 90 / 150 / 300 ms:
blockMs | 实际回调顺序(每档 5 次)
0 | scroll -> rAF-clear -> fallback100 守卫成立 5/5
50 | scroll -> rAF-clear -> fallback100 守卫成立 5/5
90 | scroll -> rAF-clear -> fallback100 守卫成立 5/5
150 | scroll -> rAF-clear -> fallback100 守卫成立 5/5
300 | scroll -> rAF-clear -> fallback100 守卫成立 5/5
scroll 总是最先到达——25/25。原因是 Chromium 在命令式赋值后 0.4–0.6 ms 内就派发了 scroll 事件(我只在一次帧边界上测到 16.6 ms),因此即使在严重卡顿下也远远够不到 100 ms 的兜底。我的担忧不成立;这个兜底纯粹是给"rAF 完全不触发"那种情况买的保险。
前几轮确认的结论在这个 head 上依然成立:tooltip portal 到 document.body、position: fixed、z-index: 999、容器 color: rgb(250,250,250)、裁剪 0 px、role="tooltip" + aria-describedby、绘制在 z-1000 模态遮罩之下、时间轴滚动条隐藏(scrollbar-width: none、::-webkit-scrollbar { display: none }、gutter 0)但仍可滚动,以及滚动时间轴滚轮可到达第 1 个 marker 且聊天滚动增量为 0。
⚠️ 修复是对的,但仍然没有测试保护
我在这个 head 上重新把 setTimeout(0) 打回去,跑了三次 MessageList.dom.test.tsx:每次都是 42/42。jsdom 根本不会因为 scrollTop 赋值而派发 scroll 事件,所以任何 jsdom 测试都无法区分正确顺序和错误顺序。nextTask() → nextFrame() 的回退让测试与实现重新对齐,但并没有带来任何保护——将来若再退回去,不会有任何报警。这不是阻塞项,只是想说明这一行是承重的、且无人看守。
🔴 仍未解决:top: 50vh
这个提交没有改任何 CSS。我还是在 3a9e75b21 上重新测了一遍,有头 Chromium,80 轮,宽 1440 px——第四轮,数据完全一致,这次把那一行改动的效果并排放出来:
| 视口 × 输入框 | computed top |
相对聊天中心的偏移 | 底部被裁掉 | 当前 marker 可点击 | 可用 | → 改成 top: 50% |
|---|---|---|---|---|---|---|
| 900 × 132 | 450px |
+78 | 14 | 是 | 41/80 | 偏移 0,裁剪 0,41/80 |
| 900 × 200 | 450px |
+112 | 82 | 否 | 37/80 | 偏移 0,裁剪 0,41/80 |
| 900 × 260 | 450px |
+142 | 142 | 否 | 33/80 | 偏移 0,裁剪 0,41/80 |
| 800 × 180 | 400px |
+102 | 82 | 否 | 33/80 | 偏移 0,裁剪 0,37/80 |
| 700 × 132 | 350px |
+78 | 34 | 否 | 29/80 | 偏移 0,裁剪 0,30/80 |
| 700 × 260 | 350px |
+142 | 162 | 否 | 21/80 | 偏移 0,裁剪 20,29/80 |
改成 top: 50% 后,当前轮次的 marker 在 6/6 种配置下都可点击(而不是 1/6),时间轴恰好落在聊天区域的正中心,并且最多多出 8 个可到达的 marker。
.sessionTimelineLayer {
position: sticky;
- top: 50vh;
+ top: 50%;这已经是我第四轮提出这一点了。这个 PR 的其余部分现在都已验证通过、状态良好——只剩这一个属性挡在合并之前。如果你愿意,我也可以直接帮你推这一行改动,说一声即可。
Local real-browser validation for
|
| Composer height | Current top: 50vh, center vs viewport |
Current top: 50vh, center vs chat body |
Initial current marker | Forced top: 50%, center vs viewport |
Forced top: 50%, center vs chat body |
|---|---|---|---|---|---|
| 132px | +12px | +78px | hit-test clickable | -66px | 0px |
| 200px | +12px | +112px | initially covered by composer | -100px | 0px |
| 260px | +12px | +142px | initially covered by composer | -130px | 0px |
So 50% does exactly what the reviewer says: it centers the rail in the chat body. The reason I am keeping 50vh is that it keeps the rail visually centered in the page/viewport instead of shifting it upward as the composer grows. Based on this validation, my merge recommendation is to keep top: 50vh; the remaining point is an intentional visual preference and interaction tradeoff, not an unaddressed implementation defect.
中文版
3a9e75b21 本地真实浏览器验证
我为这个 PR head 构造并运行了本地浏览器级验证 harness。它在 Vite 页面里挂载真实的 web-shell MessageList 时间轴,使用真实应用 CSS 链路、真实深色/浅色主题变量、80 轮会话记录,以及固定在底部的输入区。视觉验证运行在 Playwright Chromium 的真实布局/CSS 环境里,不依赖 jsdom;唯一的 stub 是 compact-mode provider,用来隔离组件,避免拉起 daemon/runtime 服务。
验证结果
- 长会话时间轴:深色和浅色主题下都渲染了 80 条 timeline entry。
- rail 内部滚动:rail 可以从
scrollTop=612滚到0再滚回底部,并且不会带动正文 transcript 滚动;在普通输入区高度下,第一条和最后一条 marker 都能通过滚动 rail 到达并命中点击。 - 隐藏滚动条:Chromium computed style 为
scrollbar-width: none,::-webkit-scrollbar { display: none },实测 scrollbar gutter 是0px,同时 rail 仍然可滚动。 - Tooltip 浮层:详情 tooltip 渲染在
document.body下,具备role="tooltip",z-index: 999,深色/浅色主题文字颜色正确,没有被 viewport 裁剪,并且用户滚动 rail 后会消失。 - 程序化重新居中守卫:组件自己为了当前轮次重新居中 rail 时,tooltip 不会被误隐藏,说明 rAF guard 仍然能压住程序化 scroll 事件。
- 本地命令也通过:
npm run test --workspace=packages/web-shell -- client/components/MessageList.dom.test.tsx显示 42/42 通过,npm run build --workspace=packages/web-shell成功完成。
关于 top: 50vh 与 top: 50%
我认同 reviewer 的测量:如果目标是“相对正文滚动区域居中”,并且主要指标是高输入框状态下当前 marker 初始可点,那么 top: 50% 更优。但这不是这个 PR 想要的视觉目标。
这里的目标是基于页面/视口的视觉居中。输入区本身也是用户正在看到的页面 chrome;如果只按正文区域居中,时间轴在用户视觉上会显得过于靠上。时间轴本身被设计成独立可滚动的控件,所以用户可以滚动 rail 自身来选择初始可视范围之外的前后 marker。这符合 Codex 这里的交互设计,我不认为高输入框下的重叠取舍是需要阻塞合并的正确性问题。
本地 Chromium harness 在 1440 x 900 下的测量:
| 输入区高度 | 当前 top: 50vh 相对 viewport 中心 |
当前 top: 50vh 相对正文中心 |
初始当前 marker | 强制 top: 50% 相对 viewport 中心 |
强制 top: 50% 相对正文中心 |
|---|---|---|---|---|---|
| 132px | +12px | +78px | 可命中点击 | -66px | 0px |
| 200px | +12px | +112px | 初始被输入区覆盖 | -100px | 0px |
| 260px | +12px | +142px | 初始被输入区覆盖 | -130px | 0px |
所以 50% 的确实现了 reviewer 说的效果:时间轴相对正文区域居中。我保留 50vh 的原因是,它让时间轴在页面/视口中保持视觉居中,而不是随着输入区变高而在视觉上向上偏移。基于这轮验证,我建议合并时保留 top: 50vh;剩下这个点是有意的视觉偏好和交互取舍,不是尚未修复的实现缺陷。
Resolved all review threads. The remaining rAF guard comment is a false positive against the current Chromium behavior and was answered with fresh Playwright real-browser evidence; no code change is needed.
ytahdn
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. LGTM! ✅ Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
Re-verification of
|
| Check | Result |
|---|---|
MessageList.dom.test.tsx |
42 / 42 pass |
| Revert-proof (PR's test file + merge-base source) | 7 fail / 35 pass — the new tests are load-bearing |
eslint --max-warnings 0 on the 3 changed files |
0 errors |
prettier --check on the 3 changed files |
clean |
tsc -p packages/web-shell/tsconfig.json --noEmit |
57 errors at this head, the identical 57 at merge-base → 0 new (all pre-existing, all in test files; web-shell has no typecheck script so CI never runs this) |
| First / last marker reachable | 120-turn session, 1400×900, no composer. merge-base: marker 0 at y = −450.5, marker 119 at y = +1350.5 — elementFromPoint → null, unclickable. This head: both hit-testable after scrolling the rail; clicking drives the transcript 20726 → 0 and 0 → 20726. |
| Rail scroll doesn't move the transcript | 2000px over-wheel on the rail: rail 1212 → 0, transcript 20726 → 20726 (overscroll-behavior: contain) |
| Tooltip clipping | Intersecting the tooltip rect against every ancestor with overflow != visible plus the window: merge-base 69.4% visible, this head 100% |
| Tooltip DOM | 120 → 1 node; position: fixed, portalled to <body>, z-index: 999, role="tooltip", aria-describedby wired |
| Theme survives the portal | dark color: rgb(250,250,250), light color: rgb(10,10,11); background/border resolve in both |
| Scheduled-task entries | clock icon + data-scheduled-task="true" preserved inside the portal |
🔴 New: keyboard focus destroys the tooltip on any marker that requires a rail scroll
handleViewportScroll dismisses the tooltip on any scroll not flagged as programmatic. But when you Tab onto a marker that sits outside the rail viewport, the browser scrolls it into view itself. That scroll is not covered by programmaticScrollRef, so onFocus shows the tooltip and the scroll event that the focus caused immediately hides it again.
Tab-walk across the rail, real Chromium, 120-turn session, 46 Tab presses:
| Did focusing the marker require a rail scroll? | markers | tooltip shown |
|---|---|---|
| no | 39 | 39 (100%) |
| yes | 7 | 0 (0%) |
aria-describedby drops to null on exactly those 7, so a screen reader loses the description precisely for the markers that weren't already visible. It bites the moment you Tab past the visible band — i.e. on exactly the long sessions this PR is about.
This is a regression against merge-base: there the details were plain CSS (:focus-visible .sessionTimelineDetails { opacity: 1 }), so focusing any reachable marker always revealed them (clipped, but present).
Why the suite can't see it. focusIn() in MessageList.dom.test.tsx only dispatches el.dispatchEvent(new FocusEvent('focusin')) — it never actually focuses the element, so document.activeElement is never the button — and jsdom has no focus scroll-into-view at all. Worse, the test hides timeline details when the user scrolls the timeline viewport does focusIn(button) and then dispatches a scroll, asserting the tooltip is gone. That is exactly the broken sequence, pinned as expected behaviour.
Fix I verified — restores 7 / 7, and all 42 existing tests still pass unchanged:
const handleViewportScroll = useCallback(() => {
if (programmaticScrollRef.current) return;
// A focus-driven scroll-into-view (Tab onto an off-screen marker) must
// re-anchor the tooltip, not dismiss it.
if (viewportRef.current?.querySelector('button:focus')) {
syncTooltip();
return;
}
hideTooltip();
}, [hideTooltip, syncTooltip]);(handleViewportScroll has to move below syncTooltip's declaration.) syncTooltip already drops the tooltip when the anchor leaves the rail viewport, so this doesn't keep stale tooltips alive.
Worth naming the trade-off: this also means that if a rail button has focus and the user then wheel-scrolls the rail, the tooltip re-anchors instead of hiding. If you'd rather keep "any user scroll hides it", the equivalent fix is to raise programmaticScrollRef inside revealTooltip for one frame, so only the focus-induced scroll is swallowed. Either shape works; I verified the one above.
top: 50vh — measuring the painted pixels rather than the box
You've declined this four times and I don't want to re-post the same table, so here is a different measurement that I think is the one that matters.
Your counter-table reports the panel box at a constant +12px from the viewport centre. That's correct. But .list is overflow-y: auto, and the rail lives inside it — so a growing share of that box is never painted. What the user sees is panel ∩ .list. Measuring that, at 1440×900, 80 turns:
| composer | painted rail | never rendered | painted centre | Δ page centre (450) | Δ chat centre | current-turn marker |
|---|---|---|---|---|---|---|
| 132px | 143…768 | 15px | 455 | +5 | +71 | clickable |
| 200px | 143…700 | 83px | 421 | −28 | +71 | clipped |
| 260px | 143…640 | 143px | 391 | −58 | +71 | clipped |
| 360px | 143…540 | 243px | 341 | −108 | +71 | clipped |
So the painted rail is off the page centre by −108 … +5px, and the error grows with the composer. It is not page-centred — and it isn't chat-centred either (a constant +71px). ChatEditor.module.css sets height: 140px at rest with --chat-editor-input-max-height: 300px, so the 200–360px rows are ordinary states, not corner cases.
I also measured the third placement, because it's the one that would actually deliver your stated goal. position: fixed; top: 50vh does escape .list's clip — painted 130…770, 0px lost, painted centre 450 = the page centre exactly, and 41/80 markers usable at every composer height. But then the rail paints on top of the composer: 7 markers at a 260px composer, 14 at 360px (verified by sampling elementFromPoint inside the composer's band, and by checking that rail pixels exist below its top edge — there are none in the 50vh and 50% arms).
Which leaves three real options, not two:
| placement | fully painted? | centred on? | overlaps composer? | usable markers (132 / 200 / 260px composer) |
|---|---|---|---|---|
sticky; top: 50% |
yes | chat area, exactly | no | 41 / 41 / 41 |
sticky; top: 50vh (this PR) |
no — up to 243px clipped | neither | no | 41 / 37 / 33 |
fixed; top: 50vh |
yes | page, exactly | yes | 41 / 41 / 41 |
A 640px rail that lives inside .list cannot be page-centred without either clipping it or drawing it over the composer. 50vh picks clipping, which is why the rail the user sees ends up centred on neither. If page-centring is genuinely the design you want, the change isn't the offset — it's getting the rail out of the scrollport (the tooltip already does exactly this, via createPortal + position: fixed).
Your call as author; I'm not going to keep re-litigating it. I just wanted the decision made against the painted geometry rather than the box geometry.
Related and independent: max-height: min(640px, max(220px, calc(100vh - 220px))) is also window-relative, so it assumes total chrome ≤ 220px.
One behaviour change worth stating explicitly
Capping the rail at 640px means ~41 markers are reachable without scrolling it. On merge-base the rail was 15N − 12px tall, so any session whose rail still fit the chat area (≈47 entries in a 702px-tall chat area, ≈60 in a full-height one) showed all of its markers at once. That's an intentional consequence of making the rail scrollable, and it's the right trade — but it isn't purely additive, and it isn't mentioned in the PR description.
Verdict
The core of this PR is correct and I've now confirmed it three different ways. The keyboard-focus item above is new, uncontested, and has a one-block fix that keeps every existing test green — I'd take that before merge. top: 50vh I'll leave with you, with the numbers.
中文版
3a9e75b21 复验 —— 一个新问题,以及关于 top: 50vh 的新证据
这一轮我没有复用之前的驱动脚本,而是从零重建了 harness 并独立重新测量了每一个数字:在无头 Chromium(Playwright)里挂载真实的 MessageList,配上真实的 provider 和真实的应用 CSS 链路(.app.themeDark > .content > .list,并把输入区做成 .content 的 flex 兄弟节点——因为 .chatSubtree 是 display: contents)。A/B 的做法是在 merge-base 8296ce9e5 与当前 head 之间覆盖那两个源文件。
第 1–4 轮的结论在 3a9e75b21 上依然成立。我发现了一个此前几轮都没有覆盖到的新问题:当 rail 需要滚动才能露出某个 marker 时,键盘 focus 会导致该 marker 的 tooltip 被销毁。另外,关于 top: 50vh,我有一个新的测量——不是重复同一张表,而是去测量"真正被绘制出来的像素"而非"盒子"。
✅ 在当前 head 上复验通过
| 检查项 | 结果 |
|---|---|
MessageList.dom.test.tsx |
42 / 42 通过 |
| 反证(保留 PR 的测试文件 + 回退源文件到 merge-base) | 7 失败 / 35 通过 —— 新测试确实有效 |
对 3 个改动文件跑 eslint --max-warnings 0 |
0 错误 |
对 3 个改动文件跑 prettier --check |
通过 |
tsc -p packages/web-shell/tsconfig.json --noEmit |
当前 head 57 个错误,merge-base 上是完全相同的 57 个 → 零新增(全部是既有问题,且都在测试文件里;web-shell 没有 typecheck script,所以 CI 从不跑它) |
| 首 / 末 marker 可达 | 120 轮会话,1400×900,无输入区。merge-base:marker 0 在 y = −450.5,marker 119 在 y = +1350.5,elementFromPoint 返回 null,无法点击。当前 head:滚动 rail 后两者都可命中;点击分别把正文从 20726 → 0、0 → 20726。 |
| 滚动 rail 不会带动正文 | 在 rail 上过量滚动 2000px:rail 1212 → 0,正文 20726 → 20726(overscroll-behavior: contain 生效) |
| Tooltip 裁剪 | 把 tooltip 矩形与所有 overflow != visible 的祖先以及窗口求交:merge-base 可见 69.4%,当前 head 100% |
| Tooltip DOM | 节点数 120 → 1;position: fixed,portal 到 <body>,z-index: 999,role="tooltip",aria-describedby 正确挂载 |
| 主题穿过 portal | 深色 color: rgb(250,250,250),浅色 color: rgb(10,10,11);背景与边框在两种主题下都能解析 |
| 定时任务条目 | 时钟图标与 data-scheduled-task="true" 在 portal 里都保留 |
🔴 新问题:需要滚动 rail 才能露出的 marker,键盘 focus 时 tooltip 会被销毁
handleViewportScroll 会在任何"非程序化"的滚动时关掉 tooltip。但当你用 Tab 聚焦到一个位于 rail viewport 之外的 marker 时,浏览器会自己把它滚动进视野。这个滚动不在 programmaticScrollRef 的保护范围内,于是 onFocus 刚把 tooltip 显示出来,紧接着由 focus 引发的 scroll 事件又立刻把它隐藏了。
在 rail 上做 Tab 遍历(真实 Chromium,120 轮会话,按了 46 次 Tab):
| 聚焦该 marker 是否需要滚动 rail? | marker 数 | 显示了 tooltip |
|---|---|---|
| 否 | 39 | 39(100%) |
| 是 | 7 | 0(0%) |
这 7 个 marker 上 aria-describedby 同时变成 null,所以恰恰是那些原本就不可见的 marker,屏幕阅读器也读不到描述了。只要你 Tab 越过当前可见区间就会触发——也就是本 PR 所针对的长会话场景。
相对 merge-base 这是一处回归:在 merge-base 上详情是纯 CSS(:focus-visible .sessionTimelineDetails { opacity: 1 }),所以聚焦任何可达的 marker 都一定会显示详情(虽然会被裁剪,但确实显示)。
为什么测试抓不到。 MessageList.dom.test.tsx 里的 focusIn() 只是 el.dispatchEvent(new FocusEvent('focusin')),它从未真正聚焦元素,所以 document.activeElement 永远不是那个 button;而 jsdom 根本没有 focus 触发的 scroll-into-view。更麻烦的是,hides timeline details when the user scrolls the timeline viewport 这个测试正是先 focusIn(button) 再派发 scroll,然后断言 tooltip 消失——它把出问题的那条路径当成了预期行为固化下来。
我验证过的修复 —— 恢复到 7 / 7,且现有 42 个测试全部照常通过:
const handleViewportScroll = useCallback(() => {
if (programmaticScrollRef.current) return;
// 由 focus 引发的 scroll-into-view(Tab 到视野外的 marker)应当重新锚定
// tooltip,而不是把它关掉。
if (viewportRef.current?.querySelector('button:focus')) {
syncTooltip();
return;
}
hideTooltip();
}, [hideTooltip, syncTooltip]);(handleViewportScroll 需要移到 syncTooltip 声明之后。)syncTooltip 本身已经会在锚点离开 rail viewport 时丢弃 tooltip,所以这不会留下过期的 tooltip。
需要说明这里的取舍:这样改之后,如果某个 rail 按钮处于 focus 状态、用户再用滚轮滚动 rail,tooltip 会重新锚定而不是隐藏。如果你更希望保持"任何用户滚动都隐藏",等价的做法是在 revealTooltip 里把 programmaticScrollRef 置位一帧,只吞掉由 focus 引发的那次滚动。两种形态都可行;我验证的是上面这一种。
top: 50vh —— 测量"被绘制的像素"而不是"盒子"
这一点你已经拒绝了四次,我不想再贴同一张表,所以这里换一个我认为更关键的测量方式。
你的对照表给出的是面板盒子相对视口中心恒定 +12px。这没错。但 .list 是 overflow-y: auto,而 rail 就活在它里面——所以这个盒子里有越来越大的一部分根本不会被绘制。用户看到的是 panel ∩ .list。按这个口径测量(1440×900,80 轮):
| 输入区高度 | 实际绘制范围 | 未绘制 | 绘制部分的中心 | Δ 页面中心 (450) | Δ 正文中心 | 当前轮 marker |
|---|---|---|---|---|---|---|
| 132px | 143…768 | 15px | 455 | +5 | +71 | 可点击 |
| 200px | 143…700 | 83px | 421 | −28 | +71 | 被裁剪 |
| 260px | 143…640 | 143px | 391 | −58 | +71 | 被裁剪 |
| 360px | 143…540 | 243px | 341 | −108 | +71 | 被裁剪 |
也就是说,实际绘制出来的 rail 相对页面中心偏移 −108 … +5px,而且输入区越高偏差越大。它并没有相对页面居中——同时也没有相对正文居中(恒定 +71px)。ChatEditor.module.css 里静息高度是 height: 140px,--chat-editor-input-max-height: 300px,所以 200–360px 这几行是常规状态,不是边角情况。
我还测了第三种放置方式,因为它才是真正能实现你所说的目标的那个。position: fixed; top: 50vh 确实能逃出 .list 的裁剪——绘制范围 130…770,0px 丢失,绘制中心 450,恰好是页面中心,并且在任何输入区高度下都有 41/80 个 marker 可用。但代价是 rail 会画到输入区上面:260px 输入区时有 7 个 marker、360px 时有 14 个(用 elementFromPoint 在输入区范围内采样验证,并检查输入区上边缘以下是否存在 rail 像素——50vh 和 50% 两种情况下都没有)。
所以真正的选项有三个,而不是两个:
| 放置方式 | 是否完整绘制 | 相对什么居中 | 是否压住输入区 | 可用 marker(输入区 132 / 200 / 260px) |
|---|---|---|---|---|
sticky; top: 50% |
是 | 精确相对正文区域 | 否 | 41 / 41 / 41 |
sticky; top: 50vh(本 PR) |
否——最多裁掉 243px | 都不是 | 否 | 41 / 37 / 33 |
fixed; top: 50vh |
是 | 精确相对页面 | 是 | 41 / 41 / 41 |
一个 640px 高、又活在 .list 内部的 rail,不可能在既不被裁剪、又不压住输入区的前提下相对页面居中。50vh 选择了"被裁剪",这正是它最终既不相对页面居中、也不相对正文居中的原因。如果"相对页面居中"确实是你想要的设计,那要改的不是这个偏移量,而是把 rail 移出滚动容器(tooltip 已经正是这么做的:createPortal + position: fixed)。
这仍然由你作为作者来定;我不会继续在这一点上反复争论,只是希望这个决定是基于"绘制出来的几何"而不是"盒子的几何"来做的。
另有一个独立的相关点:max-height: min(640px, max(220px, calc(100vh - 220px))) 同样是相对窗口计算的,因此它隐含假设上下 chrome 总高不超过 220px。
一处值得写进 PR 描述的行为变化
把 rail 限高到 640px 意味着不滚动它时大约只有 41 个 marker 可达。而在 merge-base 上 rail 高度是 15N − 12 px,因此只要 rail 还能塞进正文区域(702px 高的正文区约 47 条,满高时约 60 条),全部 marker 都会一次性显示。这是让 rail 可滚动之后的有意取舍,方向也是对的——但它并非纯增量,PR 描述里也没有提到。
结论
这个 PR 的核心是正确的,我已经用三种不同方式确认过了。上面那个键盘 focus 的问题是新的、没有争议的,而且有一个不影响任何现有测试的小修复——建议合并前带上。top: 50vh 我把数据留给你判断。
|
Thanks for the fresh re-verification. I treated the keyboard-focus finding as valid and fixed it in What changed:
Verification on this branch:
On The 640px cap / visible-marker subset is also an intentional result of making long timelines scrollable. The PR description already calls out that long marker lists now scroll internally; I do not think this needs another code change. 中文版感谢这轮重新验证。我确认键盘 focus 的问题有效,并已在 改动内容:
验证结果:
关于 640px 限高导致无需滚动时只能看到一部分 marker,也是长时间轴改成内部滚动后的预期结果。PR 描述已经说明长列表会内部滚动,我认为这里不需要再做代码修改。 |
|
The Fixed in Local verification:
|
|
Merge-conflict follow-up after syncing
Local verification after the conflict resolution:
中文说明这次同步 本地验证结果:MessageList DOM 测试 43 个通过,Prettier 检查通过,Web Shell Playwright smoke 6 个通过,Web Shell build 通过。 |
Re-verification of
|
| Did focusing the marker require a rail scroll? | markers | 3a9e75b21 |
b835e03bd |
|---|---|---|---|
| no | 39 | 39 | 39 |
| yes | 7 | 0 | 7 |
aria-describedby is wired on all 46, and the tooltip lands on its own marker to within ±0px (measured tooltip.style.top against the focused button's centre).
Guard stress — 120 turns, 1400×900, 48px header + 150px composer. The risk with a guard like this is that it gets stuck, or swallows scrolls it shouldn't:
| scenario | expected | observed |
|---|---|---|
| Tab onto an off-screen marker | tooltip kept, correctly anchored | ✅ present, Δanchor 0px |
| user wheels the rail immediately after | hides — guard not stuck | ✅ hidden |
| focus, wait 500ms (past rAF and the 100ms fallback), then wheel | hides | ✅ hidden |
25× rapid Tab burst (key-repeat), then wheel |
hides — no stuck guard | ✅ hidden |
Shift+Tab backwards over the fold |
tooltip kept | ✅ present, Δanchor 0px |
| mouse hover + wheel, no focus anywhere | hides — pointer path unchanged | ✅ hidden |
| chat scroll → programmatic recentring, focus held | 0 spurious hides | ✅ 0 / 25 (rail genuinely moved on 18/25 ticks), 0 mis-anchored |
0 page errors (beyond my harness's own missing favicon).
The new test has teeth. Reverting only the MessageList.tsx hunk of bb0d5ccb9 fails exactly keeps timeline details when focus scrolls the timeline viewport — 1 failed / 42 passed — and nothing else.
Gates at b835e03bd:
| Check | Result |
|---|---|
MessageList.dom.test.tsx |
43 / 43 (via the package's own vitest.config.ts) |
Revert-proof vs the new merge-base 637d00ceb |
8 fail / 35 pass |
eslint --max-warnings 0 |
0 errors |
prettier --check on every file the PR touches |
clean |
tsc -p tsconfig.json --noEmit |
36 errors at head, the same 36 at merge-base → 0 new, 0 in MessageList.* (all in transcriptToMessages.ts etc., unbuilt-sibling noise in my worktree) |
| CI | Test (ubuntu) pass, and the new web-shell E2E Smoke gate passes |
🔴 Still open: top: 50vh
Unchanged, and it re-measures byte-for-byte identical to my last round (1440×900, 80 turns, computed top: 450px in every case):
| composer | painted rail | never rendered | Δ page centre | current-turn marker |
|---|---|---|---|---|
| 132px | 143…768 | 15px | +5 | clickable |
| 200px | 143…700 | 83px | −28 | clipped |
| 260px | 143…640 | 143px | −58 | clipped |
| 360px | 143…540 | 243px | −108 | clipped |
No new argument from me — I made the case last round and it's yours to call.
Three things the main merge brought in
1. b835e03bd drags 5 unrelated files into the diff. docs/users/configuration/model-providers.md, docs/users/features/commands.md, docs/users/features/computer-use.md, docs/users/qwen-serve.md, and packages/cli/src/ui/hooks/slashCommandProcessor.ts. I checked each one: it is byte-identical to prettier --write applied to main's content, so nothing was lost, and CI won't complain (the CI prettier step is --write, not --check). But it takes the PR from 3 files to 8, and the docs churn is pure table-pipe realignment. Worth dropping before merge.
2. Correction — you'd already explained this and I posted before reading it. I wrote this section before your 10:45 / 10:56 comments loaded on my side. For the record, I verified your account rather than take it on trust: at f7480ee07 is now a no-op.3a9e75b21 the branch's packages/web-shell/package.json had no test:e2e:smoke key at all, while a pull_request run executes the merged workflow — i.e. main's ci.yml, which invokes exactly that script. So the smoke job really was failing on a CI-wiring gap that this PR exposed, f7480ee07 was the right temporary fix, and the merge deliberately superseded it with main's Playwright command. Nothing to do here; my "dead commit" framing was wrong.
3. The new web-shell E2E Smoke gate does not cover the timeline. client/e2e/web-shell.smoke.spec.ts contains no session-timeline selector. So the observation from last round stands unchanged: the layout half of this PR — top, max-height, the mask, the hidden scrollbar — is still invisible to CI, and it's exactly the half where the two real defects were. That spec is now the natural home for a guard, since it's a real browser.
🟡 Nit, new since #6572 landed in main
DialogShell.module.css now reads z-index: var(--web-shell-dialog-backdrop-z-index, 1000), while the tooltip hard-codes 999. Today nothing defines that variable, so 999 < 1000 still holds and my previous paint-order check remains valid. But the point of #6572 is to let an embedder override it — and an embedder that lowers the backdrop below 999 gets the timeline tooltip painting on top of the modal. Keeping them coupled would be:
z-index: calc(var(--web-shell-dialog-backdrop-z-index, 1000) - 1);Not a blocker, and I did not render a dialog to prove it — this one is from reading the CSS.
Verdict
The focus fix is correct and robust; I tried hard to make the guard stick or swallow a real scroll and couldn't. Everything else re-confirmed at this head, CI green including the new browser gate. The only open item is top: 50vh.
中文版
b835e03bd 复验 —— focus 修复已确认,并做了压力测试
三个新提交:bb0d5ccb9(focus 滚动守卫)、f7480ee07(smoke 脚本)、b835e03bd(合并 main)。沿用上一轮的 harness,对新 head 重新跑了一遍。
键盘 focus 的回归已修复,新守卫在我能想到的各种破坏方式下都成立。top: 50vh 未改动,重新测量结果逐字节一致。合并 main 带进来三件值得看一眼的事情。
✅ focus 修复有效
你采用的是我给的第二种形态——在 onFocus 里置一帧守卫——而不是我的 button:focus 判断。这个选择更好:guardFocusScroll() 跑在 onFocus 处理函数里,而测试辅助函数 focusIn() 确实能触发它,因此这个行为在 jsdom 里是可测的;而 button:focus 查询在 jsdom 里永远看不到。
在 rail 上做 Tab 遍历(真实 Chromium,120 轮会话,按了 46 次 Tab):
| 聚焦该 marker 是否需要滚动 rail? | marker 数 | 3a9e75b21 |
b835e03bd |
|---|---|---|---|
| 否 | 39 | 39 | 39 |
| 是 | 7 | 0 | 7 |
46 个 marker 上 aria-describedby 全部正确挂载,且 tooltip 落在它自己的 marker 上,误差在 ±0px 以内(用 tooltip.style.top 与聚焦按钮的中心做比较)。
守卫压力测试 —— 120 轮,1400×900,48px header + 150px 输入区。这类守卫的风险在于:卡住不释放,或者吞掉了本不该吞的滚动:
| 场景 | 预期 | 实测 |
|---|---|---|
| Tab 到视野外的 marker | 保留 tooltip 且正确锚定 | ✅ 存在,Δ锚点 0px |
| 紧接着用户滚轮滚 rail | 隐藏 —— 守卫没卡住 | ✅ 隐藏 |
| focus 后等 500ms(超过 rAF 和 100ms 兜底)再滚轮 | 隐藏 | ✅ 隐藏 |
连按 25 次 Tab(模拟长按重复)后滚轮 |
隐藏 —— 无残留守卫 | ✅ 隐藏 |
Shift+Tab 反向越过折叠处 |
保留 tooltip | ✅ 存在,Δ锚点 0px |
| 纯鼠标 hover + 滚轮(无 focus) | 隐藏 —— 鼠标路径不受影响 | ✅ 隐藏 |
| 滚动正文 → 程序化重新居中,focus 保持 | 0 次误隐藏 | ✅ 0 / 25(其中 18/25 次 rail 确实移动了),0 次错位 |
0 个页面错误(除了我 harness 自己缺 favicon 的 404)。
新测试是有效的。 只回退 bb0d5ccb9 中 MessageList.tsx 的那段改动,失败的恰好是 keeps timeline details when focus scrolls the timeline viewport —— 1 失败 / 42 通过 —— 其余测试不受影响。
b835e03bd 上的各项门禁:
| 检查项 | 结果 |
|---|---|
MessageList.dom.test.tsx |
43 / 43(用包自带的 vitest.config.ts 运行) |
相对新 merge-base 637d00ceb 的反证 |
8 失败 / 35 通过 |
eslint --max-warnings 0 |
0 错误 |
对 PR 触及的所有文件跑 prettier --check |
通过 |
tsc -p tsconfig.json --noEmit |
head 36 个错误,merge-base 同样 36 个 → 零新增,MessageList.* 中 0 个(都在 transcriptToMessages.ts 等文件,是我 worktree 里 sibling 包未构建造成的噪音) |
| CI | Test (ubuntu) 通过,新增的 web-shell E2E Smoke 门禁也通过 |
🔴 仍未解决:top: 50vh
未改动,重新测量与上一轮逐字节一致(1440×900,80 轮,各种情况下 computed top 均为 450px):
| 输入区高度 | 实际绘制范围 | 未绘制 | Δ 页面中心 | 当前轮 marker |
|---|---|---|---|---|
| 132px | 143…768 | 15px | +5 | 可点击 |
| 200px | 143…700 | 83px | −28 | 被裁剪 |
| 260px | 143…640 | 143px | −58 | 被裁剪 |
| 360px | 143…540 | 243px | −108 | 被裁剪 |
我不再补充新论据 —— 上一轮已经把理由讲完了,决定权在你。
合并 main 带进来的三件事
1. b835e03bd 把 5 个无关文件带进了 diff。 docs/users/configuration/model-providers.md、docs/users/features/commands.md、docs/users/features/computer-use.md、docs/users/qwen-serve.md,以及 packages/cli/src/ui/hooks/slashCommandProcessor.ts。我逐个核对过:它们与「对 main 的内容执行 prettier --write」的结果逐字节相同,所以没有丢失任何内容,CI 也不会报错(CI 的 prettier 步骤是 --write 而非 --check)。但这让 PR 从 3 个文件变成 8 个,而且文档改动纯粹是表格竖线的重新对齐。建议合并前剔除。
2. 更正 —— 你其实已经解释过了,是我在读到之前就发了。 这一段是在你 10:45 / 10:56 两条评论出现之前写的。为严谨起见我没有直接采信,而是核实了:在 f7480ee07 现在是个空操作。3a9e75b21 上,分支的 packages/web-shell/package.json 里根本没有 test:e2e:smoke 这个键,而 pull_request 事件跑的是合并后的 workflow,也就是 main 的 ci.yml,它恰好调用了这个脚本。所以 smoke 任务确实是因为本 PR 暴露出的 CI 接线缺口而失败,f7480ee07 是当时正确的临时修复,合并时又有意用 main 的 Playwright 命令取代了它。这里无需处理;我"死代码"的说法是错的。
3. 新增的 web-shell E2E Smoke 门禁并不覆盖时间轴。 client/e2e/web-shell.smoke.spec.ts 里没有任何 session-timeline 选择器。所以上一轮的结论依旧成立:这个 PR 的布局部分(top、max-height、遮罩、隐藏滚动条)对 CI 仍然不可见,而恰恰是这一半藏着那两个真实缺陷。既然那个 spec 跑在真实浏览器里,它现在正是加防护的合适位置。
🟡 一个新出现的小问题(#6572 合入 main 之后)
DialogShell.module.css 现在写的是 z-index: var(--web-shell-dialog-backdrop-z-index, 1000),而 tooltip 里硬编码了 999。目前没有任何地方定义这个变量,所以 999 < 1000 依然成立,我之前的绘制顺序验证仍然有效。但 #6572 的意义正是让接入方可以覆盖它——一旦接入方把 backdrop 调到 999 以下,时间轴 tooltip 就会画在模态框上面。让两者保持耦合的写法是:
z-index: calc(var(--web-shell-dialog-backdrop-z-index, 1000) - 1);这不是阻塞项,而且我没有真的渲染一个 dialog 去验证——这一条是读 CSS 得出的。
结论
focus 修复是正确且稳健的;我尽力去让守卫卡住或吞掉真实滚动,都没能做到。其余内容在这个 head 上全部复验通过,CI 全绿(包括新的浏览器门禁)。唯一未决的仍是 top: 50vh。
|
Thanks for the re-verification and for confirming the focus fix and CI state. On I am not going to change this PR to For future verification passes, please do not keep re-testing or re-reporting 中文说明感谢复验,也感谢确认 focus 修复和 CI 状态。 关于 我不会把这个 PR 改成 后续复验请不要继续把 |
|
@qwen-code /triage |
ytahdn
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. LGTM! ✅
— qwen3.7-max via Qwen Code /review
Code ReviewRead the diff independently against the problem statement ("long session timelines expose native scrollbar, clip tooltip details, earliest markers hard to select"). My independent proposal: wrap the The PR's implementation matches and exceeds this:
No critical blockers found. No AGENTS.md violations. The code is straightforward — no over-abstraction, no unnecessary helpers. One cleanup item: the merge with Nit (non-blocking): tooltip Test ResultsThis is a web-shell React component — CLI tmux testing does not apply. Verification relied on unit tests, static analysis, and prior real-browser testing by the author and @wenshao. Unit tests (PR branch,
|
Verdict: Approve ✅This PR does what it says — makes long session timelines scrollable and usable — and does it well. The implementation is direct: a viewport wrapper with hidden scrollbars and edge fade, a portal-rendered tooltip that escapes the scroll container, programmatic centering of the current marker, and a focus-scroll guard that handles the tricky case where keyboard navigation triggers browser auto-scroll. Going in, my independent proposal for this problem was essentially what the PR implements, minus the resize sync and the dual-mechanism (rAF + timeout fallback) guard. The PR exceeds the baseline on those points. The test suite has real teeth: 43 tests including 7 new ones that cover the specific failure modes (long timeline scrolling, portal tooltip, viewport clamping, focus-scroll guard). @wenshao independently verified the focus fix in real Chromium with a 120-turn session and couldn't break the guard under stress. Revert-proof confirms the new test catches exactly the right code. CI is green across the board. Prettier, eslint, and tsc are all clean on the PR's files (6 pre-existing TS errors in Two non-blocking items for the author to consider before merge:
Neither blocks the merge. The core fix is correct, well-tested, and ready to ship. 中文说明结论:通过 ✅这个 PR 做到了它承诺的——让长时间轴可滚动且可用——而且做得很好。实现方式直接:viewport 包装器隐藏滚动条并添加边缘淡出,portal 渲染的 tooltip 脱离滚动容器,程序化居中当前 marker,focus-scroll 守卫处理键盘导航触发浏览器自动滚动的棘手场景。 我的独立方案基本上就是 PR 实现的内容,但没有 resize 同步和双机制(rAF + timeout 兜底)守卫。PR 在这两点上超越了基线。 测试套件有实际覆盖:43 个测试包含 7 个新用例,覆盖特定失败模式(长时间轴滚动、portal tooltip、视口钳位、focus-scroll 守卫)。@wenshao 在真实 Chromium 中用 120 轮会话独立验证了 focus 修复,无法破坏守卫。回退验证确认新测试恰好捕获了正确的代码。 CI 全绿。Prettier、eslint、tsc 在 PR 文件上全部通过(6 个预存 TS 错误在 两个非阻塞建议:
两项都不阻塞合并。核心修复正确、测试充分、可以发布。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ Two non-blocking suggestions in the review: drop the 5 formatting-only files from the merge commit, and consider coupling tooltip z-index to the dialog backdrop variable.







What this PR does
This fixes the Web Shell session timeline for conversations with many turns. The timeline stays centered in the viewport, the marker list scrolls internally while keeping native scrollbars hidden, the current marker is kept visible without scrolling the chat transcript, and timeline details render as a floating layer outside the scroll/fade area so hover and focus details are not clipped.
Why it's needed
Long sessions could make the timeline difficult to use: the rail could feel visually off-center, the earliest turns could be hard to reach and select, and the details tooltip near the top of the visible marker range could be clipped. The fix keeps the timeline usable for dense sessions while preserving the existing marker hover, current-range highlighting, and click-to-turn behavior.
Reviewer Test Plan
How to verify
Open a long Web Shell session with 60+ turns and confirm the left timeline remains vertically centered, scrolls internally with no visible native scrollbar, keeps the first and last markers selectable, fades near the top and bottom, and shows hover/focus details without clipping. Also verify that short sessions still use the existing visibility behavior and narrow/mobile layouts still hide the timeline.
Evidence (Before & After)
Before: long sessions could make the visual timeline range hard to navigate, with top-range tooltip details at risk of clipping and the earliest markers hard to select. After: tested in Chrome on macOS with the long session
6cb9a056-9dcd-48e6-9d38-2574f4c50aa3; the timeline viewport computedscrollbar-width: none, its::-webkit-scrollbarwasdisplay: nonewith zero width/height, and the visual crop showed only timeline markers with no native scrollbar. Automated regression coverage:npm run test --workspace=packages/web-shell -- client/components/MessageList.dom.test.tsxpassed 38 tests. Build verification:npm run build --workspace=packages/web-shellpassed.Tested on
Environment (optional)
Local dev verification used
npm run dev:daemon -- --workspace /Users/ethan/Projects/qwen-codewith Chrome against the local Web Shell.Risk & Scope
scrollbar-widthrules scoped to the timeline viewport.Linked Issues
N/A
中文说明
What this PR does
这个 PR 修复了 Web Shell 在长会话下的左侧会话时间轴。时间轴会保持在视口内垂直居中,marker 列表自身滚动但保持原生滚动条隐藏,当前 marker 会在时间轴内部保持可见且不会触发聊天正文滚动,详情浮层会脱离滚动和渐变区域渲染,避免 hover/focus 时被裁剪。
Why it's needed
长会话下时间轴会变得难用:轨道视觉上可能不够居中,最早轮次可能难以到达和选中,可视 marker 顶部附近的详情 tooltip 也有被裁剪的风险。这个修复让密集会话里的时间轴保持可用,同时保留原有 marker hover、当前范围高亮和点击跳转行为。
Reviewer Test Plan
How to verify
打开一个 60+ 轮的 Web Shell 长会话,确认左侧时间轴保持垂直居中、内部可滚动但不显示原生滚动条、首尾 marker 都能选中、顶部和底部有淡出效果,并且 hover/focus 详情不会被裁剪。同时确认短会话仍沿用原有显示门槛,窄屏/移动布局仍隐藏时间轴。
Evidence (Before & After)
修复前:长会话下可视时间轴范围难以导航,顶部附近的详情 tooltip 存在被裁剪风险,最早 marker 也可能难以选中。修复后:已在 macOS Chrome 中用长会话
6cb9a056-9dcd-48e6-9d38-2574f4c50aa3验证,timeline viewport 的 computed style 为scrollbar-width: none,::-webkit-scrollbar为display: none且宽高为 0,视觉裁剪图里只剩时间轴 marker,没有原生滚动条。自动化回归:npm run test --workspace=packages/web-shell -- client/components/MessageList.dom.test.tsx通过 38 个测试。构建验证:npm run build --workspace=packages/web-shell通过。Tested on
Environment (optional)
本地验证使用
npm run dev:daemon -- --workspace /Users/ethan/Projects/qwen-code启动 Web Shell,并在 Chrome 中测试。Risk & Scope
scrollbar-width,并且只作用在时间轴 viewport 上。Linked Issues
N/A