Skip to content

feat(web-shell): add mobile responsive view for TodoPanel - #5948

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
chiga0:feat/web-shell-todo-mobile
Jun 28, 2026
Merged

feat(web-shell): add mobile responsive view for TodoPanel#5948
wenshao merged 1 commit into
QwenLM:mainfrom
chiga0:feat/web-shell-todo-mobile

Conversation

@ytahdn

@ytahdn ytahdn commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds responsive behavior to the TodoPanel's progress summary. On desktop (≥600px viewport width), the panel shows the full text like Step 3 / 8 (EN) or 第 3 / 8 步 (ZH). On mobile (<600px), it switches to a compact format showing only the fraction 3 / 8 alongside the progress ring, reducing horizontal space usage.

Why it's needed

The TodoPanel summary text (Step 3 / 8 or 第 3 / 8 步) is too long for narrow mobile viewports, causing the pill to wrap or overflow. The label (Step / ) is redundant on small screens where space is limited — the progress ring plus fraction already conveys the same information clearly.

Reviewer Test Plan

How to verify

  1. Build the web-shell package: npm run build
  2. Open web-shell in a browser with a viewport ≥600px — the TodoPanel should show ◐ Step 3 / 8 (EN) or ◐ 第 3 / 8 步 (ZH), same as before
  3. Resize the viewport to <600px — the TodoPanel should show only ◐ 3 / 8, with smaller padding
  4. Resize back to ≥600px — the full text should reappear
  5. Verify the detail tooltip (hover/focus) still works correctly on both viewports

Evidence (Before & After)

N/A — responsive CSS change, behavior depends on viewport width.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

npm run build on macOS.

Risk & Scope

  • Main risk or tradeoff: None — purely a CSS media query + i18n addition. Desktop behavior is unchanged.
  • Not validated / out of scope: No changes to the detail tooltip or task list rendering.
  • Breaking changes / migration notes: None.

Linked Issues

N/A

中文说明

本 PR 做了什么

为 TodoPanel 的进度摘要添加响应式行为。桌面端(视口宽度 ≥600px)显示完整文本,如 Step 3 / 8(英文)或 第 3 / 8 步(中文)。移动端(<600px)切换为紧凑格式,仅显示进度圆环和分数 3 / 8,减少水平空间占用。

为什么需要这个改动

TodoPanel 的摘要文本(Step 3 / 8第 3 / 8 步)在窄屏移动设备上过长,导致胶囊条换行或溢出。标签(Step / )在空间有限的小屏幕上属于冗余信息——进度圆环加分数已经足够清晰地传达相同信息。

风险与范围

  • 主要风险:无——纯 CSS 媒体查询 + i18n 新增,桌面端行为不变。
  • 未验证/不在范围内:未修改详情弹窗和任务列表的渲染。
  • 破坏性变更/迁移说明:无。

- Desktop (>=600px): show full text 'Step 3 / 8' or '第 3 / 8 步'
- Mobile (<600px): show compact text '3 / 8' with progress ring only
- Add todo.stepFraction i18n key for both EN and ZH
- Reduce summary padding on mobile for better space usage
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual body, tested-on table filled, risk & scope filled in.

On direction: this is a tight, well-scoped mobile UX fix for the web-shell TodoPanel — the summary pill wraps/overflows on narrow viewports because the Step 3 / 8 / 第 3 / 8 步 label is redundant when the progress ring already conveys the same info. Real user-facing annoyance, and the fix stays inside the existing component rather than reaching for something heavier. The web-shell has a scope/web-shell label and the change is squarely in that area. No CHANGELOG precedent needed for a CSS-only responsive tweak, but the area (TodoPanel UX) is clearly within scope.

On approach: the scope feels right — three files, ~30 lines, one concern. Media-query toggle between a full-text span and a compact fraction span is the simplest path; no JS resize observers, no new abstractions, no build-time machinery. Diff carries no drive-by refactors and no scope creep. One small question for the contributor to think about: the breakpoint 599px is unique across the web-shell — the rest of the codebase clusters around 700 / 720 / 760px. Not a blocker, but worth a one-line comment in the CSS (or a CSS custom property for the breakpoint) explaining why 599px specifically, so future maintainers don't normalize it and accidentally regress the layout. Accessibility looks fine — .summary already carries aria-label={t('todo.stepProgress', ...)}, so screen readers read the full text regardless of which inner span is visually rendered.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必填项齐全,中英双语正文,tested-on 表格已填写,风险与范围已说明。

方向:这是一个针对 web-shell TodoPanel 的紧凑、聚焦的移动端 UX 修复 —— 摘要胶囊条在窄屏上会换行/溢出,因为 Step 3 / 8 / 第 3 / 8 步 标签在有进度圆环的情况下属于冗余信息。解决了真实的用户痛点,且修复没有引入更重的方案,完全在现有组件内完成。web-shell 有 scope/web-shell 标签,本改动完全在该范围内。纯 CSS 响应式调整不需要 CHANGELOG 先例,但 TodoPanel UX 这个区域明显属于项目范围。

