feat(web-shell): add 'voice' to ComposerToolbarAction for external visibility control - #5947
Conversation
|
Thanks for the PR! Template looks good ✓ On direction: this is a natural extension of the existing On approach: the diff is exactly the minimal change — one union member added, one existing Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是对现有 方案:diff 恰好是最小改动——联合类型加一个成员,用已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: add The diff is clean: one union member, one conditional wrapper, no filler. The No blockers found. TestingThis is a programmatic API change (no TUI-visible difference), so testing focused on build + unit tests. VoiceButton unit testsProject buildBuild exits cleanly. The 15 lint warnings are all pre-existing TypecheckStandalone 中文说明代码审查独立方案:在 Diff 干净:一个联合成员、一个条件包裹、无多余改动。 未发现阻塞问题。 测试这是一个程序化 API 变更(无 TUI 可见差异),因此测试聚焦于构建和单元测试。 VoiceButton 单元测试:3/3 通过 ✅。项目构建成功(0 错误,15 个 lint 警告均为 vscode-ide-companion 中已有的 — Qwen Code · qwen3.7-max |
|
This is the kind of PR that's easy to approve: it does one thing, does it the obvious way, and doesn't try to be more than it needs to be. The problem is real — embedders of Going back to my independent proposal from Stage 2: the PR matches it exactly. I can't think of a simpler path that solves the stated problem. Build passes, tests pass, backward compat holds (omitting the prop still shows voice). Nothing to object to. Approving. ✅ 中文说明这是一个容易批准的 PR:只做一件事,用最显而易见的方式,没有多余的企图。 问题是真实的—— 回顾第二阶段的独立方案:PR 与之完全吻合。想不出更简单的路径来解决所陈述的问题。 构建通过,测试通过,向后兼容成立(不传 prop 时语音按钮照常显示)。没有可反对的地方。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
left a comment
There was a problem hiding this comment.
No issues found. voice is gated with the same showToolbarAction() pattern as the other five toolbar actions, and the default path (no composerToolbarActions prop → visibleActionSet is null → returns true) still renders the voice button, so the change is backward compatible. The gating mechanism (visibleToolbarActions/composerToolbarActions) landed only days ago in #5775 and had no external consumers, so completing it for voice carries no migration risk. JSX nesting and cross-file usage (App.tsx passthrough) check out.
Downgraded from Approve to Comment: CI still running. — claude-opus-4-8[1m] via Qwen Code /qreview
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. Downgraded from Approve to Comment: CI still running.
The change is minimal and correct — adds 'voice' to the ComposerToolbarAction union and wraps VoiceButton with the existing showToolbarAction() gate, following the identical pattern used by all 5 other toolbar actions. Default behavior (no composerToolbarActions prop → voice button visible) is preserved. Build and tests pass. LGTM ✅
— qwen3.7-max via Qwen Code /review
✅ Maintainer verification — real browser test of the voice toolbar gate (macOS)Verdict: LGTM, behaviour confirmed. Against a live Environment
The standalone entry doesn't expose 1. Tests & types (PR head)
2. Real browser — gating matrix (8/8 checks pass)Counts are live
Visually confirmed in captures: the default composer shows the 🎤 alongside Ask-Approval / model / 3. Code observations (verified)
Notes / not covered
🇨🇳 中文版(完整对应)✅ 维护者验证 —— 语音工具栏门控真实浏览器测试(macOS)结论:LGTM,行为已确认。 在 macOS 上针对真实 环境
standalone 入口不通过 URL 暴露 1. 测试与类型(PR head)
2. 真实浏览器 —— 门控矩阵(8/8 通过)计数为实时
截图中已目视确认:默认 composer 在 Ask-Approval / model / 3. 代码观察(已核实)
说明 / 未覆盖
🤖 Verified locally on macOS with a real |
| core.focus(); | ||
| }} | ||
| /> | ||
| {showToolbarAction('voice') && ( |
There was a problem hiding this comment.
[Suggestion] Silent behavior change for embedders with an explicit composerToolbarActions allowlist. Before this PR, VoiceButton rendered unconditionally — 'voice' wasn't in the union, so no consumer could include it in their allowlist. After this change, any embedder passing an explicit list (e.g., ['approvalMode', 'model', 'commands', 'files', 'widthMode']) silently loses the voice button on upgrade — no TypeScript error, no runtime warning.
The PR describes this as "fully backward compatible," which is true for the default path (no prop → all actions show), but not for explicit allowlist consumers. Consider adding a CHANGELOG entry or migration note flagging this behavioral shift, so integrators know to add 'voice' to their existing arrays.
— qwen3.7-max via Qwen Code /review
What this PR does
Adds
voiceas a new member of theComposerToolbarActionunion type and wraps theVoiceButtonrendering with the existingshowToolbarAction()gate. This allows external consumers of the web-shell component to control whether the voice dictation button is visible, using the samecomposerToolbarActionsprop already used for other toolbar actions.Why it's needed
When embedding the web-shell as a component, external consumers currently have no way to hide the voice input button. The only existing visibility control is the daemon-side
voice_transcribecapability check, which is not accessible from the embedding application. By addingvoicetoComposerToolbarAction, consumers can now hide the voice button simply by omitting it from thecomposerToolbarActionsarray — consistent with howapprovalMode,model,commands,files, andwidthModeare already controlled.Reviewer Test Plan
How to verify
npm run build<WebShell />withoutcomposerToolbarActionsprop — voice button should still appear (backward compatible, default behavior unchanged)<WebShell composerToolbarActions={['approvalMode', 'model', 'commands', 'files']} />— voice button should be hidden<WebShell composerToolbarActions={['voice']} />— only voice button should appear in the toolbar areacd packages/web-shell && npx vitest run client/voice/VoiceButton.test.tsxEvidence (Before & After)
N/A — this is a programmatic API change; no visual difference unless the consumer explicitly excludes
voice.Tested on
Environment (optional)
npm run buildandnpx vitest runon macOS.Risk & Scope
composerToolbarActionsstill shows all actions including voice.Linked Issues
N/A
中文说明
本 PR 做了什么
在
ComposerToolbarAction联合类型中新增voice成员,并用已有的showToolbarAction()门控包裹VoiceButton的渲染。这使得 web-shell 组件的外部消费者可以通过已有的composerToolbarActionsprop 来控制语音输入按钮的显隐。为什么需要这个改动
当以组件形式嵌入 web-shell 时,外部消费者目前无法隐藏语音输入按钮。唯一的显隐控制是 daemon 端的
voice_transcribecapability 检查,嵌入方无法触达。将voice加入ComposerToolbarAction后,消费者只需在composerToolbarActions数组中省略它即可隐藏——与approvalMode、model、commands、files、widthMode的控制方式一致。风险与范围
composerToolbarActions时仍然显示所有操作按钮(包括语音)。