fix(web-shell): use theme color for @ group titles - #6291
Closed
BZ-D wants to merge 7 commits into
Closed
Conversation
added 6 commits
July 2, 2026 21:14
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: The stated problem — group-title colors being hard to read in dark theme — is real, and the CSS fix ( The PR title says "use theme color for @ group titles" — a one-line CSS fix. But the diff is 856 lines across 25 files, and the bulk of it is an entirely new feature: MCP server mentions in the
The CSS fix is ~5 lines. The other ~850 lines are a separate feature that the PR description doesn't mention at all. These should be in separate PRs — the color fix can ship immediately, and the MCP server mention feature deserves its own review with a proper description and test plan. Direction: MCP server mentions in the Approach: I'd strongly recommend splitting this into two PRs:
Flagging these for discussion before diving deeper. Should I proceed with code review on the full diff as-is, or would you prefer to split first? 中文说明感谢贡献！ 模板完整 ✓ 问题： 标题说的是"修复分组标题颜色"——一行 CSS 改动。但实际 diff 有 856 行、25 个文件，其中大部分是一个全新功能：在
CSS 修复大约 5 行，其余约 850 行是一个独立的功能，PR 描述中完全没有提及。这两部分应该拆成两个 PR——颜色修复可以立刻合并，MCP 服务器 mention 功能应该有自己独立的 PR、描述和测试计划。 方向： 在 方案： 强烈建议拆分为两个 PR：
先提出来讨论，再深入看代码。是继续对完整 diff 做代码审查，还是先拆分？ — Qwen Code · qwen3.7-max |
Collaborator
|
@qwen-code /resolve |
…itle-color # Conflicts: # packages/web-shell/client/components/Editor.module.css # packages/web-shell/client/hooks/useComposerCore.ts
Collaborator
|
Qwen Code resolved the merge conflicts, but the head branch changed while resolving, so the update was not pushed. Re-run /resolve. The resolved diff is attached as the Merge Conflict Resolution Summary — PR #6291Branches
ConflictsTwo files had identical conflicts:
What conflictedBoth conflicts were in the color: var(--muted-foreground, #a1a1aa) !important;while How resolvedKept the PR branch's addition ( Verification
|
Collaborator
Code ReviewIndependent proposal: If asked to add MCP server mentions to the
Comparison with the PR: The implementation closely matches my proposal. The MCP server mention system mirrors the extension mention pattern cleanly — No critical blockers found. Specific observations:
Minor issues (non-blocking):
TestingUnit tests — all pass:
Real-scenario testing — limited. The web-shell UI changes (completion menu section headers, composer tag icons, inline tag rendering) require browser testing against a running web-shell instance with configured MCP servers. This can't be validated in a terminal tmux session. The reviewer test plan in the PR covers the right steps (open web-shell in dark theme, trigger — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: the code here is well-implemented. The MCP server mention system cleanly mirrors the existing extension mention pattern, the ordering in But the PR as packaged has a real problem: the title and description describe a 5-line CSS fix, while the diff delivers an 856-line feature. The MCP server mention system — with session integration, a new utility module, SVG icons, composer tag rendering, and i18n changes — is a full feature that deserves its own PR with an honest title, a description that explains what it does, and a test plan for the This isn't about splitting for the sake of splitting. It's about:
My recommendation: split into two PRs. The CSS fix (section header colors + icon styling) ships immediately — it's small, safe, and clearly correct. The MCP server mention feature gets its own PR with proper framing. Requesting changes on scope organization, not on code quality. 中文说明退一步来看：代码本身实现得很好。MCP 服务器 mention 系统干净地复用了已有的 extension mention 模式， 但 PR 的打包方式有一个实际问题：标题和描述说的是一行 CSS 修复，但 diff 交付的是一个 856 行的功能。 MCP 服务器 mention 系统——包含会话集成、新的工具模块、SVG 图标、编辑器标签渲染和国际化变更——是一个完整的功能，应该有自己独立的 PR、准确的标题、说明功能的描述、以及 这不是为了拆分而拆分。这关系到：
建议： 拆分为两个 PR。CSS 修复（section header 颜色 + 图标样式）可以立即合并——小且安全。MCP 服务器 mention 功能独立成 PR。 基于范围组织请求修改，不是基于代码质量。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
requested changes
Jul 4, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Scope mismatch between title and diff — the title says "theme color fix" but the diff delivers a full MCP server mention feature (856 lines, 25 files). Code quality is good and all tests pass, but the PR should be split: CSS fix ships immediately, MCP server mention feature gets its own PR with proper framing. See comments above for details. 🙏
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.
[Suggestion] Merge commit scope mismatch
The merge commit cba5b2553 introduces 17 unrelated files into this PR's diff (mobile-mcp formatting from a separate branch, run-qwen-serve.ts indent change, pr-workflow.md blank line). The authored portion is only 2 lines in 2 web-shell files, but GitHub shows 19 files / 3,174 lines changed.
This obscures the actual change, complicates git blame/bisect, and sets a precedent for hiding changes in merge noise.
Suggested fix: Rebase on origin/main and force-push to collapse the diff to only the 2 authored files:
git fetch origin main
git rebase origin/main
git push --force-with-lease— qwen3.7-max via Qwen Code /review
| margin: 6px 10px 4px; | ||
| padding: 2px 0 4px !important; | ||
| line-height: 1.2; | ||
| color: var(--muted-foreground, #a1a1aa) !important; |
Collaborator
There was a problem hiding this comment.
[Suggestion] The hardcoded fallback #a1a1aa is inconsistent with other --muted-foreground fallbacks in this package (#a0aec0 in inputHighlight.ts, #888 in SystemMessage.module.css). The dominant pattern across the web-shell codebase is to use no fallback at all, since the variable is always defined via the theme CSS and synced to tooltip portals.
Suggested change
| color: var(--muted-foreground, #a1a1aa) !important; | |
| color: var(--muted-foreground) !important; |
— qwen3.7-max via Qwen Code /review
| margin: 6px 10px 4px; | ||
| padding: 2px 0 4px !important; | ||
| line-height: 1.2; | ||
| color: var(--muted-foreground, #a1a1aa) !important; |
Collaborator
There was a problem hiding this comment.
[Suggestion] Same as Editor.module.css:432 — consider dropping the fallback to match the package convention:
Suggested change
| color: var(--muted-foreground, #a1a1aa) !important; | |
| color: var(--muted-foreground) !important; |
— qwen3.7-max via Qwen Code /review
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
4 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 fixes the group-title color used in the web-shell
@completion menu so the section headers remain readable in dark theme.Instead of relying on an implicit/default color, the section header now uses the shared muted foreground theme token, which keeps the visual hierarchy while avoiding near-black text on dark backgrounds.
Why it's needed
In dark theme, the group titles in the web-shell
@menu could render with a color that was too close to the background, making section headers such as extensions and files difficult to read.Using an explicit theme variable makes the appearance consistent across themes and avoids accidental regressions from browser or inherited default colors.
Reviewer Test Plan
How to verify
@completion.Evidence (Before & After)
Before: group titles in the web-shell
@completion menu could appear too dark to read comfortably in dark theme.After: group titles use the muted foreground theme color and remain visible while preserving the intended visual hierarchy.
Tested on
Environment (optional)
Local web-shell build.
Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
这个 PR 修复了 web-shell 中
@补全菜单分组标题的颜色，使这些 section header 在暗色主题下仍然清晰可读。这次改动不再依赖隐式/默认颜色，而是显式使用共享的 muted foreground 主题变量，从而在保留层级感的同时，避免暗色背景下出现接近黑色的文字。
Why it's needed
在暗色主题下，web-shell
@菜单中的分组标题颜色可能过于接近背景色，导致像 extensions、files 这样的 section header 难以辨认。改为使用显式的主题变量后，外观会在不同主题下保持一致，也能避免由于浏览器默认色或继承色导致的意外回归。
Reviewer Test Plan
How to verify
@补全。Evidence (Before & After)
Before：web-shell
@补全菜单中的分组标题在暗色主题下可能过暗，不利于阅读。After：分组标题改为使用 muted foreground 主题色，在保留视觉层级的同时能清晰显示。
Tested on
Environment (optional)
本地 web-shell build。
Risk & Scope
Linked Issues
N/A