fix(web-shell): constrain responsive sidebar drawer - #10020
Merged
Conversation
github-actions
Bot
added
the
review/self-reported
Collaborator
|
|
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, not theory. The linked issue #10014 carries concrete reproduction steps — persist a wide desktop sidebar width, switch to drawer mode, and the drawer overflows its container with the close control gone. It is self-reported by the PR author (already labeled Direction: aligned. This is a responsive-layout regression in an actively maintained surface (the CHANGELOG carries many recent web-shell fixes); restoring drawer containment and an always-reachable close control is squarely within the existing WebShell sidebar contract. Size: not applicable — no core module paths are touched (web-shell client + docs only); roughly 42 production lines vs ~126 test lines. Approach: the scope feels right — it matches the issue's three asks (70% drawer width within the existing min/max limits, a visible close control in drawer mode, no overwrite of the persisted desktop state) without extra surface. Reusing the existing collapse button as the drawer close control instead of adding a new button is the right call. The one subtle bit — forcing the footer block to render when the host set Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，不是理论问题。关联 issue #10014 给出了具体复现步骤——桌面端持久化较宽的 sidebar 宽度后切到抽屉模式，抽屉溢出容器且关闭入口消失。该 issue 由 PR 作者自行上报（已打 方向：对齐。这是活跃维护界面上的响应式布局回归（CHANGELOG 近期有多项 web-shell 修复）；恢复抽屉的容器约束和始终可达的关闭入口完全在现有 WebShell sidebar 契约之内。 规模：不适用——未触及核心模块路径（仅 web-shell client 与 docs）；生产代码约 42 行，测试约 126 行。 方案：范围合理——与 issue 的三点诉求一致（70% 抽屉宽度且遵守已有 min/max 限制、抽屉模式保留可见的关闭控件、不覆盖桌面端持久化状态），没有多余改动。复用现有 collapse 按钮作为抽屉关闭入口而不是新增按钮是正确的做法。唯一的微妙之处——宿主设置 风险：无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI sketched my own approach before reading the diff — override width/min-width with a container-relative clamp in drawer mode, and reuse the existing collapse button as a close control wired to a close-only callback. The PR does exactly this, and the details hold up:
No correctness, security, or convention blockers found. The PR's three behavioral claims (70% drawer width within min/max, always-visible close control, no desktop-state overwrite) each have a pinning test: the new Playwright spec would fail against the old code (a 512px min-width cannot render at 0.7×412px). Test evidence — the PR's own CIAt this snapshot there are no failing checks. The main unit suite and the web-shell visuals capture are still running; the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: 中文说明代码审查：我在读 diff 之前先独立草拟了方案——抽屉模式下用容器相对的 clamp 覆盖 width/min-width，并复用现有 collapse 按钮作为关闭控件、接入只关闭抽屉的回调。PR 的做法与此一致，且细节经得起推敲：
未发现正确性、安全性或规范层面的阻塞问题。PR 的三项行为主张（70% 抽屉宽度且受 min/max 约束、关闭控件始终可见、不覆盖桌面端状态）各有钉住测试：新的 Playwright 用例在旧代码上必然失败（512px 的 min-width 不可能渲染为 0.7×412px）。 测试证据：截至本快照没有失败的检查。主单元测试与 web-shell 视觉捕获仍在运行；执行新 Playwright 回归的 作者自述的测试结果（"574 项 Vitest 通过"、"Pixel 7 回归 1/1"）仅为作者报告，不是独立复跑的证据；如需独立结算几何布局主张，维护者可触发 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean review and a structurally correct fix; the only open item is CI still in flight, so approval is deferred until it lands green. Stepping back: this is the kind of fix that should land. The problem is real — I verified the mechanism in the base code rather than taking the issue's word for it: the persisted width feeds both 中文说明置信度：4/5 —— 审查干净、修复在结构上正确；唯一未决项是 CI 仍在进行中，因此批准推迟到 CI 变绿之后。 总体来看：这是一个应该合入的修复。问题是真实的——我没有只采信 issue 的描述，而是在基础代码中验证了机制：持久化宽度被同时用于 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
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. |
dreamWB
force-pushed
the
codex/fix-responsive-sidebar-drawer-pr
branch
from
August 25, 2026 13:18
097416f to
855c8c1
Compare
Contributor
|
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）为单个提交。 |
dreamWB
force-pushed
the
codex/fix-responsive-sidebar-drawer-pr
branch
4 times, most recently
from
August 26, 2026 02:15
f9f2ecc to
b0e6f3d
Compare
qwen-code-ci-bot
requested changes
Aug 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6a": none — I completed every check I opened within budget.; "agent 6b": full monorepo npm run typecheck (ran the package-scoped tsc instead; the residual error found there is pre-existing and unrelated to the diff)..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6a"：none — I completed every check I opened within budget.；"agent 6b"：full monorepo npm run typecheck (ran the package-scoped tsc instead; the residual error found there is pre-existing and unrelated to the diff).。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
approved these changes
Aug 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/sidebar/WebShellSidebar.collapse-persist.test.tsx:364 (+2 locations) — [probe] New tests hardcode sidebar storage-key literals instead of importing the component's constantspackages/web-shell/client/components/sidebar/WebShellSidebar.collapse-persist.test.tsx:344 — [probe] The '512px' max-width expectation is an unnamed jsdom-viewport artifact no real drawer can have
中文说明
无阻断问题。LGTM！✅
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
dreamWB
force-pushed
the
codex/fix-responsive-sidebar-drawer-pr
branch
from
August 26, 2026 08:05
4d44295 to
9f3c023
Compare
yiliang114
reviewed
Aug 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Verified the drawer geometry path against the code at 4d44295:
.sidebar.mobileOpen(two-class specificity) overrides the base rule that feeds the persisted--web-shell-sidebar-widthinto bothwidthandmin-width. The oldmax-width: 100vwcap could never beat a largermin-widthunder CSS sizing rules, so replacing it withclamp(min, 70%, max)fixes the overflow structurally. The inline--web-shell-sidebar-min-width/--web-shell-sidebar-max-widthvars keep the CSS in sync withSIDEBAR_MIN_WIDTH/getSidebarMaxWidth(), and the clamp fallbacks match those constants.- The added
right: 0on.mobileDraweris load-bearing: without it the fixed layer is shrink-to-fit and70%has no definite width to resolve against. - Rendering the footer when
footer === false && mobileOpenis safe:footerItemsis an empty set in that case and every footer child is gated by it (footer.renderis short-circuited too), so only the close control appears. - The drawer close button calls
onMobileClose→closeMobileDrawerand never touchesonCollapsedChange, so neither the persisted desktop width nor the collapse preference is overwritten — pinned by the App and collapse-persist tests. - The resize handle is hidden and pointer-guarded in drawer mode, with
mobileOpenadded to the callback deps.
CI is green on this head (Test, web-shell E2E Smoke, Desktop Shell). One non-blocking, pre-existing note: the visuals job's workspace sidebar scenario fails with a strict-mode violation (getByText('Run auth migration') resolves to two session rows) on both this head and the merge-base — that's why the visual preview shows "no preview". Unrelated to this change.
No blockers from my side.
yiliang114
enabled auto-merge
August 26, 2026 09:15
yiliang114
approved these changes
Aug 26, 2026
yiliang114
approved these changes
Aug 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Verified the drawer geometry, footer close-control, and persistence paths at 4d44295 — the clamp fixes the overflow structurally, nothing overwrites the persisted desktop state, and the current head passes the full CI matrix including the web-shell E2E regression.
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR makes an open responsive sidebar drawer use 70% of its WebShell container while preserving the sidebar's existing minimum and maximum width constraints. It also keeps a visible footer close control in drawer mode, including when the host disables the normal footer, without changing the persisted desktop collapse or resize preferences.
Why it's needed
A desktop-resized sidebar width is persisted in localStorage and was reused as both width and min-width in narrow layouts. The previous mobile max-width rule could not override that larger min-width, so the drawer could overflow the WebShell container, hide content, and make the sidebar impossible to close from inside the panel.
Reviewer Test Plan
How to verify
Automated verification: the focused Vitest run passed 574 tests across the App and sidebar persistence suites; the new Pixel 7 Playwright regression passed 1/1; WebShell typecheck and production build passed. The full preflight was attempted but lint could not start on this machine because Node 22.11 resolved ESM
ansi-regex@6into CommonJSpretty-format@27; install output also reports affected dependencies require Node >=22.12.Evidence (Before & After)
Before: with a persisted 512px width, the narrow drawer inherited a 512px min-width and could extend beyond its container, hiding content and the internal close affordance.
After: the Pixel 7 Playwright regression measures the sidebar at 70% of the drawer container, verifies it remains inside the container, activates the visible Collapse control, and confirms the persisted width remains 512 after closing. A screenshot is included in the linked internal test report: https://alidocs.dingtalk.com/i/nodes/6LeBq413JA9BOdm2izKaqenbJDOnGvpb
Tested on
Environment (optional)
macOS, Node 22.11, standalone WebShell Vite server, Playwright mobile-chromium with Pixel 7 emulation and the repository mock daemon.
Risk & Scope
mobileOpensizing rule and the forced drawer path is covered by App tests. Windows and Linux were not tested locally. IDE integration was blocked because the IDE lockfile consumes WebShell 0.21.9-preview.0 while this contribution targets 0.22.0.Linked Issues
Fixes #10014
中文说明
本 PR 的改动
本 PR 让响应式侧边栏抽屉在打开时使用 WebShell 容器宽度的 70%，同时继续遵守侧边栏已有的最小和最大宽度约束。抽屉状态下始终保留 footer 右下角的关闭控件，即使宿主关闭普通 footer 也可用，并且不会改变桌面端持久化的收起状态或 resize 宽度。
为什么需要
桌面端手动调整的 sidebar 宽度会保存在 localStorage 中，窄屏时旧实现仍把它同时作为 width 和 min-width。原有移动端 max-width 无法覆盖更大的 min-width，因此抽屉可能超出 WebShell 容器，导致内容被遮挡，并且用户无法从面板内部找到可用的收起入口。
Reviewer 测试计划
验证方式
自动化验证：App 与 sidebar persistence 的定向 Vitest 共 574 项全部通过；新增 Pixel 7 Playwright 回归 1/1 通过；WebShell typecheck 和生产构建通过。已尝试完整 preflight，但本机 Node 22.11 把 ESM
ansi-regex@6解析给 CommonJSpretty-format@27，导致 lint 无法启动；安装日志同时提示相关依赖要求 Node >=22.12。前后证据
修复前：保存 512px 宽度后，窄屏抽屉继承 512px min-width，可能超出容器并隐藏内容和内部关闭入口。
修复后：Pixel 7 Playwright 回归实测 sidebar 为 drawer 容器的 70%，确认没有越界，点击可见的 Collapse 控件后抽屉关闭，并确认持久化宽度仍为 512。截图已内嵌在内部测试报告：https://alidocs.dingtalk.com/i/nodes/6LeBq413JA9BOdm2izKaqenbJDOnGvpb
已测试平台
环境
macOS、Node 22.11、standalone WebShell Vite server、Playwright mobile-chromium Pixel 7 仿真和仓库 mock daemon。
风险与范围
mobileOpen尺寸规则，强制抽屉路径已有 App 测试覆盖；Windows、Linux 未本地测试。IDE 联调因 IDE lockfile 使用 WebShell 0.21.9-preview.0、而本次贡献目标为 0.22.0 被依赖版本阻断。关联 Issue
Fixes #10014