方案:范围合理 —— 三个文件、约 30 行、只解决一个问题。用媒体查询在全量文本 span 和紧凑分数 span 之间切换,是最简路径;没有 JS resize observer,没有新抽象,没有构建期魔法。diff 没有顺手重构,也没有范围蔓延。一个值得贡献者思考的小问题:断点 599px 在整个 web-shell 里是独一份 —— 其余代码集中在 700 / 720 / 760px 附近。不是阻塞点,但建议在 CSS 里加一行注释(或把断点抽成 CSS 自定义属性)说明为什么是 599px,避免后续维护者把它"统一"掉而意外回退布局。可访问性没问题 —— .summary 已经有 aria-label={t('todo.stepProgress', ...)},无论哪个内部 span 被视觉渲染,屏幕阅读器都能读到完整文本。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review. My independent proposal before reading the diff: a CSS media query at ~600px that hides the Step / label and leaves only the fraction, paired with a new i18n key for the shorter form, keeping aria-label on the parent so screen readers still announce the full text. The PR's implementation matches this almost exactly — max-width: 599px (just under Material's sm=600px), two sibling spans toggled by display: none / display: inline, a new todo.stepFraction key in both EN and ZH, and aria-label preserved on .summary. No critical blockers.

A couple of small observations, none blocking:

  • 599px is the only breakpoint in the web-shell at that value (others cluster at 560 / 700 / 720 / 760). Worth a one-line CSS comment naming the source (Material sm minus 1) so future maintainers don't "normalize" it and regress the layout.
  • font-variant-numeric: tabular-nums on .compactText is a nice touch — keeps the fraction from jittering as digits change width.
  • Both spans stay in the DOM, so absent the parent aria-label a screen reader would read both; the parent label correctly overrides that. No a11y regression.

Reuse check: no new utility, no parallel helper, no duplicated logic — the change uses the existing t() function and the existing CSS module. Nothing to extract.

Testing — tmux not applicable. This is a CSS @media query change; tmux drives a terminal pane, not a browser viewport, so capture-pane output cannot demonstrate the responsive swap. The only meaningful verification is rendering the web-shell in a real browser and resizing across 600px, which is exactly what the reviewer test plan asks for. No build environment is installed in this worktree, but the diff touches only CSS module class names (no type surface) and a Record<string, MessageValue> i18n dict (any string key is valid), so no TypeScript regression is possible from these edits alone.

Reviewer test plan in the PR is solid — open web-shell, resize across 600px, confirm both viewports render correctly and the hover/focus tooltip still works. That's the right test, and it has to be run by a human in a browser.

中文说明

代码审查。 在查看 diff 前,我的独立方案:在 ~600px 处加 CSS 媒体查询,隐藏 Step / 标签只保留分数,并为紧凑格式新增一个 i18n key,父元素保留 aria-label 以保证屏幕阅读器仍能播报完整文本。PR 的实现与这个方案几乎一致 —— max-width: 599px(紧贴 Material sm=600px 之下),两个兄弟 span 通过 display: none / display: inline 切换,EN/ZH 都新增 todo.stepFraction.summary 上的 aria-label 保留。无关键阻塞问题。

几个非阻塞的小观察:

  • 599px 是 web-shell 中唯一在该值的断点(其他集中在 560 / 700 / 720 / 760)。建议在 CSS 加一行注释说明来源(Material sm 减 1),避免后续维护者"统一"断点而意外回退布局。
  • .compactText 上的 font-variant-numeric: tabular-nums 是个加分 —— 避免分数数字宽度变化时抖动。
  • 两个 span 都留在 DOM 里,若没有父级 aria-label 屏幕阅读器会把两者都读出来;父级 label 正确地覆盖了这一点,无可访问性回退。

复用性检查: 没有新工具函数、没有平行辅助代码、没有重复逻辑 —— 改动使用现有的 t() 函数和现有的 CSS module,无需提取。

测试 —— tmux 不适用。 这是 CSS @media 查询改动;tmux 驱动的是终端窗格而非浏览器视口,capture-pane 输出无法展示响应式切换。唯一有意义的验证是在真实浏览器里渲染 web-shell 并跨 600px 调整大小,这正是 reviewer test plan 要求的内容。本 worktree 未安装构建环境,但 diff 只涉及 CSS module 类名(无类型面)和 Record<string, MessageValue> i18n 字典(任意字符串 key 均合法),因此这些编辑本身不可能引入 TypeScript 回退。

