Repository navigation
fix(web-shell): prevent sidebar footer overflow - #6522
Merged
Merged
Conversation
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. |
callmeYe
enabled auto-merge
July 8, 2026 08:03
wenshao
reviewed
Jul 8, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
WebShellSidebar.tsx:1527-1549 |
Drag-release width snap: clampSidebarVisualWidth floors at 200px during drag, but clampSidebarWidth floors at 220px on release. Dragging to rawWidth 200-220 causes a visible 15-20px forward jump on release. |
Align the persisted floor with the visual floor, or narrow the gap (e.g., SIDEBAR_DRAG_VISUAL_MIN_WIDTH = 210). |
WebShellSidebar.module.css:960-986 |
Redundant .footerTight CSS selectors: All .footerTight rules are paired with identical .footerCompact selectors. Since tight (width < 250) always implies compact (width < 344), the .footerTight selectors never match an element .footerCompact doesn't already cover. |
Remove .footerTight from the grouped CSS selectors. Keep the JSX footerTight flag for the version badge visibility logic. |
WebShellSidebar.test.tsx:114 |
Test helper uses MouseEvent instead of PointerEvent: new MouseEvent(type, ...) lacks pointerId, so setPointerCapture(undefined) silently fails. The pointer capture code path is entirely untested. |
Use new PointerEvent(type, { bubbles: true, clientX, pointerId: 1 }) to exercise the capture path. |
WebShellSidebar.test.tsx:390-431 |
Missing test coverage for drag edge cases: No test at the collapse threshold boundary (rawWidth 144/145), no test for handlePointerCancel, and the independent collapse check in handlePointerUp is not covered. |
Add boundary tests at 144/145, a pointercancel test verifying cleanup, and a pointerup-only collapse test. |
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 8, 2026
callmeYe
added this pull request to the merge queue
Jul 8, 2026
This was referenced Sep 9, 2026
qwen-code-dev-bot
pushed a commit
to DragonnZhang/qwen-code
that referenced
this pull request
Sep 9, 2026
…breakpoint (QwenLM#11470) The sidebar footer degraded in two steps that did not line up. Below 344px the settings button already drops its text label and every footer button becomes a fixed 26px icon, but the version label stayed until 250px. That label is the one footer child that can neither shrink nor truncate (`flex: 0 0 auto; white-space: nowrap`, with no `min-width`, `overflow` or `text-overflow`), so between the two breakpoints it overflowed `.footerPrimary` and painted into `.footerActions`. The action group comes later in the DOM, so its icons landed on top of the glyphs and stayed clickable. `SIDEBAR_DEFAULT_WIDTH` is 260, which sits inside that window, so the default sidebar rendered overlapped without any resizing. Measured in headless Chromium at a 260px sidebar: the label spans x 42 -> 88.95 while the theme toggle starts at x 71, a 17.95px overlap. Gate the label on `footerCompact`, the breakpoint that already gates the settings text label, so the footer degrades in a single step and the label is gone before it can collide. That leaves `footerTight` with no remaining effect: every CSS rule naming it was already grouped with an identical `.footerCompact` rule, and the version label was the only place the two still differed. Drop it along with `SIDEBAR_FOOTER_TIGHT_WIDTH` and the redundant selectors rather than leaving dead code behind. The redundant `.footerTight` selectors were already flagged in the review of QwenLM#6522, the PR that introduced the two-tier breakpoints, with the advice to drop them from the grouped CSS but keep the JSX flag because the version badge still needed it. Moving the badge onto the compact breakpoint removes the last thing that flag did, so this drops the CSS and the flag together. Fixes QwenLM#11453 Patrol-Run: qwen-issue-patrol/jmttxmmmbtg Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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
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 the web shell sidebar footer respond to constrained widths in stages: the settings entry becomes icon-only first, the version badge is removed at tighter widths, and dragging past the compact threshold collapses the sidebar to the rail while preserving a sane expanded width for the next open.
Why it's needed
When launching the web shell locally, I found that the sidebar footer did not have enough width: the "Settings" label was covered, only half of the settings gear was visible, and while shrinking the sidebar the buttons that should remain available in the collapsed/compact footer could become invisible. The staged fallback keeps the controls accessible and avoids clipped footer actions without changing daemon/session behavior.
Reviewer Test Plan
How to verify
Open the web shell with the sidebar enabled, make the viewport large enough to show the session overview and split-view footer actions, then drag the sidebar narrower. Confirm the settings label disappears before the version badge, the remaining icons do not overlap at tight widths, and dragging farther left collapses the sidebar to the rail. Expand it again and confirm it restores to a normal supported width rather than the sub-threshold drag width.
Evidence (Before & After)
Before: local startup showed an overcrowded sidebar footer where the "Settings" text was obscured, the settings gear was only partly visible, and footer buttons could disappear while the sidebar was being narrowed. After: local browser smoke measured no footer overlaps at 260px and 220px sidebar widths, confirmed drag collapse to the 56px rail, and confirmed expanding restored 220px.
Tested on
Environment (optional)
Local verification used
cd packages/web-shell && npx vitest run client/components/sidebar/WebShellSidebar.test.tsx,cd packages/web-shell && npm run build, and a local Vite web-shell smoke against the running daemon.Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
这个 PR 让 Web Shell 侧边栏底部在宽度受限时按阶段降级:先把设置入口变成纯图标,再在更窄时隐藏版本号,继续向左拖过紧凑阈值后把侧边栏折叠成 rail,并保留一个正常的展开宽度供下次展开使用。
为什么需要
我在本地启动 Web Shell 时发现侧边栏底部宽度不够:“设置”两个字被遮挡,设置图标只展示了一半;继续缩小侧边栏时,原本应该保留可操作的收起/紧凑态按钮也会变得不可见。这个分阶段降级方案能保持按钮可访问,避免底部操作被裁切,同时不改变 daemon 或 session 行为。
Reviewer Test Plan
如何验证
打开启用侧边栏的 Web Shell,把视口调到足够显示会话总览和分屏这两个底部入口,然后把侧边栏向窄处拖动。确认“设置”文字会先消失,版本号随后在更窄宽度下消失,剩余图标在紧凑宽度下不重叠;继续向左拖动时侧边栏会折叠为 rail。再次展开后确认宽度恢复到正常支持的宽度,而不是拖拽过程中的阈值以下宽度。
证据(Before & After)
Before:本地启动后可以看到侧边栏底部过于拥挤,“设置”文字被遮挡,设置齿轮只显示一半,并且缩小侧边栏时底部按钮可能直接不可见。After:本地浏览器 smoke 在 260px 和 220px 侧边栏宽度下测得底部无重叠,确认拖过阈值会折叠到 56px rail,并确认再次展开会恢复到 220px。
测试平台
环境(可选)
本地验证使用了
cd packages/web-shell && npx vitest run client/components/sidebar/WebShellSidebar.test.tsx、cd packages/web-shell && npm run build,以及连接本地运行 daemon 的 Vite Web Shell 浏览器 smoke。风险与范围
关联 Issue
N/A