Repository navigation
fix(web-shell): inset docked right panel below the macOS titlebar drag region - #12876
Conversation
…g region The docked right panel portals into a display:contents slot, so it is a direct .appShell child and never received the 38px --web-shell-desktop-titlebar-height inset that .contextShell and the sidebar get. On the macOS Desktop shell the fixed titlebar drag region (z-index 40, covering y 0-38px) sat on top of the panel toolbar's toggle button, so once the panel was open and the header toggle had handed over (by design), the replacement button could not be clicked and the panel could not be closed. Apply the same padding-top inset to .artifactPanelDock and add an e2e regression spec covering both the plain-browser hand-over path and the macOS-flag occlusion path. Fixes #12874 Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-issue-patrol/jmuk6uemf26
The Lint & Static gate failed on PR #12876 because the newly added e2e spec was not Prettier-formatted. No behavior change. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-issue-patrol/jmukb4q6028
|
Answering the I verified the lane facts rather than taking the description's word for it:
Why not just tag it:
So the honest statement of what this PR contributes: the four CSS lines are the fix, and the spec is a nightly guard against someone silently undoing the inset — it has teeth when it runs (the macOS occlusion assertion fails on The description has been updated to say this plainly instead of implying PR-level gating. The Prettier blocker was fixed in 中文说明按你的要求给一个明确的决定，而不是默认放过：这条 spec 有意不打 我自己核过链路事实，没有照抄描述：
为什么不直接打标签：
所以本 PR 贡献的诚实表述是：那四行 CSS 是修复，spec 是夜间防止有人悄悄撤掉内缩的保护——它跑起来是有牙的（macOS 遮挡断言在 描述已更新，明确写出这一点，而不再暗示 PR 级门禁。 Prettier 阻塞项已在 |
|
@qwen-code /triage |
|
@qwen-code /triage |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical-only review at d6ef63e0 — approving
Base 3f5ae3ff. Two files, +96/-0 and nothing deleted: four lines of CSS in packages/web-shell/client/App.module.css and a new 92-line Playwright spec. I read the changed rule, the variable it consumes, and the surrounding flex context.
Historical blocking issue — resolved
The one blocker ever filed was mechanical: Lint & Static failed at the Prettier step on the spec file this PR adds, so it could not be pre-existing noise. At this head Lint & Static passes, along with Test, web-shell E2E Smoke, Capture web-shell visuals and Integration Tests (no-AK, No Sandbox) — twelve green, zero failures, nothing pending. The bot has also replaced its CHANGES_REQUESTED with an approval at this head, and there are no review threads at all.
My scan
The inset cannot reach a non-macOS context. The declaration is padding-top: var(--web-shell-desktop-titlebar-height, 0), and that custom property is defined in exactly one place:
.app:global(.qwen-code-macos-titlebar) {
--web-shell-desktop-titlebar-height: 38px;
}So it resolves to 38px only when the root .app element also carries the global class the Tauri shell sets on macOS, and falls back to 0 everywhere else — a plain browser, and the Windows and Linux desktop shells. The :global() compound lands on the same element as .app, which is the thing that makes the scoping actually hold. This is also not a new mechanism: .contextShell at :91 consumes the identical expression, so the dock now matches the shell and the sidebar rather than introducing a second way to express the inset.
No conflicting or duplicated declaration. .artifactPanelDock had no padding-top before this change; the rule reads flex, width, min-width, display, the new declaration, then animation.
The box model and the horizontal layout are untouched. The dock is a horizontal flex item (flex: 0 0 var(--artifact-panel-dock-width) with an explicit width), and the new padding is on the vertical axis only, so the panel's width — and with it the resize divider's MIN_ARTIFACT_PANEL_WIDTH clamping — behaves exactly as before. .artifactPanelClip inside is flex: 1 1 auto with overflow: hidden, so it absorbs the reduced content height instead of overflowing. Under the package's universal border-box the padding cannot inflate the element's outer size, and the open animation keyframes only flex-basis and width, so the inset does not participate in the transition.
The fullscreen variant is correctly out of scope. It renders through a portal host at z-index: 1000 against the drag strip's 40, so it was never occluded and this rule does not apply to it.
No Critical found. The change adds a padding declaration gated on a macOS-only custom property, plus a test.
Non-blocking, recorded rather than requested
The bot's open question is a real one and worth a deliberate answer: the new spec carries no @smoke tag, and PR CI runs the e2e lane with --grep @smoke, while the full suite is post-merge and nightly. So this regression guard will not gate a pull request today — a future change that moves the panel back out of the padded shell would be caught after merge rather than before. Adding it to smoke is a budget decision for whoever owns that job's runtime, not something I would push for in this PR.
CI
Twelve checks pass at this head with zero failures and nothing pending. Capture web-shell visuals passing is useful corroboration here specifically, since it is the lane that would show an unintended inset leaking into another layout.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
The CSS one-liner mirrors the existing .contextShell pattern exactly: the variable is set to 38px on .app.qwen-code-macos-titlebar and falls back to 0 everywhere else, so non-macOS and web-only deployments are unaffected.
What was checked
- Stated intent vs. code (Class 10): The patch comment explains the portal mechanics correctly —
.artifactPanelDockportals into adisplay:contentsslot so it becomes a direct.appShellflex child and misses the.contextShellpadding-top. Adding the same variable to.artifactPanelDockis the minimal, correct fix. - Variable definition:
--web-shell-desktop-titlebar-height: 38pxis declared on.app:global(.qwen-code-macos-titlebar)in the full CSS file; the0fallback keeps non-macOS paths unaffected. - Sibling entrances:
.artifactResizeHandleis also a direct.appShellchild but is a narrow 8 px drag rail with no interactive UI near its top edge — not a blocker, worth a follow-up only if that grip ever gains a hit area near the titlebar. - E2E test validity (Class 5): Both tests are well-formed. The occlusion test uses
document.elementFromPointat the button center plus an actualpanelToggle.click()— Playwright's hit-target retry loop would keep spinning while the drag region intercepted pointer events, so the click robustly regresses the bug beyond the point-query alone. - No AI review ban found in CONTRIBUTING.md.
- Idempotency: No prior
chiga0review on this PR.
No blockers found.
Reviewed with AI assistance.
3 participants
What this PR does
On the macOS Desktop shell, the docked right panel (file changes / side tasks / web preview / trajectory / terminal) could not be closed once opened: the header toggle unmounts on open by design, the panel's own toolbar button takes over the close action — and that button sat under the fixed 38px Tauri titlebar drag region, so clicks turned into window drags. This PR gives
.artifactPanelDockthe same--web-shell-desktop-titlebar-heighttop inset that.contextShelland the sidebar already have, and adds an e2e regression spec for both the plain-browser hand-over path and the macOS occlusion path.Why it's needed
The dock portals into a
display: contentsslot, so it is a direct.appShellflex child — a sibling of.contextShell, not a descendant — and never received the 38px titlebar inset. The panel toolbar (min-height: 52px, 30px vertically-centred toggle button, y≈11–41) renders at the very top of the window, underneath the drag region (position: fixed,z-index: 40, y 0–38). With the header toggle unmounted by design, Esc only wired for the fullscreen variant, and the resize divider clamping atMIN_ARTIFACT_PANEL_WIDTH, no close path remained — and because panel state persists across restarts, the panel came back open after relaunch too.App.test.tsxalready documents this failure mode: "If the views ever move out of the padded shell, the desktop drag strip overlaps their header controls."Reviewer Test Plan
How to verify
Reproduced and verified in a real browser against the web-shell dev server (
packages/web-shell, mock daemon harness), 1440x900 viewport (docked path):App.test.tsx) and the panel toolbar's own toggle (same label,aria-pressed="true") closes the panel. Already passing before this PR.window.__QWEN_CODE_MACOS_TITLEBAR__ = trueinjected (what the Tauri shell sets on macOS): before this PR,document.elementFromPointat the panel toggle's center returns.qwen-code-macos-titlebar-drag-regionand a real mouse click there never reaches the button — panel stays open. After this PR, the panel toolbar renders below the strip, the click lands, and the panel closes.Automated:
npx playwright test --config playwright.config.ts web-shell.right-panel-macos-titlebar --project=chromiuminpackages/web-shell(2 passed; the macOS test fails on main at the occlusion assertion), plusnpx vitest run client/App.test.tsx -t "right panel"(10 passed, confirms the pinned hand-over behavior is unchanged) andnpm run typecheckinpackages/web-shell(clean).Evidence (Before & After)
Before (macOS flag on): panel toolbar sits at the very top of the window, under the drag region — the toggle button's click point belongs to the strip, clicking does not close the panel.
After (macOS flag on): panel toolbar is inset 38px, level with the chat header; clicking the toggle closes the panel.
Tested on
padding-topresolves to 0)Environment (optional)
Vite dev server + Playwright Chromium with the repo's mock-daemon e2e harness.
Risk & Scope
padding-topresolves to 0, so browser/Linux/Windows rendering is byte-identical.@smoke, and PR CI runs only--grep @smoke(ci.yml:1552, in a job additionally gated onci_profile == 'full'). The spec actually runs in the full lane ate2e.yml:787, gated toschedule || workflow_dispatch(e2e.yml:733), i.e. the0 4 * * *nightly. So it guards against someone silently undoing the inset; it does not gate a PR. That is deliberate, because the smoke job's own comment (ci.yml:1354-1356) records 184 tests plus ~7.5 min of setup already finishing "right at 20 min" of a 30-min timeout, with sharding as the real fix.Linked Issues
Fixes #12874
中文说明
这个 PR 做了什么
在 macOS Desktop 壳里，停靠态右侧面板（文件更改／侧边任务／网页预览／轨迹／终端）一旦展开就无法关闭：头部 toggle 按钮在面板展开后按设计卸载，面板自身工具栏上的按钮接管关闭动作——而那个按钮正好被顶部固定的 38px Tauri 标题栏拖拽条压住，点击变成了窗口拖动。本 PR 给
.artifactPanelDock补上.contextShell和侧边栏已有的--web-shell-desktop-titlebar-height顶部内缩，并新增一条 e2e 回归测试，同时覆盖普通浏览器的接管路径和 macOS 遮挡路径。为什么需要
dock 通过
display: contents的 slot portal 渲染，因此它是.appShell的直接 flex 子元素——是.contextShell的兄弟节点而非后代——从未获得 38px 标题栏内缩。面板工具栏（min-height: 52px，30px 高的 toggle 按钮垂直居中，约 y≈11–41）从窗口最顶端开始渲染，正好落在拖拽条（position: fixed、z-index: 40、y 0–38）之下。头部 toggle 按设计卸载、Esc 只接全屏变体、分隔线拖拽在MIN_ARTIFACT_PANEL_WIDTH处收敛——所有关闭路径都不存在；且面板状态跨重启持久化，重启后面板依然展开。App.test.tsx已记录过这个失效模式："If the views ever move out of the padded shell, the desktop drag strip overlaps their header controls."验证方式
在真实浏览器中对 web-shell dev server（
packages/web-shell，mock daemon harness）以 1440x900 视口（停靠路径）复现并验证：不注入 flag 时接管路径正常；注入window.__QWEN_CODE_MACOS_TITLEBAR__ = true后，修复前document.elementFromPoint在按钮中心命中拖拽条、真实鼠标点击无法关闭面板，修复后工具栏下移 38px、点击生效、面板关闭。自动化：packages/web-shell下npx playwright test --config playwright.config.ts web-shell.right-panel-macos-titlebar --project=chromium（2 passed，其中 macOS 用例在 main 上会红）、npx vitest run client/App.test.tsx -t "right panel"（10 passed，钉住的接管行为不变）、npm run typecheck（干净）。风险与范围
padding-top为 0，浏览器/Linux/Windows 渲染完全不变。@smoke，而 PR CI 只跑--grep @smoke（ci.yml:1552，且该 job 还额外受ci_profile == 'full'限制）。spec 实际由e2e.yml:787的完整 lane 执行，在e2e.yml:733被限制为schedule || workflow_dispatch，即0 4 * * *夜间任务。所以它防的是有人悄悄撤掉这段内缩，并不拦 PR。这是刻意的：smoke job 自己在ci.yml:1354-1356的注释记录了 184 条测试加约 7.5 分钟准备已经「正好卡在 20 分钟」（超时上限 30 分钟），真正的修法是分片。修复 #12874。