feat(web-shell): add mobile sidebar drawer with session list - #6003
Merged
Conversation
qwen-code-ci-bot
requested changes
Jun 29, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR @pomelo-nwu! The mobile sidebar drawer looks like a useful feature.
However, the PR body doesn't follow the required template. The current body uses ## Motivation / ## Changes headings, but the template requires:
## What this PR does— describe the change in prose## Why it's needed— motivation and user-facing benefit## Reviewer Test Plan— including### How to verify,### Evidence (Before & After), and the### Tested onOS table## Risk & Scope— risks, out-of-scope items, breaking changes## Linked Issues—Closes #Nwith the closing keyword under this heading
The ### Tested on table is especially important — maintainers need to know which OS/browser combos you've actually verified.
Could you reformat the body to match the template? Happy to re-run the review once that's done.
中文说明
感谢 @pomelo-nwu 的 PR！手机端侧边栏抽屉是个实用的功能。
不过 PR 正文没有遵循模板格式。当前使用了 ## Motivation / ## Changes 标题，但模板要求：
## What this PR does— 用文字描述变更内容## Why it's needed— 动机和用户价值## Reviewer Test Plan— 包括### How to verify、### Evidence (Before & After)、以及### Tested on操作系统表格## Risk & Scope— 风险、不在范围内的内容、破坏性变更## Linked Issues— 在此标题下使用Closes #N关闭关键词
### Tested on 表格尤为重要——维护者需要知道你实际验证了哪些 OS/浏览器组合。
请按模板重新整理正文，完成后我会重新审查。
— Qwen Code · qwen3.7-max
Collaborator
Author
Test Screenshots / 测试截图Mobile initial state (390x844)Hamburger button visible at top-left, sidebar hidden. 
Mobile drawer openTapping hamburger opens sidebar as overlay with backdrop. Full session list available. 
Mobile drawer closedTapping backdrop closes the drawer. 
Desktop (1280x800) - no regressionNo hamburger button, sidebar displayed inline as before. 
|
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
wenshao
requested changes
Jun 29, 2026
pomelo-nwu
force-pushed
the
feat/mobile-sidebar-drawer
branch
from
June 29, 2026 23:09
4d5d4b3 to
5c130ce
Compare
qwen-code-ci-bot
requested changes
Jun 29, 2026
DragonnZhang
left a comment
DragonnZhang
left a comment
Collaborator
There was a problem hiding this comment.
No new blocking issues found at this head. All high-confidence bugs visible in the current diff are already covered by existing unresolved review comments (stacking-context :global(.sidebar) selector mismatch, capture-phase e.stopPropagation() breaking inner Escape handlers, unconditional role="dialog" on desktop, and sidebar state reset from toggled collapsed prop). No duplicate findings posted.
— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
DragonnZhang
left a comment
DragonnZhang
left a comment
Collaborator
There was a problem hiding this comment.
Automated review pass — no new blocking issues found beyond those already tracked in open review threads.
The four unresolved threads from the previous round cover the remaining correctness problems:
:global(.sidebar)selector is a no-op against CSS-Modules hashed class names, so the z-index fix for the stacking context bug never applies (sidebar is still obscured by the backdrop on mobile).- Capture-phase
e.stopPropagation()on the Escape keydown listener intercepts Escape before inner sidebar inputs (rename, search) can handle it, closing the drawer instead of cancelling the inner action. role="dialog"andaria-modal="true"are set unconditionally, causing screen readers to treat the desktop persistent sidebar as a modal dialog.- Toggling
collapsed={sidebarCollapsed && !mobileDrawerOpen}on each drawer open/close triggers the sidebar's internaluseEffect([collapsed]), silently resetting search text and project expansion state on every drawer dismissal.
No additional issues were identified in this pass.
Generated by Claude Code
wenshao
reviewed
Jun 30, 2026
Replace the display:none behavior at viewport <=760px with an overlay drawer pattern. A hamburger menu button appears on mobile, tapping it slides the existing WebShellSidebar in as a fixed overlay with a semi-transparent backdrop. Selecting or creating a session auto-closes the drawer. Desktop layout (>=761px) is unaffected. Closes #6000
- Use display:contents for desktop wrapper transparency (Critical: sidebar was hidden) - Fix z-index stacking so sidebar renders above backdrop in drawer - Force sidebar expand when mobile drawer is open (collapsed state) - Hide resizeHandle on mobile to prevent touch scroll conflicts - Reset drawer state on viewport resize via matchMedia listener - Add role=dialog, aria-modal, Escape key dismissal, body scroll lock - Add aria-expanded to hamburger button - Close drawer when opening Settings or resuming sessions
- Remove dead :global(.sidebar) selector (CSS Modules hash class names) - Fix Escape key capture-phase handler to not intercept sidebar inputs - Conditionally apply role=dialog/aria-modal only when drawer is open - Stop toggling collapsed prop on drawer open/close to preserve sidebar state - Add closeMobileDrawer() for bare /resume command path - Fix hamburger button vertical centering in empty chat state on mobile
pomelo-nwu
force-pushed
the
feat/mobile-sidebar-drawer
branch
from
June 30, 2026 02:51
5b1a69e to
4cf8be7
Compare
Collaborator
Maintainer local verification — built & ran the real UII built this PR locally (head Verdict: one blocking bug — the drawer opens but every control inside it is un-tappable. Everything else is solid. Please fix before merge; the fix is one line and I verified it. Static checks — all green
The PR adds no tests; the suite still passes (no regression). 🔴 Blocking: the backdrop covers the sidebar — all in-drawer controls are deadWhen the drawer is open on mobile, the semi-transparent backdrop is painted on top of the sidebar, so tapping any control (New chat / search / a session / Settings) is intercepted by the backdrop, which just closes the drawer. The drawer is visually present but functionally inert for its stated purpose ("switch between sessions on mobile"). Root cause — inside the
A positioned sibling with Evidence —
Behavioral proof — raw tap at the Settings button's on-screen coordinates: (Playwright independently refuses the click: " This is why the 3 "in-drawer action" E2E checks below could not pass on the PR as-is. ✅ Verified fix (one line) @media (max-width: 760px) {
.sidebar.mobileOpen {
display: flex;
width: var(--web-shell-sidebar-width, 260px);
min-width: var(--web-shell-sidebar-width, 260px);
+ z-index: 50; /* lift above .mobileBackdrop (z-index:49); .sidebar is already position:relative */
}
}After this change I rebuilt and re-ran: Real-UI E2E matrix
M16 note (with the fix): the session switch works (the Mutation A/B (is the PR load-bearing?)Reverting the 5 files to the merge-base and rebuilding → no hamburger, no drawer wrapper, sidebar What's genuinely well doneCorrect open/close state machine; matchMedia auto-close on rotate/resize; Escape with an INPUT/TEXTAREA/contentEditable guard; body-scroll lock with proper restore; desktop truly unaffected ( Minor (non-blocking) nits
🇨🇳 中文版（完整对应）维护者本地验证 — 构建并运行了真实 UI我在本地构建了该 PR（head 结论：一个阻断性 bug —— 抽屉能打开，但里面所有控件都点不动。 其余部分都很扎实。请合并前修复；修复只需一行，我已验证。 静态检查 — 全绿
PR 没有新增测试；现有套件仍全过（无回归）。 🔴 阻断：遮罩盖住了侧边栏 —— 抽屉内所有控件失效手机上抽屉打开时，半透明遮罩被绘制在侧边栏之上，于是点击任何控件（新对话 / 搜索 / 会话 / 设置）都被遮罩拦截，只会关闭抽屉。抽屉视觉上存在，但对其宣称的用途（"手机上切换会话"）功能性失效。 根因 —— 在
带 证据 —— 抽屉打开、390×844 时，对每个控件中心点做
行为验证 —— 在"设置"按钮的屏幕坐标上原始点击： （Playwright 也独立拒绝点击：" 这就是下方 3 个"抽屉内动作" E2E 检查在 PR 原样下无法通过的原因。 ✅ 已验证的修复（一行） @media (max-width: 760px) {
.sidebar.mobileOpen {
display: flex;
width: var(--web-shell-sidebar-width, 260px);
min-width: var(--web-shell-sidebar-width, 260px);
+ z-index: 50; /* 抬到 .mobileBackdrop(z-index:49) 之上；.sidebar 已是 position:relative */
}
}改完后我重新构建并重跑：四个点的 真实 UI E2E 矩阵
M16 注（带修复时）： 会话切换是成功的（ 变异 A/B（PR 是否承重？）把 5 个文件回退到 merge-base 重新构建 → 手机上无 hamburger、无抽屉包裹层、侧边栏 确实做得好的地方正确的开/关状态机；旋转/缩放时 matchMedia 自动关闭；带 INPUT/TEXTAREA/contentEditable 守卫的 Escape；body 滚动锁定且正确恢复；桌面真正不受影响（ 次要（非阻断）问题
Verified locally on macOS: real |
qwen-code-ci-bot
requested changes
Jun 30, 2026
Collaborator
Author
Review Response — both Critical conversations fixedFixed
All review threads resolved. |
Collaborator
Author
Review Response — 2 new conversations addressedFixed (commit 44f848d)
Deferred
All review threads resolved. |
qwen-code-ci-bot
requested changes
Jun 30, 2026
The .qwen-session file is a developer-local session UUID generated by qwen serve. It was accidentally committed to the repo and should never be tracked.
Collaborator
Author
Review Response — 4 new conversations addressedFixed (commit 763a56b)
Rejected — out of scope
All review threads resolved. |
wenshao
previously approved these changes
Jun 30, 2026
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required headings present, bilingual, linked issues. On direction: this is a focused follow-up to the already-merged #6076, landing review feedback on daemon loop detection safety. The problem is real — the original bucketing by On approach: the scope feels tight and right-sized for the stated goals. Four surgical changes across two files:
No drive-by refactors, no scope creep. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必要标题齐全，双语说明，关联 issue。 方向：这是对已合并 #6076 的 review follow-up，补上 daemon loop detection 安全性方面的 review 反馈。问题真实存在——原来按 方案：范围紧凑，与目标匹配。4 个精准改动，涉及 2 个文件：
没有顺手重构，没有范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewThe implementation is well-executed. The drawer wraps the existing State management is correct. Escape key handling is thoughtful. The handler checks Touch scroll prevention correctly allows native scrolling inside the drawer panel while blocking it on the backdrop and outside the drawer. The
One minor note: VerificationDev server boots cleanly: Visual verification of the mobile drawer (CSS transitions, backdrop opacity, drawer slide) requires a browser with DevTools mobile emulation — not testable from the terminal. The Reviewer Test Plan in the PR body covers the right steps for manual verification. 中文说明代码审查实现质量很好。Drawer 包裹了现有的 状态管理正确。 Escape 键处理周到。 处理器检查了 触摸滚动防护 正确允许 drawer 面板内的原生滚动，同时阻止遮罩层和 drawer 外部的滚动。通过 桌面端的 一个小注意点： 验证
移动端 drawer 的视觉验证（CSS 过渡、遮罩透明度、drawer 滑动）需要浏览器 DevTools 移动端模拟——无法在终端中测试。PR 正文中的审查者测试计划覆盖了正确的手动验证步骤。 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: this is a well-scoped, well-implemented feature. The overlay drawer pattern is the right call for mobile sidebar access, the code is straightforward, and the implementation reuses the existing sidebar component instead of building something parallel. All 572 tests pass, typecheck is clean, lint is clean, and the dev server boots without issues. The only reservation is the handful of unrelated drive-by changes (CLI component formatting, The core mobile drawer logic — state management, Escape handling, touch scroll prevention, LGTM. ✅ 中文说明总结：这是一个范围合理、实现良好的功能。Overlay drawer 模式是移动端侧边栏访问的正确选择，代码简洁，实现复用了现有侧边栏组件而非另起炉灶。全部 572 个测试通过，类型检查通过，lint 通过，dev server 正常启动。 唯一的顾虑是少量无关的顺手改动（CLI 组件格式化、 核心移动端 drawer 逻辑——状态管理、Escape 处理、触摸滚动防护、 LGTM. ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
requested changes
Jun 30, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
The touchmove scroll lock handler is too broad — it prevents scrolling inside the sidebar drawer on iOS Safari for users with long session lists. Quick fix: scope the preventDefault() to exclude touches inside the sidebar, or use overscroll-behavior: contain. See my review comments above for details. 🙏
qwen-code-ci-bot
requested changes
Jun 30, 2026
Collaborator
|
@qwen-code /triage |
- Don't preventDefault touchmove inside the drawer so the session list can scroll natively; only block scrolling on the page behind it. - Defer Escape to a pending tool/permission approval (reject) instead of closing the drawer when a prompt is visible. - Reuse isEditableTarget from utils/dom and only bail out for editable targets outside the drawer, so the drawer search input still closes on the first Escape. - Close the drawer before awaiting loadSession so it doesn't linger over the old transcript, matching the other session-switch paths. - Keep the drawer panel visible until the backdrop finishes fading out to avoid a one-frame flicker on close. Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
# Conflicts: # packages/web-shell/client/App.tsx
wenshao
requested changes
Jun 30, 2026
… scroll - collapsed: a user who collapsed the desktop sidebar got a mobile drawer that still rendered as the icon rail (no session list — the whole point of the drawer). Force the expanded layout while the drawer is open. - touchmove: the allowlist matched the outer [data-mobile-drawer] wrapper, which also contains the full-screen backdrop, so a touchmove starting on the dim backdrop skipped preventDefault and let iOS Safari scroll the page behind. Exclude the backdrop so only the panel keeps native scroll. Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
qwen-code-ci-bot
requested changes
Jun 30, 2026
wenshao
reviewed
Jun 30, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No new issues found in this review pass. The mobile drawer implementation has been thoroughly reviewed across multiple rounds — the stacking context, Escape handler, touchmove scroll lock, collapsed rail override, backdrop allowlist, and aria-modal conditional application are all correctly addressed at this head.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
requested changes
Jun 30, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Also found (unmapped to diff lines):
- [Suggestion] The sidebar's collapse button (
.collapseButton,WebShellSidebar.tsx:948) remains visible and clickable inside the mobile drawer. Sincecollapsedis forced tofalsewhile the drawer is open, clicking it persistssidebarCollapsed=trueto localStorage with no visual effect — silently corrupting the user's desktop sidebar preference. Fix: add.collapseButton { display: none; }inside the@media (max-width: 760px)block inWebShellSidebar.module.css.
— qwen3.7-max via Qwen Code /review
- Hide the sidebar collapse button while the mobile drawer is open so its no-op toggle can no longer silently persist desktop collapsed state. - Close the drawer before awaiting createSession() so a failed create no longer leaves the drawer stuck open with page scroll locked. - Drop redundant width/min-width/position from .sidebar.mobileOpen and cap it with max-width:100vw so a wide persisted width can't overflow phones. Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
DragonnZhang
approved these changes
Jun 30, 2026
DragonnZhang
left a comment
DragonnZhang
left a comment
Collaborator
There was a problem hiding this comment.
Incremental review of commit a18ad9aea (fix: harden mobile drawer collapse, error path, and width cap):
All three changes are correct and well-targeted:
- createNewSession error path: Moving
closeMobileDrawer()before theawaitensures the drawer closes and body scroll is restored even ifcreateSession()throws, matching the pattern inloadSidebarSession. - CSS
max-width: 100vw: Correctly caps the drawer at the viewport width when the shared--web-shell-sidebar-widthvariable has been widened by the desktop resize handle. The removedposition: relative,width, andmin-widthoverrides were redundant with the base.sidebarclass. - Collapse button hidden on mobile: Correct -- collapsing the sidebar while the drawer overlay is open would produce a confusing 56px-wide mobile panel. The guard is scoped to
mobileOpen, so desktop is unaffected.
No new findings. LGTM.
wenshao
reviewed
Jun 30, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Tapping the currently-active session in the mobile drawer doesn't close it. handleLoadSession in WebShellSidebar.tsx has an early return when sessionId === currentSessionId, so onLoadSession (which calls closeMobileDrawer()) is never invoked. This is pre-existing code, but the PR makes it observable for the first time on mobile — the user taps a session and nothing happens, requiring an extra tap on the backdrop or hamburger to close the drawer.
— glm-5.2 via Qwen Code /review
| const el = e.target as HTMLElement | null; | ||
| if ( | ||
| el?.closest('[data-mobile-drawer]') && | ||
| !el.closest(`.${styles.mobileBackdrop}`) |
Collaborator
There was a problem hiding this comment.
[Suggestion] The preventScroll touchmove handler blocks scrolling in portaled dialogs. DialogShell uses createPortal(content, document.body), so dialogs opened from the sidebar (e.g., delete-session confirmation at WebShellSidebar.tsx:779) are outside [data-mobile-drawer]. Touchmove inside these portaled dialogs gets preventDefault() called, blocking native touch scrolling on mobile.
Consider also exempting [data-keyboard-scope] (the attribute DialogShell uses to claim keyboard scope):
Suggested change
| !el.closest(`.${styles.mobileBackdrop}`) | |
| !el.closest(`.${styles.mobileBackdrop}`) | |
| ) || | |
| el?.closest('[data-keyboard-scope]') |
— glm-5.2 via Qwen Code /review
wenshao
reviewed
Jun 30, 2026
| // outside the drawer; the drawer's own search input should still close | ||
| // the drawer on the first Escape. | ||
| if ( | ||
| isEditableTarget(target) && |
Collaborator
There was a problem hiding this comment.
[Suggestion] The editable-target guard only bails for inputs outside the drawer (!target?.closest('[data-mobile-drawer]')). For the rename input inside the drawer (WebShellSidebar ~line 662), the condition is true && false = false — no early return — so stopPropagation() fires and the drawer closes. The rename input's own Escape handler (cancelRename()) never runs, silently discarding in-progress edits.
The search-input case appears intentional per the code comment, but the rename case is a data-loss edge case. Simplifying the guard to bail for all editable targets (regardless of drawer ancestry) lets both inputs handle Escape first — the drawer closes on the second Escape:
Suggested change
| isEditableTarget(target) && | |
| if (isEditableTarget(target)) { | |
| return; | |
| } |
— qwen3.7-max via Qwen Code /review
Collaborator
✅ Real-browser verification — mobile sidebar drawerVerified the current head ( TL;DREverything works. The z-index stacking issue flagged in an earlier review round (the backdrop painting over the drawer controls) is fixed by commit How it was tested
Results — 29/29 behavioral checks + extras
Mutation A/B — proving
|
| Variant | elementFromPoint on the 8–11 drawer controls |
Real Playwright .click() |
|---|---|---|
PR head (z-index: 50) |
every control returns itself | "New chat" / "Settings" fire |
Reverted to z-index: auto |
every control returns _mobileBackdrop_ |
clicks time out — <div class="_mobileBackdrop_…"> intercepts pointer events |
Without the fix, every drawer control is dead to taps; with it, all are tappable. (Reverted the mutation after testing — worktree is clean.)
Static checks
tsc -p packages/web-shell/tsconfig.lib.json --noEmit→ cleaneslint --max-warnings 0on the changed.tsxfiles → cleanvitest run(web-shell) → 572 / 572 pass, no regressions
Optional nits (non-blocking)
- No automated regression test ships with the PR — the whole feature is currently guarded only by manual/E2E testing. A small RTL test asserting the drawer's stacking order and the synchronous-close paths would prevent the exact bug that already had to be fixed once from silently coming back.
- Unrelated diff noise:
InlineParallelAgentsDisplay.tsxandToolGroupMessage.tsxare prettier-only reformats, anddocs/users/configuration/settings.mdhas a whitespace-only change — all harmless, but out of scope for a web-shell feature PR. - Minor a11y: the new wrapper
<div>and the inner sidebar both carryaria-label="Workspace sidebar"(a duplicate nested label on desktop, where the wrapper isdisplay:contents). The wrapper label is required for the mobilerole=dialogcase; consider labeling only one to avoid the redundancy on desktop.
🇨🇳 中文版（点击展开）
✅ 真实浏览器验证 —— 移动端侧边栏抽屉
在本地用 tmux 托管真实的 qwen serve --web 守护进程，并用 Playwright/Chromium 驱动真实的 web-shell SPA，在真实的移动端/桌面端视口下验证了当前 head（a18ad9ae）。守护进程提供的是从该 head 重新构建的 packages/web-shell/dist。
结论速览
功能全部正常。此前一轮 review 中指出的 z-index 层叠问题（遮罩盖在抽屉控件之上）已由 commit 801af24e 修复；我通过变异测试确认了 .sidebar.mobileOpen 上的 z-index: 50 是承重的（去掉就坏）。tsc / eslint / 572 个 web-shell 单测全部通过。👍 可以合并 —— 文末有几条可选的小建议。
测试方法
- 守护进程：
node packages/cli/dist/index.js serve --web，监听127.0.0.1，工作区预置了一个 session，使会话列表/行是真实的。 - 驱动： Playwright Chromium —— 视口 390×844（移动）、1280×800（桌面），以及 760/761 边界。
- 关键探针： 抽屉打开时，对每一个可交互控件做
document.elementFromPoint(中心x, 中心y)命中测试。这能抓到截图发现不了的"被遮罩盖住"类 bug —— 深色主题下 50% 黑遮罩盖在近黑色侧边栏上，肉眼几乎无差别，只有命中测试（或真实点击）才能暴露回归。
结果 —— 29/29 行为检查 + 额外项
| 范围 | 检查项 | 结果 |
|---|---|---|
| 桌面 1280×800 | 无 hamburger；侧边栏内联显示；wrapper 为 display:contents |
✅ |
| 移动 390×844 | hamburger 可见；关闭时侧边栏隐藏；aria-expanded 在 false↔true 间切换 |
✅ |
| 抽屉打开 | role=dialog + aria-modal=true；遮罩 opacity:1；页面滚动被锁；宽度封顶（260 ≤ 390）；侧栏折叠按钮隐藏 |
✅ |
| 命中测试（z-index） | 8/8 抽屉控件在中心点可达；0 个被遮罩盖住 | ✅ |
| 关闭路径 | "New chat" / 切换会话在 await 之前同步关闭抽屉并恢复页面滚动 |
✅ |
| 遮罩 | 点击暗色区域关闭抽屉 + 恢复滚动 | ✅ |
| Escape | 关闭抽屉；当焦点在抽屉内搜索框时也能关闭（捕获阶段监听器优先于搜索框自身的 Esc 处理） | ✅ |
| Settings | 点击 Settings 关闭抽屉并打开设置弹窗 | ✅ |
| 缩放→桌面 | matchMedia('(max-width:760px)') 自动关闭抽屉、恢复滚动、隐藏 hamburger（无残留遮罩） |
✅ |
| 边界 | 宽度=760 时 hamburger 可见，=761 时隐藏 | ✅ |
| 折叠轨道 | 桌面端折叠状态不会把移动抽屉缩成窄轨（collapsed={sidebarCollapsed && !mobileDrawerOpen}） |
✅ |
变异 A/B —— 证明 z-index: 50 是真正的修复
| 变体 | 对抽屉控件的 elementFromPoint |
真实 Playwright .click() |
|---|---|---|
PR head（z-index: 50） |
每个控件返回自身 | "New chat" / "Settings" 正常触发 |
改回 z-index: auto |
每个控件都返回 _mobileBackdrop_ |
点击超时 —— <div class="_mobileBackdrop_…"> intercepts pointer events |
没有这个修复，抽屉里每个控件都点不动；有了它，全部可点。（测试后已还原变异，worktree 干净。）
静态检查
tsc -p packages/web-shell/tsconfig.lib.json --noEmit→ 干净- 对改动的
.tsx文件跑eslint --max-warnings 0→ 干净 vitest run（web-shell）→ 572 / 572 通过，无回归
可选的小建议（不阻塞合并）
- PR 没有附带自动化回归测试 —— 整个功能目前只靠手动/E2E 保障。加一个小的 RTL 测试，断言抽屉的层叠顺序与同步关闭路径，可以防止这个"已经修过一次"的 bug 再次悄悄回归。
- 无关的 diff 噪声：
InlineParallelAgentsDisplay.tsx和ToolGroupMessage.tsx是纯 prettier 格式化改动，docs/users/configuration/settings.md是纯空白改动 —— 都无害，但不属于这个 web-shell 功能 PR 的范围。 - 轻微 a11y： 新加的 wrapper
<div>和内层 sidebar 都带了aria-label="Workspace sidebar"（桌面端 wrapper 为display:contents时形成嵌套重复标签）。wrapper 的标签是移动端role=dialog所需要的；建议只在其中一处保留，避免桌面端冗余。
Verification done locally on macOS with the real serve daemon + Playwright Chromium against PR head a18ad9ae. Screenshots (desktop / mobile-closed / mobile-open / settings) captured locally.
| const el = e.target as HTMLElement | null; | ||
| if ( | ||
| el?.closest('[data-mobile-drawer]') && | ||
| !el.closest(`.${styles.mobileBackdrop}`) |
Collaborator
There was a problem hiding this comment.
[Suggestion] The backdrop exclusion in the touchmove handler uses a CSS Modules hashed class name: el.closest(\.${styles.mobileBackdrop}`). This is the only closest()/querySelector()call in the web-shell client that relies on a CSS Modules class — every other element identifier in this handler uses adata- attribute ([data-mobile-drawer]`).
If .mobileBackdrop is renamed in CSS (or a build config change causes the hash to differ), this silently breaks: closest('.undefined') never matches, and the backdrop exclusion stops working — users can scroll the page behind the open drawer on touch devices. A data- attribute would be consistent with the pattern already used one line above and survive CSS refactors.
Suggested change
| !el.closest(`.${styles.mobileBackdrop}`) | |
| !el.closest('[data-mobile-backdrop]') |
And add data-mobile-backdrop="" to the backdrop element at line 3890:
<div data-mobile-backdrop="" className={styles.mobileBackdrop} onClick={closeMobileDrawer} aria-hidden="true" />— qwen3.7-max via Qwen Code /review
| if (e.key !== 'Escape') return; | ||
| // A pending tool/permission approval owns Escape (it rejects the call), | ||
| // so don't let the drawer swallow it while a prompt is visible. | ||
| if (pendingApprovalRef.current) return; |
Collaborator
There was a problem hiding this comment.
[Suggestion] When the drawer is open and a tool/permission approval becomes pending, the approval UI renders inside .approvalOverlay in the footer (z-index: 6), which sits behind the drawer's .mobileBackdrop (z-index: 49). The Escape guard here correctly defers to the approval handler, but the approval buttons (Confirm/Deny) are unreachable behind the backdrop. On mobile, the user sees the open drawer and has no indication that an approval is waiting — the only recovery path is tapping the backdrop to close the drawer first.
Consider auto-closing the drawer when an approval appears:
useEffect(() => {
if (pendingApproval) closeMobileDrawer();
}, [pendingApproval, closeMobileDrawer]);This ensures the approval UI is always accessible when it matters.
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jun 30, 2026
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 30, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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
5 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
Replace the
display: nonebehavior at<=760pxwith an overlay drawer pattern for the web shell sidebar. On mobile viewports, a hamburger menu button appears in the chat pane header, and tapping it slides the existingWebShellSidebarin from the left as a fixed overlay with a semi-transparent backdrop. Selecting a session, creating a new one, or tapping the backdrop closes the drawer. Desktop layout is completely unaffected.Why it's needed
When accessing
qwen servefrom a mobile browser, the sidebar (which contains the session list, search, and new-session button) is hidden at viewport width<=760pxviadisplay: none— with no mobile alternative. Users have no way to switch between sessions on mobile.Reviewer Test Plan
How to verify
qwen serveand open in a mobile browser (or DevTools mobile emulation at 390x844)/resumecommand from mobile — drawer closes, session loadsEvidence (Before & After)
Before: sidebar completely hidden on mobile with no way to access it.
After: hamburger menu enables full sidebar access as an overlay drawer.
Screenshots posted in PR comments below.
Tested on
Environment (optional)
npm run devfrom packages/web-shell, tested in Chrome DevTools mobile emulation (iPhone 14 Pro, 390x844).Risk & Scope
display: contentsfor the drawer wrapper on desktop — well-supported in modern browsers but not in IE11 (irrelevant for this project)Linked Issues
Closes #6000
中文说明
将 web shell 侧边栏在 760px 以下的
display: none行为替换为 overlay drawer 模式。在手机视口中，聊天面板头部出现 hamburger 菜单按钮，点击后侧边栏以固定 overlay 方式滑入，带半透明遮罩。选择会话、新建会话或点击遮罩均可关闭 drawer。桌面端布局完全不受影响。手机浏览器访问
qwen serve时，侧边栏在 760px 以下直接display: none隐藏，用户无法切换会话。主要风险：桌面端使用
display: contents让 wrapper div 布局透明——现代浏览器支持良好，但 IE11 不支持（本项目不涉及）。无破坏性变更，无需迁移。