fix(web-shell): render a plain textarea composer on touch devices - #7587
Conversation
Mobile browsers could not type into the Web Shell composer (QwenLM#5958): CodeMirror's contenteditable interacts poorly with virtual keyboards, and three non-gesture view.focus() calls claim activeElement on iOS without opening the keyboard, after which taps may never refocus the editor. On touch devices ('(hover: none) and (pointer: coarse)' plus maxTouchPoints > 0 — touch laptops keep the desktop editor) useComposerCore now skips creating an EditorView entirely and exposes a mobileComposer backend that ChatEditor renders as a controlled <textarea> at the same mount point. The internal submit pipeline was hoisted out of the editor-creation effect and accepts view: EditorView | null, so history, prompt building, tags, images, and slash/! text interpretation are shared unchanged between both backends. Enter inserts a newline natively; submission goes through the Send button. Programmatic (non-gesture) focus is additionally suppressed on coarse-pointer devices even when CodeMirror is forced, and ?composer=textarea|codemirror serves as a debugging and rollback escape hatch. The choice is frozen at mount so a mid-session flip cannot drop the draft. Known textarea-backend degradations (commands still work as typed text): no slash/@ completion menus, no inline tag chips (fall back to the top placement), no history arrow navigation, no large-paste placeholders, and no followup Tab-accept. Fixes QwenLM#5958 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with clear reproduction — issue #5958 documents that on iOS Safari and Android Chrome, tapping the CodeMirror-based composer doesn't reliably open the virtual keyboard and typed characters can be dropped. The issue includes step-by-step reproduction and is labeled Direction: aligned. This is part of the ongoing web-shell mobile effort (#6000 sidebar, #6584 welcome slots, #5948 TodoPanel), and the linked issue explicitly suggests "fallback to a native Size: not applicable — all changes are in Approach: the scope feels right. Rather than building a parallel mobile composer, the PR hoists the existing submit pipeline to accept Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有明确复现——issue #5958 记录了在 iOS Safari 和 Android Chrome 上，点击基于 CodeMirror 的输入框不能可靠弹出虚拟键盘，且输入的字符可能丢失。issue 包含逐步复现步骤，并被维护者标记为 方向：对齐。这是 web-shell 移动端适配工作的一部分（#6000 侧边栏、#6584 欢迎位、#5948 TodoPanel），关联 issue 明确建议"在触屏设备上回退到原生 规模：不适用——所有改动在 方案：范围合理。PR 没有另建一条并行的移动端输入框，而是将现有提交管道提升为接受 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal vs. actual approach: before reading the diff, I'd have done essentially the same thing — a What's done well: The submit pipeline hoist is the key structural change and it's clean. The The No critical blockers found. The implementation follows project conventions (ESM, no One observation (not a blocker): the Test ResultsVitest (58 tests — touch detection, mobile textarea backend, desktop suites)Playwright mobile-chromium (Pixel 7 emulation — 5 e2e tests)Tests: textarea renders instead of CodeMirror; tap → type → Send round-trips through mock daemon with SSE reply; Enter inserts newline without submitting; Playwright desktop chromium (regression check)TypeScript typecheck中文说明代码审查独立方案 vs. 实际方案： 在读 diff 之前，我的方案几乎完全一致—— 做得好的地方： 提交管道的提升是关键的结构变更，做得很干净。
未发现关键阻塞问题。 实现遵循项目规范（ESM、无 一个观察（非阻塞）： 测试结果Vitest（58 个测试——触屏检测、移动 textarea 后端、桌面套件）4 个文件全部通过，58 个测试全部通过。 Playwright mobile-chromium（Pixel 7 仿真——5 个 e2e 测试）5 个全部通过。测试覆盖：textarea 渲染而非 CodeMirror；tap → 输入 → Send 经 mock daemon 往返并流式回显；Enter 换行不提交；纯文本 Playwright desktop chromium（回归检查）20 个通过，1 个 flaky（已有的："grows long text to the responsive composer cap at 600px @smoke"）。 TypeScript 类型检查
— Qwen Code · qwen3.8-max-preview Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is what a well-executed fix looks like. The problem is real and well-documented (#5958 — composer unusable on mobile browsers, labeled The implementation is straightforward and every change earns its place. The submit pipeline hoist is the key structural move and it's done cleanly — Tests are thorough: 58 vitest tests covering detection, the textarea backend, focus gating with desktop negative controls, and the untouched desktop suites; 5 Playwright e2e tests under Pixel 7 emulation exercising the full tap → type → Send → SSE round-trip; desktop e2e regression clean (20 passed, 1 pre-existing flaky). TypeScript typecheck clean. If I had to maintain this in six months, I'd thank the author — clear comments explaining why (iOS focus behavior, 16px font-size requirement), a single submit pipeline instead of two diverging paths, and an escape hatch for debugging. 中文说明置信度：5/5 —— 每个阶段都干净；毫不犹豫地合并。 这是一个执行良好的 fix 的样子。问题真实且有充分文档（#5958——输入框在移动浏览器上不可用，被维护者标记为 实现直接了当，每处改动都有其必要性。提交管道的提升是关键的结构变更，做得很干净—— 测试全面：58 个 vitest 测试覆盖检测、textarea 后端、focus 门控及桌面负向控制、以及未改动的桌面套件；5 个 Playwright e2e 测试在 Pixel 7 仿真下验证完整的 tap → 输入 → Send → SSE 往返；桌面 e2e 回归干净（20 通过，1 个已有 flaky）。TypeScript 类型检查干净。 如果六个月后我来维护这段代码，我会感谢作者——清晰的注释解释为什么（iOS focus 行为、16px 字号要求）、单一提交管道而非两条分叉路径、以及用于调试的逃生口。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Evidence — Playwright Pixel 7 emulation (Linux, repo mock daemon; per the Tested-on table, real virtual-keyboard verification on iOS Simulator / Android Emulator will follow as additional evidence):
The desktop CI visuals workflow should report no visual change for this PR — the CodeMirror path is untouched on fine-pointer devices, which doubles as a regression control. 中文以上截图来自 Playwright Pixel 7 仿真 + 仓库自带 mock daemon。第一张为逃生口强制的 CodeMirror 旧路径（对照），后三张为 textarea 新后端（明/暗主题、完整提交往返）。桌面 CI 视觉流水线预期显示「无视觉变化」——细指针设备上 CodeMirror 路径未动，可作为回归对照。iOS Simulator / Android Emulator（真实虚拟键盘栈）的验证结果将作为后续证据补充。 |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
— qwen3.7-max via Qwen Code /review
| min-height: var(--chat-editor-input-min-height, 44px); | ||
| max-height: var(--chat-editor-input-max-height, 300px); |
There was a problem hiding this comment.
[Suggestion] max-height: 300px is dead CSS — no mechanism grows the textarea between min-height (44px) and max-height. The textarea has rows={1}, resize: none, and no JavaScript auto-grow (scrollHeight/autoGrow absent from the entire diff). A mobile user typing a multi-line message sees only ~1.5 lines at a time with internal scrolling.
Consider adding an auto-grow effect in handleMobileChange or a useEffect watching mobileText:
const el = mobileTextareaRef.current;
if (el) {
el.style.height = 'auto';
el.style.height = Math.min(el.scrollHeight, 300) + 'px';
}— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in ed85222. Added an auto-grow effect watching the draft: height = min(scrollHeight, computed max-height), so --chat-editor-input-max-height overrides stay authoritative and the CSS cap is no longer dead. The mobile e2e spec now asserts the textarea's bounding box grows across newlines.
中文：已在 ed85222 修复——新增随草稿变化的 auto-grow effect，上限取计算样式的 max-height（CSS 变量覆盖仍生效）；移动 e2e 增加了多行输入后高度增长的断言。
| const setMobileText = useCallback((text: string) => { | ||
| mobileTextRef.current = text; | ||
| setMobileTextState(text); | ||
| }, []); |
There was a problem hiding this comment.
[Suggestion] setMobileText does not fire onInputTextChangeRef, unlike the CodeMirror equivalent where every document change triggers onInputTextChangeRef.current?.(text) via the updateListener. — Failure scenario: setMobileText is called from restoreSelectedHistoryMatch (history restore), commitAccepted (post-submit clear), setText, insertText, replaceEditorText, and clear. After a mobile history restore, the parent's composerTextRef.current retains stale text, so useNewSessionSuggestion operates on outdated input.
| const setMobileText = useCallback((text: string) => { | |
| mobileTextRef.current = text; | |
| setMobileTextState(text); | |
| }, []); | |
| const setMobileText = useCallback((text: string) => { | |
| mobileTextRef.current = text; | |
| setMobileTextState(text); | |
| onInputTextChangeRef.current?.(text); | |
| }, []); |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in ed85222, exactly as suggested — setMobileText now fires onInputTextChangeRef, and handleMobileChange delegates to it so there is a single notification path. Added a unit test asserting both programmatic setText and the post-submit clear notify the parent.
中文：已按建议在 ed85222 修复——setMobileText 统一上报 onInputTextChange，handleMobileChange 委托之；新增单测覆盖程序化 setText 与提交后清空两条通知路径。
| const start = el ? el.selectionStart : current.length; | ||
| const end = el ? el.selectionEnd : current.length; | ||
| setMobileText(current.slice(0, start) + text + current.slice(end)); |
There was a problem hiding this comment.
[Suggestion] The mobile insertText path does not restore the cursor position after inserting text, so the caret jumps to the end of the textarea value. — Failure scenario: if insertText is called while the caret is in the middle of the text (e.g., a tag chip insertion at the cursor), the value is correct but the caret jumps to the end. The CodeMirror path explicitly sets selection: { anchor: selection.from + insert.length }; the mobile path has no counterpart.
| const start = el ? el.selectionStart : current.length; | |
| const end = el ? el.selectionEnd : current.length; | |
| setMobileText(current.slice(0, start) + text + current.slice(end)); | |
| const start = el ? el.selectionStart : current.length; | |
| const end = el ? el.selectionEnd : current.length; | |
| const newCursorPos = start + text.length; | |
| setMobileText(current.slice(0, start) + text + current.slice(end)); | |
| requestAnimationFrame(() => { | |
| el?.setSelectionRange(newCursorPos, newCursorPos); | |
| }); |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in ed85222 — the caret is restored via setSelectionRange after React re-renders the controlled value (rAF with a setTimeout fallback for non-visual environments), matching the CodeMirror path's explicit selection anchor. The methods test now asserts selectionStart === 6 after a mid-text insert.
中文：已在 ed85222 修复——受控 value 更新后用 setSelectionRange 恢复光标（rAF，无 rAF 环境回退 setTimeout），与 CM 路径的显式 selection anchor 对齐；方法映射测试新增中间插入后的光标断言。
| if (!view && !isTouchComposer) return; | ||
| closeSearch(false); | ||
| if (!shellModeRef.current) { | ||
| restoreSelectedHistoryMatch(match); |
There was a problem hiding this comment.
[Suggestion] submitSearchMatch on the mobile path (view is null, isTouchComposer is true) is never exercised by any test. — Failure scenario: a mobile user opens history search, selects a previous prompt, and restoreSelectedHistoryMatch's mobile branch has a bug — the textarea would not be populated with the selected match. No test would catch this.
Consider adding a test in useComposerCore.mobile.dom.test.tsx that opens history search, calls submitSearchMatch, and asserts mobileComposer.value equals the match.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Added in ed85222 — new mobile test: build history by submitting, open history search from a draft, submitSearchMatch a previous prompt, and assert it round-trips through the shared pipeline (onSubmit receives the match, search closes, draft cleared).
中文：已在 ed85222 补测——移动端历史搜索选中并提交：断言经共享管道提交、搜索关闭、草稿清空。
| if (core.mobileComposer) { | ||
| // No CodeMirror to dispatch into. History search is the one key | ||
| // action with a non-keyboard equivalent; the rest are hidden on | ||
| // the textarea backend. | ||
| if (event.ctrlKey && event.key === 'r') { | ||
| core.searchState.openHistorySearch(); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The dispatchComposerKey mobile branch that intercepts Ctrl+R to open history search is not tested at any level. — Failure scenario: if the core.mobileComposer check were accidentally inverted, mobile users pressing the history-search button would dispatch a Ctrl+R into a non-existent CodeMirror view.
Consider adding a DOM test that mounts the mobile composer, calls pressQuickKey with {ctrlKey: true, key: 'r'}, and asserts that history search opens.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Added in ed85222 — ChatEditor-level tests with the mocked core exposing mobileComposer: tapping the history quick action calls searchState.openHistorySearch (this fails if the core.mobileComposer gate were inverted), plus an assertion that the keyboard shortcut hints grid is hidden on mobile with a desktop control.
中文：已在 ed85222 补测——ChatEditor 层用 mock core 暴露 mobileComposer：点历史快捷操作断言调用 openHistorySearch（门控若写反即失败）；另断言移动端隐藏快捷键提示格并带桌面对照。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No changes were made in response to this review round. The two issue-level comments are informational only:
There are no reviews, no inline comments, no failed checks, and no still-red checks to address. 中文说明本轮评审未做任何代码变更。 两条 issue 级评论均为信息性内容：
没有需要处理的评审意见、行内评论、失败检查或持续失败的检查项。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Independent macOS verificationI checked this PR at Passed
WebKit checkI also ran an iPhone 14 / WebKit probe. Playwright WebKit matched This looks like a Playwright WebKit touch-profile limitation rather than evidence of a product failure: the same Pixel 7 profile reports Remaining limitationI installed the official iOS 26.5 Simulator runtime (23F77) and disabled hardware-keyboard passthrough, but could not boot CoreSimulator because this Mac's Therefore the real Mobile Safari virtual-keyboard opening, IME behavior, and Overall, the code-level, Chromium mobile, desktop regression, and WebKit textarea-path checks are green on macOS. The only missing evidence is the real iOS virtual-keyboard layer. |
…ions, caret restore Address the five inline review suggestions on QwenLM#7587: - Auto-grow the mobile textarea with its content, capped by the computed CSS max-height (so --chat-editor-input-max-height overrides stay authoritative). Previously rows={1} plus resize:none meant multi-line drafts scrolled inside ~1.5 visible lines and the CSS max-height was dead. Asserted in the mobile e2e spec via bounding-box growth. - Fire onInputTextChange from setMobileText, matching the CodeMirror updateListener contract: programmatic draft changes (setText, history restore, post-submit clear) now notify parent trackers too. handleMobileChange delegates to setMobileText. - Restore the caret after mobile insertText: a controlled textarea resets the caret to the end on value change; setSelectionRange puts it back after React re-renders (rAF with a setTimeout fallback), matching the CodeMirror path's explicit selection anchor. - Cover the mobile submitSearchMatch path: select a history match, submit through the shared pipeline, draft cleared. - Cover the ChatEditor mobile quick-action gating: the history quick action opens the search UI (never dispatches into a missing EditorView) and the keyboard shortcut hints grid is hidden on the mobile composer with a desktop control. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Finding
[P1] Restore scrolling after the mobile textarea reaches its height cap
The textarea is the direct last child of .editorArea, so it inherits overflow: clip from .editorArea > :last-child. Once the new auto-grow logic reaches max-height (300px by default), longer content is clipped and cannot be scrolled into view.
I reproduced this in headless Chrome with the PR CSS: clientHeight=300, scrollHeight=768; after assigning scrollTop=1000, scrollTop remained 0.
Please override the textarea with overflow-y: auto (or otherwise remove the inherited clipping) and add a regression test that enters enough lines to exceed the cap and verifies the textarea can scroll. The current mobile E2E assertion only verifies that three lines make the textarea grow, so it does not exercise this boundary.
Independent macOS verification on QwenLM#7587 found that Playwright's stock WebKit iPhone profiles match '(hover: none) and (pointer: coarse)' but report navigator.maxTouchPoints === 0, so the automatic detection selected CodeMirror under unmodified WebKit emulation. The maxTouchPoints requirement added nothing the AND media query does not already provide: touch laptops are excluded by the query itself (their primary pointer hovers and is fine), and the only devices that match the query with zero touch points are emulated profiles and TV-style browsers, where the plain textarea is a safe fallback. Dropping it makes stock iPhone/WebKit Playwright runs exercise the automatic detection branch. Real-device behavior is unchanged: phones and tablets match the query and report touch points either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the thorough independent macOS verification — everything code-level, Chromium mobile, and desktop regression is confirmed green there, and the WebKit probe surfaced a real detection gap. Acted on the WebKit finding in 8eebb70: the automatic detection now keys off the The remaining gap is unchanged and honestly held: the real iOS virtual-keyboard layer (Simulator with hardware-keyboard passthrough off) is pending on the verifying host due to the CoreSimulator/ 中文感谢细致的 macOS 独立验证——代码级、Chromium 移动与桌面回归全部确认绿色，WebKit 探测更是发现了一个真实的检测缺口。 已在 8eebb70 处理 WebKit 发现：自动检测改为仅由 其余缺口保持诚实呈现：真实 iOS 虚拟键盘层（关闭硬件键盘直连的 Simulator）因验证机 CoreSimulator/ |
As .editorArea's last child the textarea inherited `overflow: clip` from the `.editorArea > :last-child` wrapper rule (written for the CodeMirror container, whose inner .cm-scroller does the scrolling). `clip` also forbids programmatic scrolling, so once auto-grow reached the CSS max-height, content beyond the cap was unreachable — scrollTop stayed pinned at 0. Override with `overflow-y: auto` via `.editorArea > textarea.mobileTextarea` (the extra type selector outweighs the wrapper rule's specificity). New mobile e2e regression fills 20 lines, asserts growth stops at the computed 300px cap, and verifies the overflow stays reachable: scrollHeight above clientHeight and scrollTop actually moving to the bottom — the exact probe from the review, which pinned at 0 before this fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@ytahdn Thanks — confirmed and fixed in e0b3014. Root cause, as you diagnosed: the textarea is Regression test added ( 中文已确认并在 e0b3014 修复。 根因与您的诊断一致：textarea 是 回归测试已加（移动 e2e）：填 20 行、等待增长，断言计算样式上限 300px 且盒高停在该处，然后执行您的原始探针——赋 |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Re-reviewed the incremental fixes through e0b30141.
The textarea now overrides the inherited overflow: clip with overflow-y: auto, and the new mobile E2E test covers content exceeding the 300px height cap. I also independently reproduced the fixed behavior in Chrome: clientHeight=300, scrollHeight=768, and scrollTop=468 after scrolling to the end.
The previous blocking finding is resolved. No new issues found in the incremental changes.
gwinthis
left a comment
gwinthis
left a comment
There was a problem hiding this comment.
Review: APPROVE (C=0)
Summary
Well-architected fix (+1437/-207, 9 files in packages/web-shell/) that renders a plain textarea composer on touch devices instead of the CodeMirror-based rich editor. Reuses a single submit pipeline via view: EditorView | null rather than building a parallel mobile composer — eliminates state-drift risk.
Verification Report
✓ 63/63 tests passed (3.57s) — 5 test files
✓ tsc --noEmit (web-shell) — clean, no errors
Test breakdown: useIsTouchComposer.test.tsx (9), useComposerCore.test.ts (16), useComposerCore.mobile.dom.test.tsx (17), ChatEditor.test.tsx (21), pre-existing useComposerCore.dom.test.tsx (18).
Key Design Decisions
-
Single-pipeline reuse: Mobile
mobileTextstate mirrors intomobileTextReffor synchronous reads, matching the CodeMirrorupdateListenercontract. No parallel submission path → no state drift. -
Touch detection split into two concerns:
useIsTouchComposer()— frozen at mount viauseStatelazy initializer. Combines?composer=textarea|codemirrorURL escape hatch with(hover: none) and (pointer: coarse)media query. Non-reactive (swapping mid-session would drop draft/tags/images).isCoarsePointerDevice()— non-reactive module function, ignores URL override, used only for focus gating. Correctly suppressesview.focus()even when CodeMirror is forced via URL.
-
Focus gating separated from editor choice: Even if a user forces CodeMirror on a phone, the physical device is still coarse-pointer, so the 4
view.focus()call sites stay suppressed. Addresses root cause of #5958 (iOS claimsactiveElementwithout opening keyboard). -
CSS follows Web Shell conventions: CSS Modules scoping, 16px font prevents iOS Safari auto-zoom, specificity override for
overflow: clipinheritance.
Minor Suggestion (non-blocking)
Dead code in dispatchComposerKey: the Ctrl+R mobile branch in ChatEditor.tsx (~line 1512) is unreachable because showKeyHints={!core.mobileComposer} hides the quick-key grid on mobile. Harmless defensive code — could be cleaned up in a follow-up.
Architecture Insight
Single pipeline with pluggable backends: When adding a device-specific UI variant, reuse the submit/state pipeline and swap only the input backend. This avoids the state-drift risk of parallel paths. The key: mirror the alternative input's state into the same ref structure the pipeline reads, so submitComposerText, getText, hasInput, and openHistorySearch work unchanged.
中文说明
评审：APPROVE (C=0)
概要
架构良好的修复（+1437/-207，9 文件）：在触屏设备上用纯 textarea 替代 CodeMirror 富文本编辑器。通过 view: EditorView | null 复用单一提交流水线，而非构建并行移动端 composer——消除状态漂移风险。
验证报告
- 63/63 测试通过（3.57s）
- typecheck 通过
架构洞察
单一流水线 + 可插拔后端： 添加设备特定 UI 变体时，复用提交/状态流水线，仅切换输入后端。将替代输入的状态镜像到流水线读取的同一 ref 结构中，使所有操作无需修改。
— qwen3.7-max via Qwen Code /review
…detection separation) from PR #7587
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| } finally { | ||
| if (originalMaxTouchPoints) { | ||
| Object.defineProperty( | ||
| Navigator.prototype, | ||
| 'maxTouchPoints', | ||
| originalMaxTouchPoints, | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
[Suggestion] The withTouchDevice cleanup restores the prototype descriptor but never deletes the instance property when originalMaxTouchPoints exists. Object.defineProperty(navigator, 'maxTouchPoints', { value: 5 }) on line 747 creates an own property on navigator that shadows the prototype. In the truthy branch, only the prototype is restored — the instance property persists with value 5, shadowing the restored prototype for any subsequent reader.
Currently no test is affected (this describe block is the last in the file, and vitest isolates test files), but adding delete before restoring the prototype would make the cleanup symmetric with the else branch and prevent a latent test-isolation bug.
| } finally { | |
| if (originalMaxTouchPoints) { | |
| Object.defineProperty( | |
| Navigator.prototype, | |
| 'maxTouchPoints', | |
| originalMaxTouchPoints, | |
| ); | |
| } else { | |
| } finally { | |
| delete (navigator as unknown as Record<string, unknown>)[ | |
| 'maxTouchPoints' | |
| ]; | |
| if (originalMaxTouchPoints) { | |
| Object.defineProperty( | |
| Navigator.prototype, | |
| 'maxTouchPoints', | |
| originalMaxTouchPoints, | |
| ); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
6 participants
What this PR does
On touch devices the Web Shell composer now renders a plain controlled
<textarea>instead of a CodeMirror EditorView. Detection is the(hover: none) and (pointer: coarse)media query (touch laptops keep the desktop editor — their primary pointer hovers and is fine; since 8eebb70 the query alone decides, so stock Playwright WebKit iPhone profiles exercise the automatic branch too), frozen at mount so a mid-session flip cannot drop the draft, with a?composer=textarea|codemirrorURL escape hatch for debugging and rollback.The internal submit pipeline was hoisted out of the editor-creation effect and widened to
view: EditorView | null, so both backends share the exact same submission path: input history, prompt building, top tags, pasted images, and slash/!interpretation from the submitted text all work unchanged on mobile. Enter inserts a newline natively (mobile chat convention) and submission goes through the existing Send button, which already trackshasContent.Independently of the backend switch, the non-gesture programmatic
view.focus()calls (mount, disabled→enabled, dialog close, composerInput seed) are now suppressed on coarse-pointer devices — including when CodeMirror is forced via the escape hatch.Why it's needed
On iOS Safari and Android Chrome the composer was unusable (#5958): tapping the input often did not open the virtual keyboard, and typed characters could be dropped. Two causes stack: mobile virtual keyboards and IMEs interact poorly with contenteditable-based editors in general, and the composer performs programmatic
view.focus()outside user gestures — on iOS this claimsdocument.activeElementwithout opening the keyboard, after which a later tap may no longer fire a fresh focus event, so the keyboard never appears. A native textarea gets the platform's full input stack (keyboard, IME, paste) for free. This closes the last major gap in the ongoing web-shell mobile work (#6000 mobile sidebar, #6584 mobile welcome slots, #5948 responsive TodoPanel).Scale note: this lands larger than the triage bot's 50–80 line estimate (~300 product lines) because it reuses the single submit pipeline through a nullable view instead of building a parallel mobile composer — more diff inside
useComposerCore, but no state-drift risk between two submission paths.Known degradations on the textarea backend, all deliberate and all with working text-based equivalents: no slash/@ completion menus (typed commands still execute — verified in e2e), no inline tag chips (they fall back to the top placement), no history arrow navigation, no large-paste placeholders, no followup Tab-accept. The keyboard-shortcut hints grid is hidden on touch devices; the history-search quick action maps to the search UI, which is a plain React input and fully works.
Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/hooks/useIsTouchComposer.test.tsx client/hooks/useComposerCore.mobile.dom.test.tsx client/hooks/useComposerCore.dom.test.tsx client/hooks/useComposerCore.test.ts— 60 tests: touch detection (AND media query, touch-laptop exclusion, both escape hatches, frozen-at-mount), the textarea backend (render seam with.cm-editorabsence, typing→hasContent, pipeline reuse incl.!-prefix passthrough, inline→top tag fallback, imperative method mapping, composerInput seeding and auto-submit, history search from the draft, image paste via the shared helper), focus gating with a desktop negative control, plus the untouched desktop composer suites.npx playwright test --project=mobile-chromium— 5 e2e tests under Pixel 7 emulation: textarea renders and CodeMirror never mounts; tap → type → Send round-trips through the mock daemon and streams the SSE reply; Enter inserts a newline without submitting;/helptyped as plain text opens the Help dialog (command interpretation from submitted text);?composer=codemirrorforces the CodeMirror path.npx playwright test --project=chromium— desktop e2e untouched (21 passed locally).qwen serve, open a session from a phone or DevTools device emulation, tap the composer → keyboard opens, type, Send. Append?composer=codemirrorto compare against the old path.Evidence (Before & After)
?composer=codemirror)Screenshots are from the Playwright Pixel 7 emulation against the repo's mock daemon.
Tested on
Linux: full vitest suite + Playwright chromium desktop and Pixel 7 mobile emulation + package
tsc --noEmit. macOS (Apple Silicon): independently verified — full vitest suite, the reviewer-plan selection, Playwright Pixel 7 mobile (5/5) and desktop Chromium (21/21), plus a WebKit iPhone probe of the textarea path (see the verification comment below). Browser emulation cannot exercise the real OS virtual-keyboard stack; the real iOS Simulator keyboard layer remains pending on the verifying host (CoreSimulator issue) and will be posted as follow-up evidence.Environment (optional)
Playwright against
npm run devwith the repo'spage.routemock daemon; no real daemon needed.Risk & Scope
submitTextmoved from the editor-creation effect to the render scope withview: EditorView | null. It reads only refs and stable setters, the Enter keymap goes through the samesubmitTextRef, and the full desktop suites pass unchanged, so desktop behavior is preserved byte-for-byte except the focus gating (a no-op on fine-pointer devices).?composer=codemirrorremains as an immediate per-user rollback. iOS keyboard viewport occlusion (visualViewportinsets) is a known separate issue already acknowledged inindex.htmland not addressed here.useComposerCore/ChatEditorare internal (not part of the package's public exports); no new i18n keys.Linked Issues
Fixes #5958
中文说明
这个 PR 做了什么
在触屏设备上，Web Shell 输入框改为渲染一个受控的原生
<textarea>，不再创建 CodeMirror EditorView。检测条件为(hover: none) and (pointer: coarse)媒体查询（带触屏的笔记本主指针可悬停且精细，天然被排除；自 8eebb70 起仅由该查询决定，Playwright 原生 WebKit iPhone 档位也能走到自动检测分支），在挂载时冻结（会话中途切换会丢草稿），并提供?composer=textarea|codemirrorURL 逃生口用于调试与回滚。内部提交管道从编辑器创建 effect 中提升到渲染作用域，参数放宽为
view: EditorView | null，两条路径共享完全相同的提交链路：输入历史、prompt 组装、顶部标签、粘贴图片、以及按提交文本解释的斜杠命令与!前缀在移动端全部原样可用。Enter 原生换行（移动端聊天惯例），提交走已有的 Send 按钮（由hasContent驱动启用）。与后端切换相互独立的一点：非手势的程序化
view.focus()（挂载、禁用恢复、对话框关闭、composerInput 播种）在粗指针设备上一律抑制——包括通过逃生口强制使用 CodeMirror 时。为什么需要
在 iOS Safari 与 Android Chrome 上输入框此前不可用（#5958）：点击输入框经常不弹出虚拟键盘，输入的字符也可能丢失。两个原因叠加：移动端虚拟键盘与输入法本身就与 contenteditable 编辑器兼容性差；同时输入框存在手势之外的程序化
view.focus()——iOS 上这会占住document.activeElement却不弹键盘，之后用户点击可能不再触发新的 focus 事件，键盘永远不出现。原生 textarea 免费获得平台完整的输入栈（键盘、输入法、粘贴）。这补齐了 web-shell 移动端适配（#6000 移动侧边栏、#6584 移动欢迎位、#5948 响应式 TodoPanel）的最后一块主要缺口。规模说明：本 PR 比 triage bot 预估的 50–80 行大（约 300 行产品代码），原因是通过可空 view 复用单一提交管道，而不是另建一条并行的移动提交路径——
useComposerCore内 diff 更多，但避免了双管道间的状态漂移风险。textarea 路径的已知降级（均为有意取舍，且都有文本等价物）：无斜杠/@ 补全菜单（直接输入命令照常执行，e2e 已验证）、无行内标签 chips（回退到顶部放置）、无历史上下箭头导航、无大段粘贴占位符、无 followup Tab 接受。触屏设备上隐藏键盘快捷键提示格；「搜索历史」快捷操作映射到搜索 UI（纯 React input，完整可用）。
审阅者验证方案
如何验证
cd packages/web-shell && npx vitest run client/hooks/useIsTouchComposer.test.tsx client/hooks/useComposerCore.mobile.dom.test.tsx client/hooks/useComposerCore.dom.test.tsx client/hooks/useComposerCore.test.ts— 60 个测试：触屏检测（AND 媒体查询、排除触屏笔记本、两个逃生口、挂载冻结）、textarea 后端（渲染缝与.cm-editor缺席、输入驱动hasContent、管道复用含!前缀透传、行内→顶部标签回退、命令式方法映射、composerInput 播种与自动提交、从草稿打开历史搜索、经共享 helper 的图片粘贴）、focus 门控及桌面负向控制，以及未改动的桌面套件。npx playwright test --project=mobile-chromium— Pixel 7 仿真下 5 个 e2e：textarea 渲染且 CodeMirror 不挂载；tap → 输入 → Send 经 mock daemon 往返并流式回显 SSE 回复；Enter 只换行不提交；纯文本输入/help打开 Help 对话框（按提交文本解释命令）；?composer=codemirror强制走 CodeMirror 路径。npx playwright test --project=chromium— 桌面 e2e 未受影响（本地 21 个全过）。qwen serve，手机或 DevTools 设备仿真打开会话，点击输入框 → 键盘弹出，输入，发送。URL 加?composer=codemirror可对比旧路径。证据（前后对比）
见上方英文部分的截图表格（Playwright Pixel 7 仿真 + 仓库自带 mock daemon）。
已测平台
Linux：完整 vitest 套件 + Playwright chromium 桌面与 Pixel 7 移动仿真 + 包内
tsc --noEmit。macOS（Apple Silicon）：已独立验证——完整 vitest 套件、审阅者验证选集、Playwright Pixel 7 移动（5/5）与桌面 Chromium（21/21），另有 WebKit iPhone 档位对 textarea 路径的探测（见下方验证评论）。浏览器仿真无法覆盖真实的系统虚拟键盘栈；真实 iOS Simulator 键盘层因验证机 CoreSimulator 故障暂缓，结果将作为后续证据补充。风险与范围
submitText从编辑器创建 effect 移到渲染作用域并放宽为view: EditorView | null。它只读取 ref 与稳定的 setter，Enter keymap 走同一个submitTextRef，桌面全套测试原样通过，因此除 focus 门控（细指针设备上为空操作）外桌面行为逐字节保持不变。?composer=codemirror可作为用户级即时回滚。iOS 键盘遮挡视口（visualViewportinsets）是index.html中已承认的独立问题，本 PR 不处理。useComposerCore/ChatEditor为内部实现（不在包的公开导出中）；无新增 i18n key。关联 Issue
Fixes #5958
🤖 Generated with Claude Code