fix(web-shell): keep mobile composer at chat pane bottom - #8601
Conversation
70af873 to
6ef0bfc
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
🖼️ 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 |
|
Thanks for the PR! Template looks good ✓ — the headings are abbreviated ("What" / "Why" / "Verification" / "Risk"), but every required section is covered in substance, including verification steps and what was tested where. Problem: observed, not theoretical. The NewSessionDotField stacking change added Direction: aligned — this repairs a layout regression in the Web Shell's empty-session mobile layout. CHANGELOG has no direct reference, but the area is squarely in scope. Size: not applicable — no core module paths touched ( Approach: the scope feels right. A 6-line CSS override scoped by three independent conditions ( Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 标题做了简写("What" / "Why" / "Verification" / "Risk"),但所有必需章节的内容都齐全,包括验证步骤和各平台的测试情况。 问题:是已观测到的回归,不是理论性问题。NewSessionDotField 的层级改动给 方向:对齐 —— 修复 Web Shell 空会话移动端布局回归,CHANGELOG 无直接引用,但该领域完全在范围内。 规模:不适用 —— 未触及核心模块路径(仅 方案:范围合理。6 行 CSS 覆盖,由三个独立条件限定( 风险:无升级风险信号 —— 改动文件均未命中 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code reviewBefore reading the diff, my independent take on the fix: the footer is Verified against the code:
No correctness, security, or convention issues found. Test evidence — the PR's own CIThis is an unattended CI run; I did not build or run the PR's code. Evidence below is the PR's own CI on the reviewed commit, fetched via API. The one red check is Classified as not caused by this PR, from the diff and check identity rather than log claims: this diff is CSS plus web-shell e2e tests, and nothing in The central claim is behavioural, and the PR's own CI largely settles it: the new Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Not verified: the author's manual browser pass was macOS-only (their claim, not independently re-run here; Windows and Linux untested by them). The behavioural claim itself does not rest on that pass — the pinned CI test above encodes it — but if a maintainer wants proof beyond the PR's own suite: 中文说明代码审查:在读 diff 之前,我的独立判断是——footer 是 已对照代码核实:选择器特异性相同但出现位置更靠后,覆盖只在目标场景生效;修饰类只在空会话 + 测试证据:本次为无人值守 CI 运行,未构建或执行 PR 代码,以上证据来自 PR 自身 CI(API 拉取)。唯一的红项是 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — a textbook regression repair: observed bug, identified root cause, smallest possible fix, and a regression test that provably fails without it and passes with it in CI. Stepping back: the PR's approach is exactly the one I'd have picked independently — a scoped The only red on this commit is the Approving, pinned to the reviewed commit. Thanks for the clean fix and the thorough test. 🚀 中文说明总体评价:这是一次教科书式的回归修复——问题真实观测到、根因明确、修复取最小改动、回归测试在没有修复时必然失败而在 CI 中通过。 回顾:PR 的方案与我的独立判断完全一致——局部 本提交上唯一的红项是单测套件中 批准,锚定到已审查的提交。感谢干净的修复和充分的测试。🚀 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.6)
6ef0bfc to
6a6323a
Compare
|
Thanks for flagging the coverage boundary. This state is intentionally covered by the dedicated Chromium regression test |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
已审查。 建议见行内评论。 未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.21.6)
99d571a to
2a13238
Compare
|
The visual workflow does not enter the opt-in |
|
The updated visual run and all CI checks are green. The screenshot suite still does not enter the opt-in |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
已审查。 建议见行内评论。 未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
Automatic review follow-up is complete in c90e4e0. All five suggestions were addressed in the E2E harness/specs: containing-block ownership, direct flex-item structure, dot-field stacking order, shared assertion reuse, and the footerless public-prop combination. Validation: 3 focused Playwright tests passed; formatting, lint, typecheck, and an independent review passed; the WebShell suite passed 2825/2825. Full preflight still reproduces 17 unrelated CLI AuthDialog/serve baseline failures. |
|
The refreshed visual workflow is green, and the unchanged screenshot is expected because the existing visual scenarios do not enter the opt-in empty mobile welcome state. The focused Chromium regression coverage now directly pins the 760/761 breakpoint, footer and dot-field offset parents, direct flex-item structure, relative stacking order, geometry, interaction, both composer backends, and the footerless selector case. WebShell unit tests passed 2825/2825 and the WebShell E2E smoke check passed. A dedicated screenshot scenario would therefore be supplementary coverage outside this focused fix, so I am keeping it out of this PR. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.6)
51bf0d5 to
7a82f56
Compare
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.6)
7a82f56 to
6d10bd7
Compare
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review round summary — PR #8601Round addressed: the round-6 Feedback points and dispositionsrc:3732722703 — [Critical] R6-2: custom-footer + welcome-middle variant clips
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const style = getComputedStyle(ancestor); | ||
| if (style.position === 'absolute' || style.position === 'relative') { | ||
| footer = ancestor; | ||
| break; |
There was a problem hiding this comment.
[Suggestion] R7-1: The footer-discovery ancestor walk selects the .footerWithCustomFooter element in the welcome-middle + custom-footer variant, but that element is display: contents there (pre-existing rule .appChatEmpty .chatViewWithWelcomeMiddle .footerWithCustomFooter), so it generates no box — getBoundingClientRect() returns a zero rect and offsetParent is null. Probe-verified in live Chromium at 760×900 against this commit: after gotoEmptyMobileWelcomeHarness(page, { customFooter: true }), emptyMobileComposerLayout(page) returns footerBottom: 0 vs chatPaneBottom: 900. — Failure scenario: a maintainer extends this suite by combining the customFooter: true harness option this PR ships with emptyMobileComposerLayout(page) → the walk picks the box-less footer div (it still computes position: absolute), and expectEmptyMobileComposerAnchored fails with a false "footer not anchored to the chat pane bottom" error that sends the debugger after CSS that is not broken. Note: this variant's layout expectations differ by design (the :not() exclusion keeps the wrap positioned and the footer display: contents), so even with honest measurements the anchored assertions would fail for it — documenting/asserting that this helper is invalid for customFooter: true is the effective remedy; the box-skip below only changes the failure mode.
| const style = getComputedStyle(ancestor); | |
| if (style.position === 'absolute' || style.position === 'relative') { | |
| footer = ancestor; | |
| break; | |
| const style = getComputedStyle(ancestor); | |
| if ( | |
| (style.position === 'absolute' || style.position === 'relative') && | |
| ancestor.getClientRects().length > 0 | |
| ) { | |
| footer = ancestor; | |
| break; |
中文说明
footer 发现用的祖先遍历在 welcome-middle + 自定义 footer 组合下会选中 .footerWithCustomFooter 元素,但该元素在此组合下是 display: contents(既有规则 .appChatEmpty .chatViewWithWelcomeMiddle .footerWithCustomFooter),不生成盒子——getBoundingClientRect() 返回零矩形,offsetParent 为 null。已在本提交上用实时 Chromium(760×900)探测验证:gotoEmptyMobileWelcomeHarness(page, { customFooter: true }) 之后调用 emptyMobileComposerLayout(page) 返回 footerBottom: 0,而 chatPaneBottom: 900。— 失败场景:维护者扩展本套件时,将本 PR 提供的 customFooter: true harness 选项与 emptyMobileComposerLayout(page) 组合使用 → 遍历选中无盒子的 footer div(其计算样式仍为 position: absolute),expectEmptyMobileComposerAnchored 以误报的 "footer 未锚定到聊天面板底部" 失败,让排查者去追并没有问题的 CSS。注意:该变体的布局预期本就不同(:not() 排除使 wrap 保持定位、footer 为 display: contents),即使测量值正确,锚定断言在该变体下也会失败——因此有效的修复是声明/断言本帮助函数对 customFooter: true 无效;下方的跳过无盒子元素改动只是改变失败形式。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31149884144 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round summaryAddressed the single inline finding from the automated review (R7-1) on PR #8601. One commit: Feedback pointsR7-1 — [Suggestion] Footer-discovery walk selects the box-less
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| await gotoEmptyMobileWelcomeHarness(page); | ||
| const textarea = page.locator(COMPOSER_TEXTAREA); | ||
| const composer = page.locator(emptyMobileComposerSelectors.composerSurface); |
There was a problem hiding this comment.
[Suggestion] R8-1: The welcome-chrome setup block — four locator declarations off emptyMobileComposerSelectors, the four visibility assertions, and the emptyMobileComposerLayout + expectEmptyMobileComposerAnchored calls — is pasted near-verbatim into this spec and web-shell.smoke.spec.ts (three test bodies in two files; the without-a-welcome-footer variant has already forked deliberately), even though this PR introduces utils/emptyMobileComposer.ts specifically to own this scenario's shared code. — Concrete cost: any future change to the mobile welcome chrome (dot-field canvas child, welcome-footer element restructure, a new chrome element) forces lock-step edits in three test bodies across two files instead of one helper.
Suggested fix (spans both spec files, so no one-click block): fold the locator creation + visibility waits into the utility, e.g. export async function expectEmptyMobileWelcomeChromeVisible(page: Page) in utils/emptyMobileComposer.ts, and call it from all three tests (the per-test interaction tails legitimately stay separate).
中文说明
R8-1:welcome 外观搭建块——基于 emptyMobileComposerSelectors 的四个 locator 声明、四个可见性断言,以及 emptyMobileComposerLayout + expectEmptyMobileComposerAnchored 调用——被近乎原样地粘贴进本文件和 web-shell.smoke.spec.ts(两个文件共三处测试主体;其中"无 welcome footer"变体已有意识地分叉),而本 PR 恰恰引入了 utils/emptyMobileComposer.ts 来承载该场景的共享代码。— 具体代价:未来对移动端 welcome 外观的任何改动(dot field 的 canvas 子元素、welcome footer 元素重构、新增外观元素)都将迫使三处测试主体跨两个文件同步修改,而不是只改一个帮助函数。
建议修复(跨两个 spec 文件,故不使用一键应用块):将 locator 创建 + 可见性等待折叠进工具函数,例如在 utils/emptyMobileComposer.ts 中增加 export async function expectEmptyMobileWelcomeChromeVisible(page: Page),并在三个测试中调用(各测试的交互尾部保持独立)。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| npx playwright test --config packages/web-shell/playwright.config.ts packages/web-shell/client/e2e/web-shell.smoke.spec.ts --grep 'anchors the empty mobile composer' | ||
| ``` | ||
|
|
||
| After the repair, the focused upstream run passed (`1 passed`, 4.5s). The |
There was a problem hiding this comment.
[Suggestion] R8-2: This evidence line claims the focused grep command passes with 1 passed, but the PR's final state has two tests in web-shell.smoke.spec.ts matching --grep 'anchors the empty mobile composer' (line 468 and line 527). Probe-verified in this worktree with npx playwright test ... --grep 'anchors the empty mobile composer' --list → Total: 2 tests in 1 file. — Failure scenario: a reviewer re-running the documented evidence command gets 2 passed where the doc says 1 passed, making the verification evidence unverifiable as written.
| After the repair, the focused upstream run passed (`1 passed`, 4.5s). The | |
| After the repair, the focused upstream run passed (`2 passed`). The |
中文说明
R8-2:此证据行声称聚焦 grep 命令以 1 passed 通过,但 PR 的最终状态下 web-shell.smoke.spec.ts 中有两个测试匹配 --grep 'anchors the empty mobile composer'(第 468 行与第 527 行)。已在本工作树用 npx playwright test ... --grep 'anchors the empty mobile composer' --list 探测验证 → Total: 2 tests in 1 file。— 失败场景:评审者重新运行文档中的证据命令时会得到 2 passed,与文档所写的 1 passed 不符,导致验证证据按原文无法复现。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| .appChatEmpty | ||
| .chatViewWithMobileComposerBottom:not( | ||
| .chatViewWithWelcomeMiddle.chatViewWithCustomFooter | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R8-3: One branch of this new selector is never exercised: chatViewWithCustomFooter present with chatViewWithWelcomeMiddle absent (custom footer but no welcome footer). That state is reachable — hasMobileComposerBottom does not depend on welcomeFooter, so a host passing mobileWelcomeFooterMiddle + renderFooter without renderWelcomeFooter on a ≤760px empty session gets exactly this class combination, and the position: static branch applies. No test pins that layout. — Failure scenario: rewriting the exclusion to :not(.chatViewWithCustomFooter) leaves all four new tests green, but in this combination the wrap would stay position: relative, so the absolute composer footer anchors to the centered, content-height wrap instead of the pane — the composer floats mid-pane, the same defect class (the 419px gap) this PR repairs.
Suggested fix (a new test, not an edit here): add one smoke test calling gotoEmptyMobileWelcomeHarness(page, { customFooter: true, welcomeFooter: false }) and asserting expectEmptyMobileComposerAnchored(layout, { requireWelcomeFooter: false }); the harness and helper already support the combination.
中文说明
R8-3:这条新选择器有一个分支从未被测试覆盖:存在 chatViewWithCustomFooter 但没有 chatViewWithWelcomeMiddle(有自定义 footer 但无 welcome footer)。该状态可达——hasMobileComposerBottom 不依赖 welcomeFooter,因此宿主在 ≤760px 的空会话中传入 mobileWelcomeFooterMiddle + renderFooter 而不传 renderWelcomeFooter 时,恰好得到这一类组合,且会走 position: static 分支。目前没有任何测试钉住该布局。— 失败场景:把排除条件改写为 :not(.chatViewWithCustomFooter) 后,四个新测试仍然全绿,但在该组合下 wrap 会保持 position: relative,绝对定位的 composer footer 将锚定到居中的、内容高度的 wrap 而非聊天面板——输入框悬浮在面板中部,与本 PR 修复的缺陷同类(419px 间距)。
建议修复(新增测试,而非修改此处):增加一个冒烟测试,调用 gotoEmptyMobileWelcomeHarness(page, { customFooter: true, welcomeFooter: false }) 并断言 expectEmptyMobileComposerAnchored(layout, { requireWelcomeFooter: false });harness 与帮助函数已支持该组合。
— qwen3.8-max via Qwen Code /review (v0.21.7)
…oter branch Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Address review summary — PR #8601 (round 8 suggestions)All three inline suggestions from the automated reviewer were addressed in commit Feedback points and dispositionsR8-1 — Duplicated welcome-chrome setup block (
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| hasWelcomeMiddle | ||
| ? styles.chatViewWithWelcomeMiddle | ||
| : undefined, | ||
| CustomFooter ? styles.chatViewWithCustomFooter : undefined, |
There was a problem hiding this comment.
[Suggestion] chatViewWithCustomFooter is a declaration-less marker class whose only purpose is to be matched by the :not(.chatViewWithWelcomeMiddle.chatViewWithCustomFooter) exclusion in App.module.css — and nothing at this call site documents that it is load-bearing. — Concrete cost: a future cleanup that removes or renames this reference (it styles nothing, so it looks dead), or renames the :not() argument in the CSS, silently breaks the exclusion: in the welcome-middle + custom-footer mobile variant the wrap flips to position: static, .bottomPanels (whose parent .footerWithCustomFooter is display: contents and provides no containing block) re-anchors to the chat pane, and its bottom: calc(100% + 6px) places the bottom status panel entirely above the pane's top edge, where overflow: hidden clips it — the status items silently vanish. The existing smoke test catches this after the fact; a one-line comment prevents it.
| CustomFooter ? styles.chatViewWithCustomFooter : undefined, | |
| // Marker class (no declarations): keeps the ':not(...)' exclusion in App.module.css matching. | |
| CustomFooter ? styles.chatViewWithCustomFooter : undefined, |
中文说明
chatViewWithCustomFooter 是一个不含任何 CSS 声明的标记类,其唯一作用是匹配 App.module.css 中的 :not(.chatViewWithWelcomeMiddle.chatViewWithCustomFooter) 排除条件——而此调用处没有任何注释说明它是承重的。— 具体代价:未来的清理如果删除或重命名这个引用(它不产生任何样式,看起来像死代码),或重命名 CSS 中 :not() 的参数,都会悄悄破坏该排除条件:在 welcome-middle + 自定义 footer 的移动端变体中,wrap 会变为 position: static,.bottomPanels(其父级 .footerWithCustomFooter 为 display: contents,不提供包含块)将重新锚定到聊天面板,其 bottom: calc(100% + 6px) 会把底部状态面板整个定位到面板顶部上方,被 overflow: hidden 裁剪——状态项会无声消失。现有冒烟测试能在事后捕获该回归;加一行注释即可防患于未然。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #8601Feedback points
No other feedback was in scope this round (no issue-level comments, no failed or still-red checks). No conflict resolution was needed ( Changed files
Verification
中文说明Autofix 审查轮次总结 — PR #8601反馈点
本轮没有其他需要处理的反馈(无 issue 级评论,无失败或持续失败的检查)。无需解决冲突( 变更文件
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max via Qwen Code /review (v0.21.7)
Verification report — independently reproduced, recommend merge ✅I built a real verification environment for this PR and reproduced the defect, the fix, and the boundaries of the fix from scratch. The fix is correct, minimal, and the added tests genuinely guard it. One unrelated pre-existing flake surfaced (details at the end). Environment
Before A/B-ing I confirmed I also independently confirmed the root cause you name: What I confirmed
1. The regression and the fixMeasured with my own instrumentation (not the PR's assertions), at 760×900:
That last row is worth calling out: on base the composer doesn't merely sit in the wrong place, it paints over the welcome header. The PR restores it.
2. Mutation test — is the
|
| scenario | metric | base | PR |
|---|---|---|---|
| 761px control | wrap / footer position | relative / relative |
relative / relative |
| footer bottom, gap | 617, 283px | 617, 283px | |
| custom-footer welcome variant | wrap position | relative |
relative |
status panel y |
226.703125 | 226.703125 |
5. Regression sweep
| Command | Result |
|---|---|
npm run typecheck -w packages/web-shell |
✅ pass |
eslint on all 5 changed TS/TSX files |
✅ clean |
prettier --check on all 7 changed files |
✅ clean |
npm run build -w packages/web-shell |
✅ pass |
npm run test -w packages/web-shell |
✅ 171 files / 2860 tests passed |
| Full Playwright e2e (46 tests, both projects) | 45 passed, 1 failed — pre-existing flake, see below |
Note on your PR description: your run reported
171 files / 2858 tests; I get171 / 2860, which is just your 2 new spec cases plus the merge from main. Theverifyformat-check failures you mention on untouched baseline files did not reproduce here —prettier --checkis clean across all seven changed files.
6. Unrelated finding — a flaky smoke test on main
grows long text to the responsive composer cap at 600px @smoke failed in my full run. It is not caused by this PR — I A/B-ed the flake itself:
| branch | outcome |
|---|---|
| PR head | 1 pass / 2 fail out of 3 runs |
origin/main (production files reverted) |
1 pass / 3 fail out of 4 runs |
It fails at web-shell.smoke.spec.ts:644, where Escape is expected to dismiss the composer history search:
Error: expect(locator).toHaveCount(expected) failed
Locator: locator('[data-web-shell-composer-surface]').locator('input')
Expected: 0 Received: 1
Nothing to do with empty-session layout. Probably worth its own issue.
Repro
git fetch origin pull/8601/head:pr-8601 && git worktree add /tmp/pr8601 pr-8601
cd /tmp/pr8601 && npm ci
cd packages/web-shell
# PR head → 5 passed
npx playwright test --config playwright.config.ts \
--grep "empty mobile composer|custom footer mobile welcome variant"
# base A/B → 4 failed (419 / 429.5 / 409 px gaps)
git -C /tmp/pr8601 checkout origin/main -- \
packages/web-shell/client/App.module.css packages/web-shell/client/App.tsx
npx playwright test --config playwright.config.ts \
--grep "empty mobile composer|custom footer mobile welcome variant"Non-blocking notes
- The marker class is invisible to the type system.
styles.chatViewWithCustomFooterresolves only because the name appears inside the CSS:not(). If someone later edits or renames that clause, the class silently becomesundefinedand vanishes from the DOM — no TS error, no build error. Your comment inApp.tsx(936e156) plus smoke test 如何自定义密钥文件 .env可能与其他文件冲突 #3 cover this behaviourally, which I think is adequate; just worth knowing it's a behavioural guard rather than a structural one. :not(.a.b)with a compound argument is Selectors Level 4 (Chrome/Edge 88+, Firefox 84+, Safari 9+). I confirmed the build does not downlevel it. On a browser that lacked it the whole rule would be dropped and you'd land back on today's behaviour — degrades to the current bug, never to something worse.- The harness is dev-server-only by construction (
await import(/* @vite-ignore */ '../index.tsx'), and it isn't a build input), so a production-bundle E2E of this path isn't possible. I closed that gap statically instead — see §3.
I verified your claim that no CLI baseline applies rather than taking it on faith: mobileWelcomeFooterMiddle appears nowhere in the repo outside the App.tsx prop definition and the e2e harness, so no in-repo caller can put the standalone qwen serve UI into this layout. The harness really is the only executable path, and the doc is right to say so.
Verdict: LGTM. Correct root cause, minimal production change (8 CSS lines + 1 marker class), the exclusion is justified, and the new tests fail on base and fail under mutation — they are real guards, not decoration.
中文版(合并参考)
验证报告 — 已独立复现,建议合并 ✅
我从零搭建了真实验证环境,独立复现了缺陷、修复效果以及修复的边界。修复正确、改动最小,新增测试是真正有效的护栏。 另外发现一个与本 PR 无关的既有 flaky 测试(见文末)。
环境
| 工作区 | 全新检出 936e156b06 + 真实 npm ci(2048 个包) |
| A/B 基线 | origin/main @ 20b9504276 |
| 运行器 | Playwright 1.61.1 / Chromium 1228,Linux |
| 被测对象 | Vite dev server,composer-layout-harness.html |
做 A/B 之前我先确认了 git diff origin/main HEAD -- App.module.css App.tsx 恰好等于本 PR 的两处生产改动、没有任何漂移,所以「只回退这两个文件、保留 PR 的测试」是真实基线而非近似。
我也独立确认了你指出的根因:git show e379ea4e53(#8098)正是给 .appChatEmpty .chatViewWrap 加上 position: relative; z-index: 1 的那次提交。
确认结论
| # | 检查项 | 结果 |
|---|---|---|
| 1 | 5 个新测试在 PR 上通过 | ✅ 5/5 |
| 2 | 回退生产文件后同样的测试在基线上失败 | ✅ 4 个失败,间距 419 / 429.5 / 409 px |
| 3 | 基线上仍通过的那一个正是被有意排除的分支 | ✅ 符合设计 |
| 4 | 宽屏(761px)不受影响 | ✅ 基线与 PR 测量值完全一致 |
| 5 | 自定义 footer 分支不受影响 | ✅ 基线与 PR 测量值完全一致 |
| 6 | :not(...) 排除项是必需的,不是过度设计 |
✅ 变异测试已证明 |
| 7 | 标记类在生产构建中依然有效 | ✅ 在压缩后的 CSS 与 JS 类名映射中均已核实 |
| 8 | e2e 文档中「CLI 基线不适用」的说法 | ✅ 属实(已核实,非假定) |
1. 回归与修复
在 760×900 下用我自己的埋点(而非 PR 的断言)测得:
| 指标 | 基线(origin/main) |
PR #8601 |
|---|---|---|
chatViewWrap 计算 position |
relative |
static |
footer 计算 position |
absolute |
absolute |
| footer 底边 / 聊天区底边 | 481 / 900 | 900 / 900 |
| 距聊天区底部间距 | 419px | 0px |
欢迎标题中心处 elementFromPoint |
输入框(标题被遮挡) | 欢迎标题 |
最后一行值得强调:基线上输入框不只是位置不对,而是直接盖住了欢迎标题,本 PR 恢复了正常。
position: static 下 z-index: 1 依然生效是这个修复中最微妙的一点,理由与你注释所写一致 —— 该 wrap 是 .chatPane 的 flex item,而 z-index 对 flex item 生效与是否定位无关。运行时已确认:计算出的 z-index 仍为 1,chatViewIsPaneFlexItem 为 true,dot field 仍在下层且 pointer-events: none。
2. 变异测试 —— :not(...) 排除项是否必要
复合形式的 :not() 属于评审时应当质疑的写法,所以我验证了它是否真的在起作用:把选择器替换成朴素写法 .appChatEmpty .chatViewWithMobileComposerBottom { position: static; }。
结果底部状态面板被推到 y = -31.8 —— 跑到聊天区顶部之上,被 overflow: hidden 完全裁掉。这正是你代码注释预测的失效路径:.footerWithCustomFooter 为 display: contents 时没有 footer 盒子,.bottomPanels(bottom: calc(100% + 6px))会向上找到最近的定位祖先。
而且 PR 自带的测试正好抓住了它(这是我最想看到的一点):
✘ keeps the bottom status panel visible in the custom footer mobile welcome variant @smoke
Expected: >= 0
Received: -31.796875
这份复杂度是值得的,并且有测试守住。
3. 标记类确实生效 —— 包括生产包
styles.chatViewWithCustomFooter 自身没有任何声明,它能存在仅仅因为该名字出现在 :not() 里。这种写法足够特殊,值得核实而非默认可靠。两个层面都成立:
- 开发态 —— 实时 DOM 类名:
… _chatViewWithWelcomeMiddle_1hqst_918 _chatViewWithCustomFooter_1hqst_918 - 生产
vite build—— 压缩器完整保留了选择器,且与 JS 类名映射哈希一致:
._appChatEmpty_1hqst_49 ._chatViewWithMobileComposerBottom_1hqst_917:not(._chatViewWithWelcomeMiddle_1hqst_918._chatViewWithCustomFooter_1hqst_918){position:static}上面的变异实验同时充当了反向对照:移除 :not() 后,该类名从 DOM 中静默消失。
4. 无附带影响
宽屏对照与被排除分支在基线和 PR 上测量值完全一致,唯一的行为差异就是预期中的那一处。
| 场景 | 指标 | 基线 | PR |
|---|---|---|---|
| 761px 对照 | wrap / footer position | relative / relative |
relative / relative |
| footer 底边、间距 | 617、283px | 617、283px | |
| 自定义 footer 分支 | wrap position | relative |
relative |
状态面板 y |
226.703125 | 226.703125 |
5. 回归扫描
| 命令 | 结果 |
|---|---|
npm run typecheck -w packages/web-shell |
✅ 通过 |
5 个改动 TS/TSX 文件的 eslint |
✅ 无告警 |
7 个改动文件的 prettier --check |
✅ 无问题 |
npm run build -w packages/web-shell |
✅ 通过 |
npm run test -w packages/web-shell |
✅ 171 文件 / 2860 用例全部通过 |
| 完整 Playwright e2e(46 用例,两个 project) | 45 通过,1 失败 —— 既有 flaky,见下 |
关于 PR 描述的两点补充:你记录的是
171 files / 2858 tests,我这里是171 / 2860,差异就是你新增的 2 个用例加上合并 main 带来的变化。你提到的verify中 5 个基线文件的格式检查失败在我这里没有复现 —— 7 个改动文件的prettier --check全部干净。
6. 无关发现 —— main 上的一个 flaky 测试
完整 e2e 中 grows long text to the responsive composer cap at 600px @smoke 失败了。与本 PR 无关,我对这个 flaky 本身也做了 A/B:
| 分支 | 结果 |
|---|---|
| PR head | 3 次中 1 通过 / 2 失败 |
origin/main(回退生产文件) |
4 次中 1 通过 / 3 失败 |
失败点在 web-shell.smoke.spec.ts:644,即 Escape 应当关闭输入框历史搜索之处,与空会话布局无关,建议单独开 issue 跟踪。
遗留说明(均不阻塞合并)
- 标记类对类型系统不可见。
styles.chatViewWithCustomFooter能解析出来完全依赖 CSS:not()中出现了该名字。若日后有人改动或重命名该子句,这个类会静默变成undefined并从 DOM 消失,既没有 TS 报错也没有构建报错。你在App.tsx里的注释(936e156b06)加上 smoke 测试 如何自定义密钥文件 .env可能与其他文件冲突 #3 已经从行为层面覆盖了,我认为够用,只是需要知道这是行为护栏而非结构护栏。 - 复合参数的
:not(.a.b)属于 Selectors Level 4(Chrome/Edge 88+、Firefox 84+、Safari 9+)。我确认构建不会对其降级。即便某浏览器不支持,整条规则会被丢弃、退回到当前行为 —— 只会退化成现有 bug,不会更糟。 - 该 harness 在构造上只能跑开发服务器(
await import(/* @vite-ignore */ '../index.tsx'),且不是构建入口),因此无法对这条路径做生产包 E2E。我改用静态方式补上了这一环,见第 3 节。
关于「CLI 基线不适用」这一点我做了核实而非采信:mobileWelcomeFooterMiddle 在整个仓库中除了 App.tsx 的 prop 定义和 e2e harness 之外没有任何调用方,因此独立的 qwen serve UI 无法进入该布局。harness 确实是唯一可执行路径,文档的说法是准确的。
结论:LGTM。 根因判断正确,生产改动最小(8 行 CSS + 1 个标记类),排除项有充分理由,新增测试在基线上会失败、在变异下也会失败 —— 是真正有效的护栏,而非摆设。
|
Released in v0.21.8. |
… panels Dropping the #8601 override removed the positioned containing block for the bottom panels (status panel) in the custom-footer mobile welcome variant: that variant renders the composer footer display: contents, so its wrap is the only positioned ancestor the absolutely-positioned bottom panels can anchor to. Restore position: relative on just that variant instead of the full-positioned wrap the deleted rule had excluded, keeping the plain variants static so the footer keeps anchoring to the chat pane.
… opacity (QwenLM#9349) * Revert "fix(web-shell): stabilize mobile composer after resume (QwenLM#8263)" This reverts commit d213c85. # Conflicts: # packages/web-shell/client/e2e/web-shell.composer.mobile.spec.ts * Revert "feat(web-shell): enhance composer and empty-session animations (QwenLM#8098)" This reverts commit e379ea4. * fix(web-shell): drop QwenLM#8601 mobile-composer overrides superseded by the animation revert The QwenLM#8098 animation revert removes the position: relative / z-index: 1 on .appChatEmpty .chatViewWrap, so QwenLM#8601's position: static override (a fix for the containing-block shift that QwenLM#8098 introduced) is a no-op and its comment describes a dot field that no longer exists. Sync the e2e layout assertions to the restored pre-QwenLM#8098 values (z-index auto, static wrap at 761px) while keeping the footer-anchoring contract they verify. * style(web-shell): cap the dac composer glow at 50% opacity Design feedback: the restored composer glow reads too bright. Halve the opacity ceiling for the aura grid, the halo and the pulse glow while keeping the dac-aura-cols dance animation and the focus/pulse dynamics intact. * fix(web-shell): keep the custom-footer wrap positioned for the bottom panels Dropping the QwenLM#8601 override removed the positioned containing block for the bottom panels (status panel) in the custom-footer mobile welcome variant: that variant renders the composer footer display: contents, so its wrap is the only positioned ancestor the absolutely-positioned bottom panels can anchor to. Restore position: relative on just that variant instead of the full-positioned wrap the deleted rule had excluded, keeping the plain variants static so the footer keeps anchoring to the chat pane. * fix(web-shell): address review feedback on the animation revert - Restore disabled?: boolean on the ChatEditor test render props: two surviving call sites still pass disabled: true, and dropping the member leaves object-literal excess-property violations (latent only while test files stay out of tsc). - Update the stale chatViewWithCustomFooter marker comment in App.tsx: the :not(...) exclusion it described was removed, and the class now completes the compound selector that keeps the custom-footer wrap positioned for the bottom panels. - Document the once-only glow effect invariant and the legacy data-dac-glow marker, both restored verbatim from the pre-QwenLM#8098 implementation. --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>


What
Keep the empty-session mobile composer anchored to the bottom of the chat pane when
mobileWelcomeFooterMiddleandchatViewWithMobileComposerBottomare enabled at widths up to 760px.This change:
position: staticfor the mobile composer-bottomchatViewWrap;z-index, so welcome content remains aboveNewSessionDotField;Why
The
NewSessionDotFieldstacking change made.appChatEmpty .chatViewWrappositioned. That unintentionally changed the absolutely positioned footer's containing block fromchatPaneto the centered welcome-content wrapper, moving the composer into the middle of a narrow empty session.Verification
npm run test --workspace=packages/web-shell— 171 files / 2858 tests passednpm run typecheck --workspace=packages/web-shell— passednpm run build --workspace=packages/web-shell— passedThe repository-wide preflight completed formatting, lint, build, and typecheck, then failed in unrelated full-repository CLI tests (1 suite and 14 tests, including
AuthDialogand temporary-directory cleanup timing). All WebShell suites passed. The scoped WebShellverifycommand also passed lint; its format check remains blocked by five untouched baseline files.Tested on macOS. Windows and Linux were not tested locally.
Risk
Low. The CSS override is scoped to empty sessions, composer-bottom mobile layout, and the existing
max-width: 760pxmedia query. Wider layouts and non-empty sessions are unchanged. The final PR diff is 356 insertions and 1 deletion across six files; production CSS is eight added lines.中文说明
展开查看
修改内容
在宽度不超过 760px,且启用
mobileWelcomeFooterMiddle与chatViewWithMobileComposerBottom时,让空会话输入框继续固定在聊天区域底部。chatViewWrap恢复为position: static;z-index,确保欢迎内容仍显示在NewSessionDotField上方;根因
NewSessionDotField的层级改动给.appChatEmpty .chatViewWrap增加了定位属性,使绝对定位输入框的 containing block 从整个chatPane变成居中的欢迎内容容器,因此输入框移动到了欢迎内容中间。范围审计
本轮撤销了内部审计阶段扩展出的 Grid 布局、自定义 footer、短屏滚动与隐藏聊天场景,保留原始小范围 CSS 修复和直接对应评审意见的防回归测试。最终相对主干为 6 个文件、356 行新增、1 行删除,其中生产 CSS 仅新增 8 行。
风险
较低。修复只作用于空会话、移动端 composer-bottom 布局和现有的
max-width: 760px媒体查询;宽屏及非空会话不受影响。Linked issues
N/A