fix(web-shell): split-view pane fixes (remove "current" badge, clear composer on send) - #6454
Conversation
In the split view every pane is an equal, independently interactive session (its own DaemonSessionProvider, SSE, transcript, and approvals), so tagging one pane as the workspace's "current" session carried no operational meaning. It only leaked a single-view concept into a peer-of-equals layout and reliably prompted "what is this?" questions from users. Panes are already identified by their titles. Drop the isCurrent badge and its border highlight from ChatPane, and stop passing isCurrent from SplitView. The sidebar and session overview keep their "current" indicators, which are legitimate "you are here" navigation. currentSessionId is retained only to seed the initial pane.
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, with detailed reviewer test plan. Problem: Two real, observable UI issues in the split-view pane: (1) a "current" badge on one pane that conveys no actionable information among equal peers, and (2) composer text persisting through the entire response instead of clearing on send. Both are directly verifiable by opening a split view — not theoretical hardening. Direction: Aligned. The badge removal eliminates a confusing singleton concept leaking into a peer layout. The composer fix matches main-view behavior (clear on admission via the existing Size: 55 production lines (ChatPane.module.css: 14 deletions, ChatPane.tsx: 17+/18−, SplitView.tsx: 2+/4−), 78 test lines. Single package ( Approach: Scope is tight — two related fixes in the same component area, every edit serves the stated goal. No drive-by refactors; the formatting adjustments (line wrapping) are minor Prettier-style changes. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有章节齐全，中英双语，附带详细的复核测试计划。 问题：分屏窗格中两个真实可观测的 UI 问题：(1)「当前」徽标在对等窗格中传达不了任何可操作信息；(2) 发送后文本在整个回复期间留在输入框而非立即清空。两者均可通过打开分屏直接验证——不是理论性加固。 方向：对齐。移除徽标消除了单例概念泄漏到对等布局的困惑；输入框修复通过既有 规模：55 行生产代码（ChatPane.module.css 删 14 行、ChatPane.tsx 增 17/删 18、SplitView.tsx 增 2/删 4），78 行测试代码。单一包（ 方案：范围紧凑——同一组件区域的两个相关修复，每处改动都服务于目标。无顺手重构；格式调整（换行）属 Prettier 风格微调。进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: To fix these two issues, I would: (1) remove the Comparison with the diff: The PR's approach matches my proposal exactly. The badge removal is straightforward dead-code elimination — prop, JSX, CSS class, all gone in one atomic change. The composer fix correctly uses the existing Reuse check: No critical blockers. No security concerns. The changes are localized to a single component with no downstream consumers affected. TestingUnit Tests (PR branch, 34/34 pass)Key test assertions verified:
Type CheckClean — no output, exit code 0. Tmux Real-Scenario TestN/A — this PR modifies web-shell split-view components (browser UI), not CLI behavior. The changes (CSS removal, composer timing) require a browser + running daemon to exercise, which is outside tmux's scope. The unit tests thoroughly cover both behavior changes with controlled promise/admission timing. 中文说明代码审查独立方案： 修复这两个问题我会：(1) 从 与 diff 对比： PR 的方案与我的方案完全匹配。徽标移除是直接的死代码消除——属性、JSX、CSS 类，一次性原子删除。输入框修复正确使用了 复用检查： 无阻断性问题。 无安全顾虑。改动局限于单一组件，不影响下游消费者。 测试单元测试（PR 分支，34/34 通过）关键测试断言已验证：草稿在受理时提交而非整轮结束时；整轮结束后不二次提交；整轮失败后草稿保持清空且错误仍上报；受理前拒绝时草稿保留。 类型检查
Tmux 真实场景测试不适用——本 PR 修改 web-shell 分屏组件（浏览器 UI），不涉及 CLI 行为。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped two-part fix. The badge removal is dead-code elimination done right — prop, JSX, CSS, and caller all updated atomically with no orphaned references. The composer timing fix is the more interesting change: it correctly moves draft clearing from turn-completion to admission using the existing My independent proposal matched the PR's approach exactly. I don't see a simpler path — both changes are minimal and necessary for the stated goals. The diff is focused: no scope creep, no unrelated refactors, just the two fixes and their tests. 34/34 unit tests pass, typecheck is clean. The problem is real (not theoretical), the direction is sound, and the implementation is idiomatic. Approving. ✅ 中文说明这是一个干净、范围良好的两部分修复。徽标移除是标准的死代码消除——属性、JSX、CSS 和调用方一次性原子更新，无残留引用。输入框时序修复是更有趣的那个改动：它正确地将草稿清空从整轮完成移到受理，使用既有的 我的独立方案与 PR 的做法完全一致。看不到更简路径——两处改动都是目标所需的最小集。diff 专注：无范围蔓延，无无关重构，只有两个修复及其测试。 34/34 单元测试通过，类型检查干净。问题真实（非理论性），方向正确，实现符合项目惯例。 批准 ✅ — Qwen Code · qwen3.7-max |
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. ✅
A split-view pane kept the just-sent text sitting in its composer until the whole turn finished. handleSubmit committed the draft on the sendPrompt promise resolving, but that promise resolves via waitForAcceptedPromptCompletion (turn end), not at admission. Switch to the onAdmitted hook so the composer clears the moment the daemon accepts the prompt, matching the main view. A prompt rejected before admission still preserves the draft and surfaces the error.
wenshao
changed the title
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅ Downgraded from Approve to Comment: self-PR; CI still running.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI checks still pending.
— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
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. ✅
…it-then-fail Review follow-up: - commitAccepted is already `() => void`, so pass it directly as the onAdmitted option instead of wrapping it in a redundant `() => commitAccepted?.()` closure. - Add a test for the turn failing after admission: the draft stays cleared (no second commit) and the error is still surfaced to onError.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Both fixes are well-implemented: the "current" badge removal is clean with no dead references, and the composer draft-clear timing change correctly uses the onAdmitted hook with solid test coverage (admission, rejection-before-admission, and post-admission-failure paths all tested). LGTM. ✅
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
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. ✅
4 participants
What this PR does
Two small fixes to the in-window split view, both in the split-pane
ChatPane:Remove the "current" badge. Panes no longer show a rounded "当前 / current" pill (or the matching accent-colored border) on the one pane that happens to be the workspace's active session. The
isCurrentprop, the badge, and the border-highlight branch are removed fromChatPane, andSplitViewno longer passesisCurrent. The session sidebar and the session-overview grid keep their own "current" indicators — those are unchanged.Clear the composer on send, not at turn end. After sending a message from a split pane, the text stayed in the composer for the entire response.
handleSubmitcommitted (cleared) the draft on thesendPromptpromise resolving, but that promise resolves only when the whole turn finishes (waitForAcceptedPromptCompletion), not when the prompt is accepted. It now clears via theonAdmittedhook, which fires at admission — matching the main view. A prompt rejected before admission still preserves the draft and surfaces the error.Why it's needed
Inside the split view every pane is an equal, independently interactive session — its own
DaemonSessionProvider, SSE stream, transcript, and approvals, with keyboard focus scoped per pane by the browser. The "current" pane had no special behavior; the marker only surfaced a single-view singleton concept (connection.sessionId) into a layout of equal peers, where it conveyed nothing actionable and reliably prompted "what does this do?" questions. Panes are already identified by their titles. The sidebar and overview keep their indicators on purpose, because there they are genuine "you are here" navigation.Committing the draft only on turn completion meant the just-sent text sat in the composer for the whole (possibly long) response — confusing, and easy to accidentally resend. Clearing at admission is prompt and still safe: if the daemon rejects the prompt before admission (transcript still loading, session disconnected, or a turn already active),
onAdmittednever fires, so the draft is preserved.Reviewer Test Plan
How to verify
packages/web-shell):npx vitest run client/components/ChatPane.test.tsx client/components/SplitView.test.tsx— 33 tests pass. New/updated cases: the composer commits the draft on admission and NOT again on turn completion (guarding against regressing to the turn-end behavior);SplitView's "seeds with the current session" test keeps its seed assertion, dropping only the assertion on the removeddata-currentmarker.npx tsc --noEmit -p packages/web-shell/tsconfig.lib.json(the build's typecheck) is clean. The broaderpackages/web-shell/tsconfig.jsonreports the same pre-existing errors in unrelated test fixtures before and after this change — zero new errors.Evidence (Before & After)
--primary) border; after, neither — panes render identically apart from their titles.Both are behavior/markup changes exercised by the unit tests above; the badge removal is a pure element removal (the
.currentBadgespan and.paneCurrentborder class), so there is no new UI to screenshot.Tested on
Environment (optional)
Vitest (jsdom) unit tests plus a local
tsctype check, run on macOS.Risk & Scope
onAdmittedhook, preserving the "don't drop the draft if the prompt is rejected before admission" property.ChatPane'sisCurrentprop is removed and its only caller (SplitView) is updated in the same change;currentSessionIdis retained purely to seed the initial pane.Linked Issues
中文说明
这个 PR 做了什么
对窗口内分屏视图的两处小修复,都在分屏窗格
ChatPane里:移除「当前」徽标。 窗格不再对「恰好是工作区活动会话」的那一个窗格显示圆角的「当前 / current」药丸(以及配套的高亮色边框)。
ChatPane中的isCurrent属性、徽标、边框高亮分支都被移除,SplitView也不再传isCurrent。会话侧栏和会话总览网格各自的「当前」标识保持不变。发送时清空输入框,而不是等整轮结束。 从分屏窗格发送消息后,文本会在整个回复期间一直留在输入框里。
handleSubmit原本是在sendPrompt的 Promise resolve 时才清空草稿,但那个 Promise 要等整轮结束(waitForAcceptedPromptCompletion)才 resolve,而不是在 prompt 被受理时。现改为通过onAdmitted钩子清空——它在受理时触发,和主界面一致。若 prompt 在受理前被拒绝,草稿仍会保留并弹出错误。为什么需要
在分屏视图内部,每个窗格都是完全对等、可独立交互的会话——各自拥有
DaemonSessionProvider、SSE 流、转录和审批,键盘焦点由浏览器按窗格隔离。「当前」那个窗格没有任何特殊行为;这个标记只是把一个单视图的单例概念(connection.sessionId)暴露进对等窗格并排的布局,在那里它传达不了任何可操作的信息,反而稳定地引发「这是干嘛的?」这类疑问。窗格本来就靠标题区分。侧栏和总览之所以保留标识,是因为在那些场景里它们是真正的「你在这里」导航。只在整轮完成时才清空草稿,意味着刚发出的文本会在整段(可能很长的)回复期间一直留在输入框里——既令人困惑,又容易误重发。改为在受理时清空既及时又安全:若 daemon 在受理前拒绝了 prompt(转录仍在加载、会话断连,或已有一轮在进行),
onAdmitted不会触发,草稿因此得以保留。复核测试计划
如何验证
packages/web-shell下):npx vitest run client/components/ChatPane.test.tsx client/components/SplitView.test.tsx—— 33 个测试通过。新增/更新用例:输入框在受理时提交草稿,并且在整轮完成时不再二次提交(防止回退到旧的「轮末清空」行为);SplitView的 "seeds with the current session" 测试保留 seed 断言,只删掉针对已移除的data-current标记的那条断言。npx tsc --noEmit -p packages/web-shell/tsconfig.lib.json(构建所用的类型检查)干净通过。更宽的packages/web-shell/tsconfig.json在改动前后都报同样的既有错误(都在无关的测试夹具里)——零新增。证据(改动前后)
--primary)边框;改动后两者都没有——除各自标题外所有窗格渲染一致。两者都由上面的单元测试覆盖;徽标移除是纯粹的元素删除(
.currentBadgespan 和.paneCurrent边框类),因此没有新的 UI 可供截图。测试平台
环境(可选)
Vitest(jsdom)单元测试,外加本地
tsc类型检查,在 macOS 上运行。风险与范围
onAdmitted钩子把清空时机从轮末挪到受理,保留了「受理前被拒则不丢草稿」的性质。ChatPane的isCurrent属性被移除,其唯一调用方(SplitView)在同一改动中一并更新;currentSessionId仅保留用于 seed 初始窗格。