PR 中的 reviewer test plan 合理 —— 打开 web-shell,跨 600px 调整大小,确认两种视口渲染正确且 hover/focus tooltip 仍工作。这是正确的测试方式,必须由人工在浏览器里执行。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Stepping back: the PR matches my independent proposal almost exactly, the diff is minimal and focused on a single user-visible concern, and the implementation doesn't try to be clever. No new abstraction, no scope creep, no drive-by refactors — just a media query, two i18n keys, and two JSX spans. The kind of PR you wish every mobile tweak looked like.

The one soft concern — a nonstandard 599px breakpoint without an explanatory comment — is minor enough that I'd rather see it addressed in a follow-up (or ignored if the maintainer doesn't care) than block this merge on it. Accessibility is preserved via the parent aria-label, the bilingual i18n is correctly added for both EN and ZH, and the reviewer test plan is the right one (manual browser resize across 600px — tmux cannot help here, and that's a property of the change, not a gap in the review).

Net: ships the fix cleanly, solves a real narrow-viewport annoyance in the TodoPanel, low risk to merge. Approving. ✅

中文说明

退一步看:PR 与我的独立方案几乎一致,diff 最小化且聚焦于单一用户可见问题,实现没有炫技。没有新抽象、没有范围蔓延、没有顺手重构 —— 就是一个媒体查询、两个 i18n key、两个 JSX span。希望每个移动端微调都能长这样。

唯一的柔性顾虑 —— 非标准的 599px 断点缺少解释性注释 —— 轻微到宁愿后续跟进处理(或维护者不在意就忽略),也不想因此阻塞合并。通过父级 aria-label 保持了可访问性,EN/ZH 双语 i18n 都正确添加,reviewer test plan 也正确(在浏览器里手动跨 600px 调整大小 —— tmux 帮不上忙,这是改动本身的性质,不是审查的缺口)。

结论:干净地交付了修复,解决了 TodoPanel 在窄视口下的真实烦恼,合并风险低。批准。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. Downgraded from Approve to Comment: CI still running.

Clean, minimal responsive CSS change (3 files, +31/-1). The media query toggle between full text and compact fraction is the simplest correct approach. Accessibility is well-handled via the parent aria-label. Build passes, all 538 web-shell tests green.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

✅ Real-browser verification — TodoPanel responsive toggle works in EN & ZH

This is a viewport-driven CSS change, so jsdom/vitest can't see it — I rendered the actual <TodoPanel> + the real CSS module in real Chromium (Playwright) via the web-shell Vite server, and read the computed display of the full/compact spans, the visible text, and the summary padding at several widths. The @media (max-width: 599px) rule keys off the viewport (not a container), so an isolated mount reproduces the in-app behavior exactly.

Verdict: PASS — every breakpoint, both languages, plus a mutation A/B proving the behavior is this PR's.

  • Head verified: 3bf6e696eaa5e4a73084af79471a25be07c428b2 (matches PR head)
  • Build: npm run build → exit 0. Lint: eslint on the two .ts/.tsx files → 0 errors.

What I observed (real Chromium, computed styles)

Viewport Lang .fullText .compactText Visible summary text .summary padding
800px (desktop) EN inline none Step 3 / 8 8px 18px
400px (mobile) EN none inline 3 / 8 6px 12px
800px (desktop) ZH inline none 第 3 / 8 步 8px 18px
400px (mobile) ZH none inline 3 / 8 6px 12px
600px (boundary) EN inline none Step 3 / 8 8px 18px
599px (boundary) EN none inline 3 / 8 6px 12px

The breakpoint is exactly where the CSS says: ≥600px → full text, ≤599px → compact N / M + tighter padding. Screenshots captured locally confirm the visual: a desktop ◐ Step 3 / 8 pill vs a compact ◐ 3 / 8 pill.

Mutation A/B (proves it's this PR)

Reverting the 3 files to base and re-running: at 400px the base panel still renders the full Step 3 / 8 with desktop padding 8px 18px (no compact span exists) — i.e. exactly the mobile overflow this PR fixes. With the PR applied, 400px → 3 / 8. Restored to PR head, all 6 checks pass again.

Notes for the reviewer

  • Accessibility preserved: the .summary aria-label uses todo.stepProgress (the full text) at every viewport (TodoPanel.tsx:48), so screen-reader users still hear "Step 3 / 8" / "第 3 / 8 步" even when the visible text is the compact 3 / 8. The compaction is visual-only.
  • ✅ Both new i18n keys resolve: todo.stepFraction is added to EN and ZH (both → N / M); desktop strings are unchanged.
  • ✅ Purely additive — two spans + one i18n key + one media query. No existing TodoPanel component test asserts on the summary text (the only todo tests cover todos.ts and the separate TodoView), so the extra span can't break a textContent assertion. font-variant-numeric: tabular-nums on the compact span keeps the fraction digits aligned.
  • BLOCKED = REVIEW_REQUIRED (awaiting a maintainer approval), not a CI failure. macOS (Darwin, Node 22, Chromium via Playwright).
🇨🇳 中文版本

✅ 真实浏览器验证 —— TodoPanel 响应式切换在中英文下均正常

这是依赖视口宽度的 CSS 改动,jsdom/vitest 看不到,所以我用 真实 Chromium(Playwright) 通过 web-shell 的 Vite 服务渲染了真实的 <TodoPanel> + 真实 CSS module,在多个宽度下读取 full/compact 两个 span 的计算 display、可见文本和 summary 内边距。@media (max-width: 599px) 是按视口(而非容器)生效的,所以隔离挂载能精确复现 app 内行为。

结论:PASS —— 每个断点、两种语言全部正确,并用变异 A/B 证明该行为来自本 PR。

  • 验证的 head: 3bf6e696e…(与 PR head 一致)
  • 构建: npm run build → exit 0。Lint: 两个 .ts/.tsx 文件 eslint → 0 error。

观察结果(真实 Chromium,计算样式)

视口 语言 .fullText .compactText 可见摘要文本 .summary 内边距
800px(桌面) EN inline none Step 3 / 8 8px 18px
400px(移动) EN none inline 3 / 8 6px 12px
800px(桌面) ZH inline none 第 3 / 8 步 8px 18px
400px(移动) ZH none inline 3 / 8 6px 12px
600px(边界) EN inline none Step 3 / 8 8px 18px
599px(边界) EN none inline 3 / 8 6px 12px

断点正好在 CSS 规定处:≥600px → 完整文本,≤599px → 紧凑 N / M + 更小内边距。本地截图也确认了视觉效果:桌面 ◐ Step 3 / 8 胶囊 vs 紧凑 ◐ 3 / 8 胶囊。

变异 A/B(证明来自本 PR)

把 3 个文件 revert 到 base 后重跑:在 400px 下 base 面板仍渲染完整 Step 3 / 8 且用桌面内边距 8px 18px(不存在 compact span)—— 正是本 PR 要修的移动端溢出问题。打上 PR 后 400px → 3 / 8。还原到 PR head 后 6 项再次全过。

给 reviewer 的说明

  • 可访问性保留: .summaryaria-label 在所有视口都用 todo.stepProgress完整文本,TodoPanel.tsx:48),所以即使可见文本是紧凑的 3 / 8,屏幕阅读器用户仍能听到 "Step 3 / 8" / "第 3 / 8 步"。紧凑化只是视觉层面。
  • ✅ 两个新 i18n key 都能解析:todo.stepFractionEN 和 ZH 都加了(都 → N / M);桌面字符串不变。
  • ✅ 纯增量改动 —— 两个 span + 一个 i18n key + 一条媒体查询。没有现有 TodoPanel 组件测试断言摘要文本(仅有的 todo 测试覆盖 todos.ts 和另一个 TodoView 组件),所以多出的 span 不会打断 textContent 断言。紧凑 span 上的 font-variant-numeric: tabular-nums 让分数数字对齐。
  • BLOCKED = REVIEW_REQUIRED(等维护者 approve),非 CI 失败。环境 macOS(Darwin, Node 22, Playwright 驱动 Chromium)。

@wenshao
wenshao added this pull request to the merge queue Jun 28, 2026
font-variant-numeric: tabular-nums;
}

