feat(web-shell): slide the active pill between tabs - #11844
Conversation
The tab list used to recolor each trigger in place, so switching tabs (e.g. Tasks/Channels in the sidebar) read as a flat cross-fade. Render a single pill in the list that measures the active trigger and glides to it, arming transitions only after first paint and honoring reduced motion.
|
E2E test report (2026-09-14, local Chromium, vite dev server + mock daemon):
Evidence plan and frame stills (resting Tasks state, mid-slide frame, resting Channels state) are in |
|
Thanks for the PR! Template looks good ✓ — every required heading is filled in, including the Risk & Scope bullets and a complete Chinese translation. Problem: this is a Direction: aligned. Web Shell UI polish is squarely in scope, and the two house rules this could have tripped are both respected — the change lands in the shared Size: not applicable — no core paths. All four files sit under Approach: scope feels right and the diff is focused — no drive-by refactors and no unrelated churn. The One correction to the description, since you enumerated the affected surfaces: it is nine default-variant Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需标题都已填写，包括 Risk & Scope 各项和完整的中文翻译。 问题： 这是一个 方向： 对齐。Web Shell 的 UI 打磨完全在范围内，而且这个改动可能踩到的两条项目规矩都遵守了 —— 改动落在共享的 规模： 不适用 —— 未触及核心路径。四个文件全部位于 方案： 范围合理，diff 聚焦 —— 没有顺手重构，也没有无关改动。 有一处需要更正你的描述，因为你逐条列出了受影响的界面：实际是 8 个文件里的 9 处 default 变体 风险： 无升级风险信号 —— 未触及任何与回滚相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewMy independent proposal before reading the diff: make No correctness blockers found. Two things to fix before this merges, neither in the product code: 1. The new browser spec never runs as a PR gate. CI's only pre-merge web-shell browser job is 2. So this is PR-caused and PR-fixable, not infra noise: merge or rebase Things I checked that are not broken — recording them so the next reader does not have to re-derive them:
Not verified: the actual rendered result. No pre-merge CI lane renders these pixels (the visuals suite is capture-only — no CI test evidenceThe PR's own CI, read via the API at the reviewed commit — nothing was built or executed locally. Bot orchestration checks (triage, review-pr, assign, label, authorize, and the skipped publish/resolve/precheck jobs) are omitted as noise; Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Sandboxed verification would settle this: 中文说明代码审查在读 diff 之前我自己的方案是：让 未发现正确性阻塞问题。 合并前有两件事要处理，都不在产品代码里： 1. 新增的浏览器测试不会作为 PR 门禁运行。 CI 中唯一在合并前跑的 web-shell 浏览器任务是 2. 我检查过、确认没有问题的点 —— 记录下来，免得下一位读者重新推导：
未验证： 实际渲染结果。合并前没有任何 CI 通道会渲染这些像素（visuals 套件只做截图捕获 —— 没有 CI 测试证据以上是通过 API 在受审 commit 上读取的、该 PR 自身的 CI —— 本地没有构建或执行任何东西。机器人编排类检查（triage、review-pr、assign、label、authorize，以及被跳过的 publish/resolve/precheck 各 job）作为噪音省略； CI 表格见上方英文部分（该表格由后续流程原地更新，此处不重复）。 沙箱验证可以定论： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Confidence: 3/5 — the code is right as far as I can tell, but nothing has actually executed it: static analysis never ran, and the one real-browser check for the headline claim is not wired into any pre-merge gate. Stepping back. My independent proposal for this converged on the same design, which is usually a good sign — I am not looking at an approach I would have done more simply. The "not broken" list in my review is long because there were several plausible traps here and none of them fired: an absolutely-positioned child inside two Context that raises my prior on the direction call: this author wrote #11262, the unified session sources feature that created the Tasks/Channels switch in the first place. So this is the owner of that surface improving its feedback, not a stranger decorating it. I would normally push harder on an app-wide animation with no linked issue behind it — nobody reported that tab switching was hard to follow — but a maintainer with write access polishing a control they built, in the shared primitive where AGENTS.md says it belongs, is a reasonable way for that change to arrive. The blast radius is still under-described in the PR body (nine default-variant lists across eight files, not seven; Why 3/5 and not 4/5. I cannot describe this as "solid, only non-blocking nits" when the diff has had zero lint, format, or static analysis — Why not lower. Nothing in the diff is wrong, and I am not going to file a formal request-changes over a stale branch and a missing test tag. That would be theatre. One forward-looking fragility, non-blocking but worth knowing before someone maintains this in six months: What would move me to approve: merge or rebase So: not approving this run, and not requesting changes. @callmeYe — the two items above are yours and both are small; ping for a re-run once they land and CI is green on the new head. 中文说明Confidence: 3/5 —— 就我能判断的而言代码是对的，但实际上没有任何东西真正执行过它：静态分析从未运行，而针对核心论点的唯一真实浏览器检查也没有接入任何合并前门禁。 退一步看。我为这个问题独立构想的方案与它收敛到了同一个设计，这通常是个好信号 —— 我面对的不是一个我本可以做得更简单的方案。审查里那份"没有问题"的清单之所以长，是因为这里有几个看起来很可能会踩的坑，而一个都没踩中：两个 有一点提升了我对方向判断的先验：这位作者写了 #11262，也就是最初创建 Tasks/Channels 切换的统一会话来源功能。所以这是该界面的负责人在改进它自己的反馈，而不是一个外人在给它做装饰。对于一个背后没有关联 issue 的全局动画，我通常会追问得更狠一些 —— 并没有人上报过标签切换难以辨认 —— 但一位有写权限的维护者去打磨自己构建的控件，并且改在 AGENTS.md 指定应该改的共享基础组件里，这是这类改动合理的来路。不过影响范围在 PR 描述里仍然被低估了（8 个文件里的 9 处 default 变体列表，不是 7 处；枚举里漏掉了 为什么是 3/5 而不是 4/5。当这个 diff 完全没有经过 lint、格式化或静态分析时 —— 为什么不更低。diff 里没有任何东西是错的，而我也不打算为一个落后的分支和一个缺失的测试标记提交正式的 request-changes。那只是做戏。 有一处面向未来的脆弱点，非阻塞，但在六个月后有人维护它之前值得知道： 能让我转向批准的条件：合并或 rebase 所以：本轮不批准，也不提交 request-changes。 @callmeYe —— 上面两项是你的，而且都很小；处理完、CI 在新的 head 上变绿之后，回复一声重跑即可。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
The tab trigger is a plain button with no cursor rule, so it kept the UA default arrow; give it the pointer the rest of the sidebar uses.
Flex triggers defaulted to min-width:auto, so on narrow lists their content could not shrink, justify-center split the overflow to both sides, and the active trigger (and the pill mirroring it) poked past the list's padding edge; unequal labels also produced unequal tab widths despite flex-1. min-w-0 lets the triggers shrink to equal shares, which keeps the indicator inside the frame.
Once triggers can shrink (min-w-0), a label wider than its slot overflows symmetrically from the centered content and paints over the sliding pill's edge. Clip content to the trigger box in the default variant; the line variant keeps visible overflow because its underline sits outside the box.
Equal halves can undercut a label in narrow sidebars or wider font environments, and the trigger's overflow guard then cut the text mid-glyph. Truncating spans turn that into a clean ellipsis while leaving the common case untouched.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": the two new Playwright specs were not executed in a real browser (they need the npm run dev vite server plus a Chromium install), so the pill's measured geome….
Test Plan (not a blocker): components/ui/tabs.test.tsx — no such file or directory.
中文说明
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：the two new Playwright specs were not executed in a real browser (they need the npm run dev vite server plus a Chromium install), so the pill's measured geome…。
Test Plan（非阻断）：components/ui/tabs.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.23.3)
Local verification on a real stack (maintainer)I built Short version: the sliding pill itself does exactly what the description says, and I'd like it in. But the three follow-up commits pushed after the triage review ( Setup
What holds up1 · It really slides, and the fill really moved onto the pill. Frames captured by pausing the live CSS transition at fixed
2 · Reduced motion — computed 3 · Keyboard — 4 · Vertical orientation works. No consumer passes 5 · Rest-state fill is unchanged. Inside the pill, 0–6 of 6732 sampled pixels differ (text antialiasing, dark and light, both tabs). See finding 2 for the edge. 6 · Everything green here. 7 · The unit tests have teeth, but only for presence. Mutating the component to never render the indicator fails 3 of 4 tests; pinning the pill at a wrong fixed box ( Findings1 · Blocking — tab labels are now hard-clipped mid-word, with no ellipsis, on shipped surfaces
Measured at 1440×900 against the same daemon and the same data:
Viewport-independent: identical at 1100, 1440 and 1920 px.
A/B in the head tree: removing Suggested shape:
2 · The new Playwright spec fails here — deterministically, 3/3 — and CI still never runs it
The spec is also still untagged, so
3 · Minor — the pill trails the tab during a sidebar resize dragThe 200 ms transition also applies to 4 · Minor / latent — a list that first mounts hidden flies in from the origin
5 · Doc nit — the
|
| 代码树 | head 5c6f1d5 与 merge base c666ec1，各自独立 worktree、各自 npm ci |
| 栈 | 一个真实 daemon（node scripts/dev.js serve，隔离的 QWEN_HOME/QWEN_RUNTIME_DIR，fixture 工作区）+ 两个 vite dev（head :5911、base :5912）指向同一个 daemon，因此唯一变量就是代码树 |
| 驱动 | Playwright 1.61.1 / Chromium 149，1440×900 @ DPR 2（另测 1100 与 1920），en + zh-CN，深色 + 浅色 |
| 机器 | macOS 26.6.2（Darwin 25.6.0），Node 24.18.1 |
站得住的部分
- 确实在滑动，且背景确实转移到了滑块上。 用固定
currentTime暂停真实 CSS 过渡抓帧：滑块left在 t = 0/25/50/100/200 ms 分别为 23 → 61.4 → 84.8 → 112.8 → 130 px，同时两个标签的background-color在每个采样点都是rgba(0,0,0,0)。base 上没有任何元素移动，两个标签在原地交叉渐变（alpha 0.045 → 0、0 → 0.045）。实时 rAF 采样也一致（23 … 91.0 @116 ms … 130 @230 ms）。 - Reduced motion —— 计算值
transition-property: none，整个切换过程中滑块只有[23, 130]两个位置：直接跳过去，没有中间帧。 - 键盘 ——
ArrowLeft/ArrowRight同样带动滑块（Radix 自动激活），dx = 0。 - 纵向排列可用。 仓库里没有任何使用方传
orientation="vertical"，所以这条路径此前从未被执行过；两棵树分别渲染后，滑块沿y从 56 移到 108，dx = dy = 0，测量中top/height的那一半是正确的。 - 静止态填充没有变化。 滑块内部 6732 个采样像素中只有 0–6 个不同（文字抗锯齿，深浅色、两个标签都如此）。边框部分见发现 2。
- 本机全绿。
packages/web-shellvitest：head 313 个文件 / 8011 条通过，base 312 / 8006，差值正好是本 PR 新增的文件与用例。tabs.test.tsx4 条 ✓、react18-ref-compat.test.tsx31 条 ✓、typecheck✓、eslint✓、prettier --check✓。给 PR 描述的一条更正：所谓**「既有的 jsdomlocalStorage环境问题」在这里复现不出来** —— 干净npm ci之后两棵树都完全通过，所以那应该是你本机的问题，而不是仓库的问题。 - 单测有辨别力，但只针对「是否渲染」。 把组件改成永不渲染滑块，4 条中有 3 条失败；而把滑块钉死在错误的固定位置（
left:0; top:0; width:4; height:4），4 条仍然全过 —— jsdom 不计算布局，所以只有浏览器 spec 能固化定位。这正好引出发现 2。
发现
1 · 阻塞项 —— 标签现在会被硬裁切（无省略号），且发生在已发布的界面上。
TabsTrigger 是 flex-1（flex: 1 1 0%）。base 依赖 flex 默认的 min-width: auto 把每个标签保持在内容宽度。7fa81bb 加了 min-w-0，57a62ff 加了 group-data-[variant=default]/tabs-list:overflow-hidden —— 两者任意一个单独存在都会把自动最小尺寸降为 0 —— 于是所有 default 标签列表现在都把宽度平均分配，并把最长的标签切掉。只有侧栏拿到了 truncate（5c6f1d5），其它地方是直接切断，而且因为文字居中，是两端都被切。
在 1440×900、同一 daemon、同一数据下实测：
| 界面 | PR base | 本 PR |
|---|---|---|
Plugins 头部（Extensions / MCP / Skills / Agents） |
84 / 43 / 47 / 58 px，标签完整 | 各 58 px；Extensions 溢出 13 px → 显示成 xtension |
智能体详情（Basic Information / System Prompt / Tools / MCP / Hooks） |
127 / 113 / 47 / 43 / 54 px，完整 | 各 77 px；溢出 25 px 与 18 px → sic Informat / ystem Promp |
Plugins 头部（zh-CN） |
40 / 43 / 40 / 54 px | 各 44 px；智能体 被切 5 px |
max-w-full overflow-x-auto 列表（Agents 管理页、Extensions 管理页、Agent 创建页） |
内容宽度，真正可横向滚动（scrollLeft 会变） |
7 个等宽 36 px 格子，7 个标签里 5 个被裁，scrollLeft 恒为 0 —— 列表不再滚动 |
| 侧栏拖到 220 px 最小宽度（本 PR 自己的界面） | Tasks 74 / Channels 95，都完整 |
85 / 85 → Chan… |
与视口无关：1100、1440、1920 px 下结果一致。
在 head 树上做的 A/B：只删 min-w-0 不够（overflow-hidden 仍把最小尺寸压到 0）。两个都删、并保留侧栏的 truncate span 之后，每个界面都回到 base 的宽度（Plugins 84/43/47/58、智能体详情 0 px 溢出、侧栏 74/95），而滑块照常工作 —— 它测的就是标签最终的尺寸。
建议的形状：
- 从基础组件里删掉
min-w-0与group-data-[variant=default]/tabs-list:overflow-hidden； - 如果侧栏想要左右等宽，就把
min-w-0放到侧栏那两个TabsTrigger上（它们已经带了会截断的 span），而不是加在全应用所有标签上； - 如果仍然想要一个全局兜底，请用
truncate（省略号）而不是硬切。
2 · 新增的 Playwright spec 在本机稳定失败（3/3），而 CI 依然不会跑它。
web-shell.tabs-indicator.spec.ts:36 › pill overlays the active trigger and slides on switch
Expected: 106.5 <- 标签的 getBoundingClientRect().width
Received: 107 <- 滑块宽度，来自 active.offsetWidth（整数）
Expected difference: < 0.5 ; Received difference: 0.5
offsetLeft/offsetWidth 是四舍五入后的整数，而 flex 布局落在半像素上，所以滑块低 0.5 px、宽 0.5 px，而 toBeCloseTo(…, 0) 要求小于 0.5。同一个原因也让「静止态逐像素一致」这个说法对填充成立、对边框不成立：内部完全一致，但边框环整体移动了一个设备像素 —— 占整条标签栏 2.5%（深色）到 4.1%（浅色），单通道最大差值 46。改用 getBoundingClientRect() 测量可以同时修好 spec 和对齐。
另外该 spec 仍未打标记，所以 npm run test:e2e:smoke（--grep @smoke）不会选中它 —— triage 评审的第 1 条在当前 head 上仍未处理，这也正是上面这个失败此前没人发现的原因。
3 · 次要 —— 拖动侧栏改变宽度时滑块会拖尾。 200 ms 过渡同样作用于 ResizeObserver 触发的重新测量，因此拖动过程中滑块滞后：正常拖动下 |Δleft| 最大 10.7 px（Channels 激活），松手后约 200 ms 归位（单次 140 px 跳变时 t = 40 ms 偏差 69.5 px，t = 200 ms 为 0.1 px）。「始终贴合」在静止态成立，拖动过程中不成立。让 ResizeObserver 触发的测量不走过渡即可。
4 · 次要 / 潜在 —— 首次在隐藏容器中挂载的列表会从原点飞入。 ready 在挂载后的第一个 requestAnimationFrame 就置位，不管列表当时是否有可测量的盒子。一个 default TabsList 若首次渲染在 display: none 里、之后才显示，滑块会飞入：x 29 → 83、宽度 2 → 53，约 200 ms。目前没有使用方会触发（Radix 会卸载未激活的 TabsContent），所以只是潜在问题 —— 把 ready 改成在第一次测到非零尺寸后才置位即可。
5 · 描述上的小出入 —— line 变体并非未受影响。 min-w-0 加在所有变体的标签上，所以 Workflows 的标签栏也变了：Saved/Running/History 106.15 / 119.91 / 113.6 px → 各 113.22 px，标签位置与下划线宽度随之改变。影响无害，但这正是本 PR 的 visuals 预览标出 workflow 页面的原因。
覆盖面备注
我还在两棵树上跑了仓库自带的 visuals 截图套件：71 张里 51 张完全一致，20 张有差异 —— 其中 12 张只是滑块边缘的亚像素差（单通道最大差值 ≤ 9），8 张是 workflow 页面（文字位移，见发现 5）。发现 1 涉及的界面没有任何套件会截到，而能覆盖滑块的浏览器 spec 又不在门禁里 —— 所以本仓库没有任何一条通道能拦下这次裁切回归。
结论
核心改动是好的，而且从 diff 中无法验证的两条主张（确实在滑动、确实遵循 reduced motion）现在已经在真实栈上得到确认，另外还补上了此前从未被执行过的纵向排列。合并前希望处理：
- 从基础组件里去掉或收窄
min-w-0+overflow-hidden（发现 1）； - 用
getBoundingClientRect()测量，让滑块与 spec 一致（发现 2）； - 给 spec 打
@smoke，让 CI 真的跑它。
推完之后叫我一声，我会用同一套装置对新的 head 复测。
Keep the overflow policy of every other tab list intact: min-w-0 and the default-variant overflow guard move off the primitive; the sidebar keeps equal halves by opting its own triggers into min-w-0, which its truncating labels already support. Measure the pill with fractional rects so it lands exactly on half-pixel flex boundaries, skip the transition for resize-driven measurements so the pill tracks a sidebar drag 1:1, and arm transitions only once a measurable box exists so a list revealed from display:none never flies in from the origin. The pill now also dims with a disabled active trigger, TabsList stops advertising an asChild it can no longer honour, and the variant fallback resolves once. Tests cover the hide path for real and the resize-vs-switch animation split; the e2e spec joins the smoke gate.
|
@wenshao Thanks for the thorough real-stack run — the label-clipping table and the 0.5px boundary analysis were exactly right. New head
Also folded in from the inline review, same push: finding 4 — transitions arm on the first measurable box, so a list revealed from Deliberately not in this PR: R1-6 (forwardRef on One correction to my earlier caveat, per your note: the jsdom Local gates on |
Re-run on
|
| # | Ask | On c555bc4 |
|---|---|---|
| 1 | drop min-w-0 / overflow-hidden from the primitive |
Fixed. Every surface is back to base widths, same daemon, same data: Plugins header 85.8 / 45.2 / 48.7 / 60.3 px, zh-CN 42 / 45.2 / 42 / 56, agent detail 129.3 / 115.3 / 49.0 / 45.2 / 55.8, extension detail 75.7 / 101.5 / 61.3 / 70.6 / 110.3 / 109.7, Workflows (line) 106.2 / 119.9 / 113.6 — all identical to base, 0 clipped labels (scrollWidth − clientWidth) anywhere. The overflow-x-auto strips scroll again (probe: scrollLeft 223), the pill scrolls with the content and lands on a tab activated while scrolled (Δ = 0 on all four axes). |
| 2 | measure with getBoundingClientRect() |
Fixed. Pill vs active trigger Δleft/Δtop/Δwidth/Δheight = 0.000 at rest in every list I measured (sidebar dark + light, both tabs, Plugins, agent, extension, MCP). Pixel diff of the sidebar strip head vs base at rest: 1 of 28 032 pixels differs (max channel Δ 13) — last round the border ring moved on 2.5–4.1 % of the strip. The spec passes 6/6 here (3 repeats × 2 cases). |
| 3 | tag the spec @smoke |
Done. The smoke job on this head ran both cases ([56/80], [57/80], 80 passed) — run. |
Paused-transition frames on this head: pill left 23 → 61.25 → 84.5 → 112.38 → 125.69 → 129.5 px at 0 / 25 / 50 / 100 / 150 / 200 ms, both triggers rgba(0,0,0,0) at every sample; base still cross-fades in place (alpha 0.045 → 0 and 0 → 0.045). Live rAF trace: 5 intermediate positions, settles at Δ = 0. Reduced motion: transition-property: none, positions [23, 129.5]. Keyboard ArrowLeft/Right: Δ = 0.
Also fixed since 5c6f1d5
- Resize drag (finding 3): during a 4 px/frame drag the pill is at most 2 px behind, for one frame, with no easing — last head trailed by 10.7 px with a 200 ms ease. A 140 px width jump is corrected on the next frame (2 sampled frames off, 0 at 40 / 100 / 200 ms). The transition class is absent during resize-driven writes and present again on the next switch.
- Reveal from
display:none(finding 4): after the reveal the pill takes exactly two positions, the hidden one and its final one (32 → 86.8 px), one sample apart, no transition class — it snaps. - Disabled active trigger (R1-4): pill opacity 0.5 while disabled, 1 again on re-enable, Δ = 0 throughout.
- Vertical orientation still tracks
ywith Δ = 0;linevariant renders no pill.
Findings
1 · New in c555bc4 — dialog-hosted lists open with a 5 % short pill (fix-induced)
DialogContent and AlertDialogContent enter with zoom-in-95 (a 100 ms keyframe from scale(.95)). The first measure() runs in useLayoutEffect while that keyframe is on its first frame (computed transform matrix3d(0.953…) right after open), getBoundingClientRect() returns the transformed boxes, and the pill is written 5 % too small. Nothing re-measures afterwards: the ResizeObserver never fires (the layout box did not change) and there is no data-state mutation. Measured 700 ms after open, dialog already at transform: none:
| Surface | active tab | pill | Δw / Δh | after one tab switch |
|---|---|---|---|---|
Extensions › Add (AlertDialog, grid-cols-2) |
269 × 25 px | 255.55 × 23.75 | −13.45 / −1.25 | 0 / 0 |
Skills › Upload (Dialog, grid-cols-3) |
158 × 25 px | 150.09 × 23.75 | −7.91 / −1.25 | 0 / 0 |
Reproduces on every open, also under prefers-reduced-motion (the enter keyframe still runs). Δleft is only −0.15 px because the default tab sits 3 px into the list; the same 5 % on a third tab is a 12 px offset (probe: a list mounted under scale(.95), third tab active, ends at Δleft −11.95 / Δw −5.9 after the transform is removed).
Attribution: the same head with only the measurement reverted to offsetLeft/offsetWidth gives Δw 0 in both dialogs (with the old Δtop 0.5 back), so this comes from the finding-2 fix, not from the pill itself.
Suggested shape — validated as a third arm in the rig — divide the bounding rects by the list's own bounding/layout ratio so the pill is written in the list's untransformed coordinate space:
const listStyle = getComputedStyle(list);
const layoutWidth = parseFloat(listStyle.width);
const layoutHeight = parseFloat(listStyle.height);
const scaleX = layoutWidth > 0 ? listRect.width / layoutWidth : 1;
const scaleY = layoutHeight > 0 ? listRect.height / layoutHeight : 1;
// …
left: (rect.left - listRect.left) / scaleX + list.scrollLeft,
top: (rect.top - listRect.top) / scaleY + list.scrollTop,
width: rect.width / scaleX,
height: rect.height / scaleY,With that hunk: Extensions dialog Δw −0.016 / Δh 0, Skills dialog 0 / 0, the scaled probe −0.016 / 0 / 0 / 0, and every other number in this report is unchanged (paused frames identical, reduced motion, keyboard, resize, scroll strip, spec 6/6). An alternative is to re-measure when an ancestor animation finishes (document.getAnimations() filtered to effects whose target contains the list, then .finished), but the ratio is one hunk and needs no event plumbing. A jsdom case that stubs the list rect at 0.95× its computed width would pin it.
2 · Note, not a blocker — the sidebar at its 220 px minimum
Base fits both labels there (75.6 + 97.4 = 173 < 179 px); with its two triggers in min-w-0 this head splits 86.5 / 86.5 and shows Chan…. That is the documented worst case — noting it only because it is the one surface where a label still differs from base. If you would rather keep both labels whole at the minimum, basis-auto on those two triggers does that at the price of exactly equal halves (each gets its content width plus half the slack) — your call.
3 · Coverage note
Mutation matrix against tabs.test.tsx (5 cases): pin the pill at a wrong box, swap the resize/switch flags, never arm → each killed (1 failed). Drop + list.scrollLeft, ignore listRect.left, drop the disabled dimming, arm on a zero-width box → all survive (5/5). Those four are pinned only by the browser (my probe page), so the R1-4 and finding-4 fixes have no test guard — the same stubbed-rect technique the new case uses covers the disabled opacity and the zero-width arming cheaply. Not blocking.
Gates on c555bc4 (this machine)
typecheck ✓ · eslint ✓ · prettier --check ✓ · packages/web-shell vitest 313 files / 8012 tests ✓ (last head 8011, base 8006 — the delta is the new resize-vs-switch case) · tabs.test.tsx 5 ✓ + react18-ref-compat.test.tsx 31 ✓ · web-shell.tabs-indicator.spec.ts 6/6 across 3 repeats ✓.
Verdict
Three asks closed and confirmed on the real stack, plus findings 3 / 4 and R1-4. One new one-hunk regression in the two dialog-hosted lists (finding 1) — fold in the scale ratio (or equivalent) and I'll re-run the dialog check; everything else is ready.
中文说明
在 c555bc4 上复测（维护者，同一套装置）
装置与上一轮相同：head c555bc4 与 merge base c666ec1 各自独立 worktree，一个真实 qwen serve daemon（隔离的 QWEN_HOME/QWEN_RUNTIME_DIR，fixture 工作区带 3 个项目级 agent、1 个扩展、1 条 MCP 配置），每棵树一个 vite dev 指向同一个 daemon；Playwright 1.61.1 / Chromium 149，1440×900 @2x，en + zh-CN，深色 + 浅色；macOS 26.6.2 / Node 24.18.1。本轮为归因额外加了两臂：head 上只把测量改回 offsetLeft/offsetWidth，以及 head + 一个单 hunk 的候选修复（发现 1）。
结论先说：上一轮的三条合并要求全部已处理，且在真实栈上站得住；resize 拖尾、display:none 飞入、禁用变淡也都修好了。但发现 2 的修复带来了一个新回归：两个挂在对话框里的标签列表（Extensions › Add、Skills › Upload）打开时滑块小了 5%，并一直保持到下一次切换标签。下面给出了已在装置里验证过的单 hunk 修法；补上之后我会批准。
上一轮的要求
| # | 要求 | c555bc4 上 |
|---|---|---|
| 1 | 从基础组件去掉 min-w-0 / overflow-hidden |
已修。 同一 daemon、同一数据下所有界面回到 base 宽度：Plugins 头部 85.8 / 45.2 / 48.7 / 60.3 px，zh-CN 42 / 45.2 / 42 / 56，智能体详情 129.3 / 115.3 / 49.0 / 45.2 / 55.8，扩展详情 75.7 / 101.5 / 61.3 / 70.6 / 110.3 / 109.7，Workflows（line）106.2 / 119.9 / 113.6 —— 与 base 完全一致，任何地方都没有被裁的标签（scrollWidth − clientWidth 全为 0）。overflow-x-auto 列表重新可以滚动（probe：scrollLeft 223），滑块随内容滚动，滚动状态下激活标签时也精确落位（四个轴 Δ = 0）。 |
| 2 | 用 getBoundingClientRect() 测量 |
已修。 我测的每个列表（侧栏深色 + 浅色、两个标签、Plugins、智能体、扩展、MCP）静止态滑块与激活标签的 Δleft/Δtop/Δwidth/Δheight 都是 0.000。侧栏条带静止态 head vs base 逐像素对比：28 032 个像素里只有 1 个不同（单通道最大差 13）；上一轮边框环移动影响了条带的 2.5–4.1%。spec 本机 6/6（3 次重复 × 2 用例）。 |
| 3 | 给 spec 打 @smoke |
已做。 这个 head 的 smoke job 跑了两个用例（[56/80]、[57/80]，80 通过）—— 运行。 |
本 head 上暂停过渡抓的帧：滑块 left 在 0 / 25 / 50 / 100 / 150 / 200 ms 分别为 23 → 61.25 → 84.5 → 112.38 → 125.69 → 129.5 px，每个采样点两个标签的背景都是 rgba(0,0,0,0)；base 仍是原地交叉渐变（alpha 0.045 → 0、0 → 0.045）。实时 rAF 采样 5 个中间位置，终点 Δ = 0。Reduced motion：transition-property: none，位置只有 [23, 129.5]。键盘 ArrowLeft/Right：Δ = 0。
自 5c6f1d5 以来一并修好的
- resize 拖动（发现 3）： 以每帧 4 px 拖动时滑块最多落后 2 px、只持续一帧、没有缓动 —— 上一个 head 是 10.7 px 拖尾 + 200 ms 缓动。140 px 的宽度跳变下一帧就纠正（2 个采样帧偏离，40 / 100 / 200 ms 时为 0）。resize 驱动的写入没有过渡类名，下一次切换时又恢复。
- 从
display:none显示（发现 4）： 显示之后滑块只出现两个位置 —— 隐藏态和最终位置（32 → 86.8 px），相隔一个采样、没有过渡类名，直接到位。 - 激活标签被禁用（R1-4）： 禁用期间滑块 opacity 0.5，重新启用后回到 1，全程 Δ = 0。
- 纵向排列仍然 Δ = 0 跟随
y；line变体不渲染滑块。
发现
1 · c555bc4 新引入 —— 对话框里的标签列表打开时滑块小 5%（修复引入）
DialogContent 与 AlertDialogContent 进场动画是 zoom-in-95（100 ms 关键帧，从 scale(.95) 开始）。第一次 measure() 在 useLayoutEffect 里执行时，这个关键帧正处于第一帧（刚打开时计算得到的 transform 是 matrix3d(0.953…)），getBoundingClientRect() 返回的是变换后的盒子，于是滑块被写成 5% 偏小。之后没有任何东西重新测量：ResizeObserver 不会触发（布局盒子没变），也没有 data-state 变化。打开 700 ms 后、对话框已经 transform: none 时实测：
| 界面 | 激活标签 | 滑块 | Δw / Δh | 切换一次标签后 |
|---|---|---|---|---|
Extensions › Add（AlertDialog，grid-cols-2） |
269 × 25 px | 255.55 × 23.75 | −13.45 / −1.25 | 0 / 0 |
Skills › Upload（Dialog，grid-cols-3） |
158 × 25 px | 150.09 × 23.75 | −7.91 / −1.25 | 0 / 0 |
每次打开都复现，prefers-reduced-motion 下也一样（进场关键帧照常运行）。Δleft 只有 −0.15 px 是因为默认标签离列表左边只有 3 px；同样的 5% 落到第三个标签上就是 12 px 的偏移（probe：列表挂载在 scale(.95) 下、第三个标签激活，去掉变换后 Δleft −11.95 / Δw −5.9）。
归因：同一个 head 上只把测量改回 offsetLeft/offsetWidth，两个对话框的 Δw 都是 0（同时老的 Δtop 0.5 回来了），所以问题来自发现 2 的修复，而不是滑块本身。
建议的形状 —— 已作为第三臂在装置里验证 —— 用列表自身的「包围盒 / 布局尺寸」比值把变换除掉，让滑块写在列表未变换的坐标系里：
const listStyle = getComputedStyle(list);
const layoutWidth = parseFloat(listStyle.width);
const layoutHeight = parseFloat(listStyle.height);
const scaleX = layoutWidth > 0 ? listRect.width / layoutWidth : 1;
const scaleY = layoutHeight > 0 ? listRect.height / layoutHeight : 1;
// …
left: (rect.left - listRect.left) / scaleX + list.scrollLeft,
top: (rect.top - listRect.top) / scaleY + list.scrollTop,
width: rect.width / scaleX,
height: rect.height / scaleY,加上这个 hunk 之后：Extensions 对话框 Δw −0.016 / Δh 0，Skills 对话框 0 / 0，缩放 probe −0.016 / 0 / 0 / 0，本报告里其它所有数字不变（暂停帧完全一致、reduced motion、键盘、resize、滚动列表、spec 6/6）。另一种做法是等祖先动画结束后重测（document.getAnimations() 过滤出目标包含该列表的动画，再等 .finished），但比值法只有一个 hunk、不用接事件。用一个把列表 rect 桩成计算宽度 0.95 倍的 jsdom 用例就能固化。
2 · 备注，非阻塞 —— 侧栏 220 px 最小宽度
base 在那里放得下两个完整标签（75.6 + 97.4 = 173 < 179 px）；这个 head 给侧栏两个标签加了 min-w-0，于是等分成 86.5 / 86.5 并显示 Chan…。这是描述里写明的最坏情况 —— 只是记一笔，因为这是唯一一个标签仍与 base 不同的界面。如果更想在最小宽度下两个标签都完整，给这两个标签加 basis-auto 即可，代价是不再严格等分（各自拿到内容宽度加一半富余）—— 由你定。
3 · 覆盖面备注
对 tabs.test.tsx（5 个用例）做的变异矩阵：把滑块钉在错误位置、交换 resize/切换的标志、永不 arm → 各被杀死（1 失败）。去掉 + list.scrollLeft、忽略 listRect.left、去掉禁用变淡、在零宽度时 arm → 全部存活（5/5 通过）。这四条目前只靠浏览器（我的 probe 页）钉住，所以 R1-4 与发现 4 的修复没有测试守护 —— 新用例用的桩 rect 技术同样能便宜地覆盖禁用 opacity 与零宽度 arm。不阻塞。
c555bc4 上的门禁（本机）
typecheck ✓ · eslint ✓ · prettier --check ✓ · packages/web-shell vitest 313 个文件 / 8012 条 ✓（上个 head 8011、base 8006 —— 差值就是新增的 resize-vs-切换用例）· tabs.test.tsx 5 ✓ + react18-ref-compat.test.tsx 31 ✓ · web-shell.tabs-indicator.spec.ts 3 次重复 6/6 ✓。
结论
三条要求已关闭并在真实栈上确认，发现 3 / 4 与 R1-4 也一并修好。两个对话框里的标签列表有一个新的单 hunk 回归（发现 1）—— 补上比值（或等价做法）后我会复测对话框这一项；其它都已就绪。
Superseded: every finding in this round is addressed at head c555bc4 (see the per-thread replies and the summary comment) or explicitly deferred as pre-existing (R1-6). A fresh review is queued on the current head and will govern from here.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
What this PR does / PR 主旨
Adds a sliding pill indicator to the Web Shell's default-variant tab lists: instead of recoloring each tab in place, a single absolutely-positioned pill measures the active trigger and glides to it over 200ms. It snaps (no transition) on first render and on container resizes so it tracks a sidebar drag 1:1, dims when the active trigger is disabled, supports vertical lists, honors prefers-reduced-motion, and leaves the line (underline) variant untouched. TabsList becomes a ref-forwarding component; the sidebar's two triggers opt into shrink-to-equal halves with truncating labels locally.
为 Web Shell 的 default 变体标签列表加入滑动药丸指示器：不再就地给每个标签重新着色，而是一个绝对定位的药丸测量当前激活触发器并以 200ms 滑过去。首帧与容器缩放时直接吸附（无过渡），从而 1:1 跟随侧栏拖拽；激活触发器被禁用时变暗；支持纵向列表；遵循 prefers-reduced-motion；line（下划线）变体不受影响。TabsList 改为转发 ref 的组件；侧栏的两个触发器在本地选择「等分收缩 + 标签截断」。
Static / diff-only review; verified directly against head tree c555bc45.
仅静态 / diff 审查；直接在 head tree c555bc45 上核实。
🎉 Clean implementation, and the prior bot round is fully addressed / 实现干净，且上一轮 bot 意见已全部处理
I independently read the head-tree useSlidingIndicator and confirmed the design is sound:
我独立阅读了 head tree 的 useSlidingIndicator，确认设计正确：
- Measurement uses
getBoundingClientRect()(fractional) pluslist.scrollLeft/scrollTop, so the pill shares the trigger's exact box (tabs.tsx:88-101).
测量使用getBoundingClientRect()（含小数）加上list.scrollLeft/scrollTop，因此药丸与触发器共享精确的盒子（tabs.tsx:88-101）。 - Animate vs snap is correctly split:
MutationObserver(data-state/disabled/childList) drives animated re-measures,ResizeObserverdrives non-animated ones, so a tab switch slides while a sidebar drag tracks 1:1 (:108-122). A tab switch doesn't resize the list (all triggers stay mounted), so the two observers don't fight.
动画与吸附正确分离：MutationObserver（data-state/disabled/childList）驱动带动画的重测，ResizeObserver驱动不带动画的重测，因此切换标签会滑动、而拖拽侧栏 1:1 跟随（:108-122）。切换标签不会改变列表尺寸（所有触发器始终挂载），所以两个 observer 不会互相打架。 - Arm-on-first-measurable-box (
arm()only whenrect.width > 0,:104-106) prevents the pill flying in from the origin on mount or on a reveal fromdisplay:none.
首个可测量盒子才武装（arm()仅在rect.width > 0时，:104-106）避免挂载时或从display:none显现时药丸从原点飞入。 - Cleanup cancels the rAF and disconnects both observers; the effect deps
[listRef, enabled]are stable, so no leak (:124-131).
清理取消 rAF 并断开两个 observer；effect 依赖[listRef, enabled]稳定，无泄漏（:124-131）。 - The active background moved out of
TabsTriggerinto the pill, and the disabled dimming is written inline (opacity: 0.5when the measured trigger:disabled) so it isn't beaten by the inline geometry;asChildis omitted fromTabsListProps(the two-child render can't be slotted);variant ?? 'default'is resolved once and read consistently.
激活背景从TabsTrigger移到药丸，禁用变暗以内联写入（测得的触发器:disabled时opacity: 0.5），以免被内联几何样式覆盖；TabsListProps省略了asChild（双子节点渲染无法 slot）；variant ?? 'default'只解析一次并被一致读取。
The earlier bot round's one Critical (R1-1: min-w-0/overflow-hidden leaking onto the shared trigger base) and its Suggestions are all genuinely fixed at this head — the truncation policy now lives only on the sidebar caller (WebShellSidebar.tsx, local min-w-0 + truncate span), and the tests pin the real behaviors (indicator opacity 1→0 on active-disappear, no pill in line variant, ref forwarding, resize-snaps vs mutation-animates). R1-6 (the React-18 ref drop on the sibling TabsTrigger) is pre-existing and reasonably recorded as a follow-up rather than widened into this diff.
上一轮 bot 的唯一 Critical（R1-1：min-w-0/overflow-hidden 泄漏到共享触发器基类）及其各 Suggestion 在此 head 上均已真正修复——截断策略现在只存在于侧栏调用方（WebShellSidebar.tsx，本地 min-w-0 + truncate span），测试也钉住了真实行为（激活消失时药丸 opacity 1→0、line 变体无药丸、ref 转发、resize 吸附 vs mutation 动画）。R1-6（兄弟 TabsTrigger 上的 React-18 ref 丢失）是既有问题，合理地记为后续项而非塞进本 diff。
Verdict / 结论
✅ APPROVE. No verified Critical or Important: the indicator hook is correct, observers are cleaned up, the animate/snap and disabled-dim semantics are right and tested, the prior Critical is fixed, and the change is well-scoped (the line variant and other tab lists are untouched). CI is green except the pending automated-review lane.
✅ APPROVE（批准）。 无经核实的 Critical 或 Important：指示器 hook 正确、observer 有清理、动画/吸附与禁用变暗语义正确且有测试、此前的 Critical 已修复，改动范围收得好（line 变体与其它标签列表未受影响）。CI 除 pending 的自动审查 lane 外全绿。
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — no blocking issues. Pill measurement runs in useLayoutEffect + observers (never during render), MutationObserver/ResizeObserver/rAF all cleaned up on unmount, active-tab-removal fades to opacity 0, and rapid switching collapses into one CSS transition via React 18 batching. Consistent with ytahdn's earlier approval; CI green on c555bc4.
5 participants
What this PR does
Tab lists in the Web Shell (the sidebar's Tasks/Channels session-source switch, plus the MCP/Plugins/Agents/Extensions managers, the skill install dialog, and the settings message) now move a single pill-shaped highlight to the newly activated tab with a 200ms slide, instead of recoloring each tab in place. The pill measures the active tab's fractional rect and glides between positions; it snaps without animating on first render, tracks sidebar drags 1:1 (resize-driven re-measures skip the transition), dims when the active tab is disabled, works for vertical lists, and disables the motion under
prefers-reduced-motion. The underline-stylelinevariant is untouched, and the tab list now forwards refs like the other interactive primitives. The sidebar's two triggers opt into shrink-to-equal halves with truncating labels so the switch stays symmetric without leaking that policy into other tab lists.Why it's needed
The session-source switch is one of the most frequent interactions in the sidebar, and the old in-place recolor made it hard to perceive which tab became active. A moving indicator gives immediate spatial feedback about the direction and target of the switch, matching what users expect from modern segmented controls.
Reviewer Test Plan
How to verify
Run the Web Shell, open a workspace session, and click between the Tasks and Channels tabs in the sidebar: the highlight should visibly slide across (about 0.2s) rather than fade in place. Drag the sidebar width — the pill tracks 1:1 with no easing lag. Other tabbed surfaces (Plugins, MCP, Agents managers) get the same slide while keeping their content-sized labels; the workflow-runs page keeps its underline style. Automated coverage:
client/e2e/web-shell.tabs-indicator.spec.ts(tagged@smoke) asserts the pill exactly overlays the active tab before and after a click, records intermediate positions during the transition to prove it slides, and asserts a position jump with no intermediate frames under reduced motion;components/ui/tabs.test.tsxcovers indicator presence/hide, ref forwarding, and the resize-vs-switch transition split.Evidence (Before & After)
Before: the active tab's background/shadow transitioned in place per trigger; no element moved. After: a sampled trace of the pill's left edge during a Tasks → Channels click shows a continuous ease-out glide (67.8 → 74.9 → 81.8 → … → 123.9 → 124px), landing exactly on the Channels tab's bounds; stills at rest are pixel-identical to the old active state since the pill carries the same background, border, and shadow tokens as the former active trigger. A maintainer re-ran this on a real daemon with transition pausing and confirmed the slide, reduced-motion snap, keyboard activation, and vertical orientation.
Tested on
Environment (optional)
npm run dev(vite) with the Playwright mock daemon and with a realqwen serve; Chromium.Risk & Scope
TabsTrigger(PluginManagerPage passes a focus-restore ref to a plain function component) is unchanged by this PR and worth a follow-up; nothing else was touched.TabsListno longer advertisesasChild(the two-child render it would receive cannot be slotted); no caller used it.Linked Issues
N/A
中文说明
本 PR 做了什么
Web Shell 中的标签列表（侧栏的 Tasks/Channels 会话来源切换，以及 MCP/Plugins/Agents/Extensions 管理页、技能安装对话框、设置消息）现在会把一个药丸形高亮块以约 200ms 的滑动动画移动到新激活的标签上，而不是在原地为每个标签换色。滑块按激活标签的小数像素矩形测量并在位置间平滑移动；首次渲染时直接到位不播放动画；拖动侧栏宽度时 1:1 跟随（resize 驱动的重测不走过渡）；激活标签被禁用时滑块同步变淡；纵向标签列表同样适用；在
prefers-reduced-motion下完全禁用动画。下划线样式的line变体不受影响；标签列表现在也和其他交互基础组件一样转发 ref。侧栏的两个标签单独选择"可收缩等分 + 文字截断"策略，保证切换开关对称，而不影响其他标签列表。为什么需要
会话来源切换是侧栏里最高频的交互之一，原来的原地换色让人难以感知哪个标签被激活了。移动的指示块能对切换的方向和目标给出即时的空间反馈，符合用户对现代分段控件的预期。
评审验证
运行 Web Shell，打开一个工作区会话，在侧栏 Tasks 和 Channels 标签间点击：高亮块应可见地滑动过去（约 0.2s）而不是原地渐变。拖动侧栏宽度，滑块 1:1 跟随无迟滞。其他带标签的页面（Plugins、MCP、Agents 管理页）获得同样的滑动，同时保持内容宽度的标签；workflow-runs 页面保持下划线样式。自动化覆盖：
client/e2e/web-shell.tabs-indicator.spec.ts（已打@smoke）断言点击前后滑块与激活标签精确重合、记录过渡期间的中间位置以证明确实在滑动，并验证 reduced-motion 下无中间帧的跳变；components/ui/tabs.test.tsx覆盖滑块渲染/隐藏、ref 转发、resize 与切换的过渡差异。证据（前后对比）
改动前：激活标签的背景/阴影在原地渐变，没有位移。改动后：Tasks → Channels 点击期间对滑块左缘采样得到连续 ease-out 轨迹（67.8 → 74.9 → 81.8 → … → 123.9 → 124px），终点与 Channels 标签边界精确重合；静止态与旧激活态逐像素一致，因为滑块沿用了原激活标签的背景、边框与阴影 token。维护者已在真实 daemon 上用暂停过渡的方式复核了滑动、reduced-motion 跳变、键盘激活与纵向排列。
测试平台
macOS ✅；Windows / Linux 未本地验证（由 CI 覆盖）。
风险与范围
TabsTrigger在 React 18 下既有的 ref 丢失问题（PluginManagerPage 给普通函数组件传了 focus 恢复用的 ref）与本 PR 无关、未做改动，值得后续跟进。TabsList不再声明asChild（两个子节点的渲染无法被 Slot 承接）；没有任何调用方使用过它。