Skip to content

fix(conversation): preserve shortcuts and IME-safe multiline drafting - #321

Closed
PeterGuy326 wants to merge 5 commits into
bytefolk:mainfrom
PeterGuy326:fix/conversation-enter-progress
Closed

PeterGuy326 wants to merge 5 commits into
bytefolk:mainfrom
PeterGuy326:fix/conversation-enter-progress

Conversation

@PeterGuy326

Copy link
Copy Markdown
Contributor

Supersedes #311 after rebuilding the change on the latest main.

Summary

  • keep configurable send shortcuts while making Enter-send mode support Ctrl/⌘+Enter multiline insertion
  • preserve Shift+Enter and IME composition safety
  • retain the accepted → processing → completed progress interaction tests
  • avoid all stale-file and packaging regressions from the original PR

Verification

  • renderer: 553/553 tests passed
  • renderer and UI typechecks passed
  • renderer production build passed
  • package layout/safety: 19/19 passed

@sun-970 sun-970 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.

Review — Approve

Head cfbea72. MERGEABLE. Required CI on this head is green (Node 24 ubuntu/macOS, unsigned installers, unpacked smoke, layout parity, dependency review, Scorecard).

Enter-send mode: Ctrl/⌘+Enter inserts a newline at the caret (or replaces the selection) and restores the caret after the controlled value commits; a second chord uses the restored caret, not the end of the field. Same-value newline replacement (nextValue === value) updates the selection immediately. Conversation switches will not move another textarea (isConnected + value match).

Shift+Enter stays native. IME is ignored via composing ref + isComposing + keyCode 229, so confirmation neither sends nor inserts an extra line. mod-enter still sends on Ctrl/⌘+Enter; switching the preference on the same draft is covered. Send stays gated on trim / running / disabledReason.

Tests in turn-composer-shortcuts.test.tsx cover caret positions, selection replace, running-task drafting, shortcut preference, and IME. Progress interaction tests still cover accepted → processing → completed / failed / indeterminate.

Locales (en/zh) match the new chord. CHANGELOG [Unreleased] is present.

Non-blocking

  • turn.keyboardHint is written for Enter-send mode. If the composer shows the same string in mod-enter mode, the copy is still slightly wrong (pre-existing shape).

Not a CODEOWNER.

@Bindy-lbb

Copy link
Copy Markdown
Collaborator

Review conclusion — no blocking issues found.\n\nI reviewed the 6-file diff covering TurnComposer, shortcut/progress tests, en/zh locales, and CHANGELOG. The Ctrl/⌘+Enter path preserves the caret after controlled-value commits, replaces selections correctly, keeps Shift+Enter native, and avoids dispatch during IME composition. I also checked running-task drafting, sendShortcut switching, and the existing progress interaction coverage.\n\nThe PR description reports green renderer tests/typechecks/build/package checks, and the focused shortcut/progress coverage is consistent with the implementation. One non-blocking copy caveat remains: the keyboard hint is specifically written for Enter-send mode; the existing mod-enter hint shape is outside this PR's scope. No blocking issue found; formal reviewer can proceed with the normal review flow.

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

在 exact head 080ae595f0f5af03fb874fb628b3c642e18605d2 上完成正式复核:未发现阻塞问题。

确认:

  • Enter-send 模式下 Ctrl/⌘+Enter 在光标处插入换行,并正确替换选区、恢复受控 textarea 光标;连续触发不会跳到末尾。
  • Shift+Enter 保持原生行为;IME 通过 composition ref、isComposing 和 keyCode 229 三层保护,不会误发送或多插入换行。
  • mod-enter 模式仍由 Ctrl/⌘+Enter 发送;同一草稿切换 sendShortcut 后行为立即更新。
  • running、disabledReason 和空白输入的发送门禁保持不变;进度交互覆盖 accepted → processing → completed/failed/indeterminate。

本地 exact-head 证据:npm run build、renderer typecheck 通过;目标 renderer 38 tests 全通过;git diff --check 通过。GitHub 当前 required checks 也为绿,且 PR 可合并。

非阻塞:keyboard hint 主要对应 Enter-send 模式;mod-enter 下的既有文案形态不在本 PR 范围内。

注:该 PR 作者是当前 GitHub 账号,GitHub 禁止作者自批,因此以正式 COMMENT review 提交;合并仍需非作者审批。

@PeterGuy326

Copy link
Copy Markdown
Contributor Author

Content merged to main via #383 (in-repo recreation, rebase merge, commit authored as @PeterGuy326; all five file blobs byte-identical to this PR head). Closing because fork pull requests cannot satisfy the required CodeQL check name in branch protection on this repository. Original review history (approval by sun-970) preserved here.

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