@media (max-width: 599px) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The 599px breakpoint is unique to this file. Every other responsive breakpoint in web-shell/client/components/ uses one of four values: 560px (DialogShell), 700px (StreamingStatus, StatusBar, Editor), 720px (StatusMessage, ContextUsageMessage, HelpDialog), or 760px (WebShellSidebar, WelcomeHeader, ToolApproval). At viewport widths 600–699px, the rest of the web-shell UI has already collapsed into its mobile layout, but TodoPanel still renders the full "Step 3 / 5" text — making it the last component to adapt.

Consider aligning with an existing breakpoint (e.g., 560px from DialogShell, which also handles narrow panel widths), or adding a brief comment explaining why 599px was chosen intentionally.

Suggested change
@media (max-width: 599px) {
@media (max-width: 560px) {

— qwen3.7-max via Qwen Code /review

Merged via the queue into QwenLM:main with commit c458830 Jun 28, 2026
52 checks passed

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. Downgraded from Approve to Comment: CI still running.

Clean, minimal responsive CSS change (3 files, +31/-1). The dual-span full/compact toggle via CSS media query is the simplest correct approach. Both EN and ZH locales covered. Accessibility preserved — the pre-existing aria-label on .summary gives screen readers the full text regardless of which visual variant is shown. Build and all 538 tests pass.

— qwen3.7-max via Qwen Code /review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants