-
Notifications
You must be signed in to change notification settings - Fork 8
feat(sender): support Ctrl+Enter and Shift+Enter for newline insertion in input #263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughDocs clarify Sender single-line→multi-line switching and newline/submit shortcut rules. Keyboard handler refactor centralizes newline insertion (Ctrl+Enter/Shift+Enter support), moves newline processing before other key logic, and streamlines submit checks. Changes
Sequence DiagramsequenceDiagram
participant User
participant HandleKeyPress as HandleKeyPress
participant HandleNewLine as HandleNewLine
participant SetMultipleMode as SetMultipleMode
participant InsertNewLine as InsertNewLine
participant InputState as InputState
User->>HandleKeyPress: keydown (Enter / Ctrl+Enter / Shift+Enter / other)
Activate HandleKeyPress
HandleKeyPress->>HandleNewLine: delegate newline handling
Activate HandleNewLine
alt submitType == "enter"
alt Ctrl+Enter or Shift+Enter
HandleNewLine->>SetMultipleMode: ensure multi-line mode if needed
Activate SetMultipleMode
SetMultipleMode->>InputState: update mode
Deactivate SetMultipleMode
HandleNewLine->>InsertNewLine: insert "\n" at cursor & adjust scroll
Activate InsertNewLine
InsertNewLine->>InputState: update value & cursor position
Deactivate InsertNewLine
HandleNewLine->>HandleKeyPress: preventDefault / stop propagation
else if plain Enter (submit shortcut allowed)
HandleNewLine->>HandleKeyPress: defer to submission path
end
end
Deactivate HandleNewLine
HandleKeyPress->>HandleKeyPress: run navigation, suggestion, escape, and submit checks
alt submit conditions met
HandleKeyPress->>InputState: triggerSubmit()
end
Deactivate HandleKeyPress
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/src/sender/composables/useKeyboardHandler.ts (2)
87-100: Scroll behavior may be incorrect when inserting newline mid-content.Line 98 sets
scrollTop = scrollHeight, which scrolls to the bottom of the textarea. However, when the user inserts a newline in the middle of existing text, they likely want the view to remain at the cursor position, not jump to the bottom.Consider using
scrollIntoViewbehavior or calculating the proper scroll position based on cursor location:setTimeout(() => { target.selectionStart = target.selectionEnd = cursorPosition + 1 - // 滚动到光标所在位置,确保光标可见 - target.scrollTop = target.scrollHeight + // 确保光标可见(浏览器通常会自动处理光标可见性) }, 0)Alternatively, if explicit scroll control is needed, consider a more precise calculation based on line height and cursor position.
107-138: MissingmetaKeysupport for Cmd+Enter on macOS.Line 111 checks
event.ctrlKey && !event.shiftKeybut doesn't account formetaKey. On macOS, users typically use Cmd instead of Ctrl. ThecheckSubmitShortcutfunction (line 75) handlesmetaKeyfor submissions, buthandleNewLinedoesn't, creating inconsistent behavior.Additionally, the Ctrl+Enter and Shift+Enter branches (lines 119-126 and 128-135) contain duplicated logic.
const handleNewLine = (event: KeyboardEvent): boolean => { // 只在 submitType='enter' 时支持 Ctrl+Enter 和 Shift+Enter 换行 if (props.submitType !== 'enter' || event.key !== 'Enter') return false - const isCtrlEnter = event.ctrlKey && !event.shiftKey + const isCtrlEnter = (event.ctrlKey || event.metaKey) && !event.shiftKey const isShiftEnter = event.shiftKey && !event.ctrlKey if (!isCtrlEnter && !isShiftEnter) return false event.preventDefault() const target = event.target as HTMLTextAreaElement - // Ctrl+Enter: 单行模式切换到多行,多行模式直接换行 - if (isCtrlEnter) { - if (currentMode?.value === 'single' && setMultipleMode) { - setMultipleMode() - } - insertNewLine(target) - return true + // 单行模式切换到多行,然后插入换行 + if (currentMode?.value === 'single' && setMultipleMode) { + setMultipleMode() } - - // Shift+Enter: 单行模式切换到多行,多行模式直接换行 - if (isShiftEnter) { - if (currentMode?.value === 'single' && setMultipleMode) { - setMultipleMode() - } - insertNewLine(target) - return true - } - - return false + insertNewLine(target) + return true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/src/components/sender.md(2 hunks)packages/components/src/sender/composables/useKeyboardHandler.ts(1 hunks)
🔇 Additional comments (5)
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
143-149: LGTM!The integration of
handleNewLineat the beginning ofhandleKeyPresswith early return is correct. This ensures newline shortcuts are processed before other handlers (suggestions, escape, submit) whensubmitType='enter'.docs/src/components/sender.md (4)
20-24: LGTM!The documentation clearly explains the auto-switch behavior in single-line mode, including both the content overflow trigger and the new Ctrl+Enter/Shift+Enter shortcuts when
submitType="enter".
149-155: LGTM!The info block provides a clear and accurate summary of the submit vs newline behavior for each
submitTypeoption, along with the auto-switch behavior in single-line mode.
162-163: Consider adding macOS keyboard shortcut clarification.The shortcut table is accurate. If the suggested
metaKeysupport is added in the code, consider clarifying thatCmd+Enterworks asCtrl+Enteron macOS for better cross-platform documentation.
168-174: LGTM!The tip block provides helpful details about newline shortcuts, including the auto-switch behavior in single-line mode. This complements the info block well and gives users a complete understanding of keyboard interactions.
packages/components/src/sender/composables/useKeyboardHandler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
132-185: Keyboard flow refactor looks good; consider simplifyingcanSubmitchecksThe new
handleKeyPressflow (IME guard → newline handling → suggestions → escape → submit shortcut) is clear and matches the documented shortcut rules. Delegating newline logic tohandleNewLineand submit logic tocheckSubmitShortcut/triggerSubmitis a nice cleanup.Minor duplication:
triggerSubmitalready checkscanSubmit.value, and Line 181 also guards oncanSubmit.value. You can remove one of these guards so there’s a single source of truth for the “can submit” condition, e.g.:- if (checkSubmitShortcut(event, props.submitType as SubmitTrigger) && canSubmit.value) { - event.preventDefault() - triggerSubmit() - } + if (checkSubmitShortcut(event, props.submitType as SubmitTrigger)) { + event.preventDefault() + triggerSubmit() + }(or drop the check inside
triggerSubmitinstead).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/src/sender/composables/useKeyboardHandler.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (1)
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
107-127: I'm unable to access the repository due to persistent cloning errors. However, I can analyze the review comment based on the information provided:Verification inconclusive—repository access unavailable; concern remains theoretical without template inspection
The review comment raises a valid concern about potential DOM element stale references if mode switching replaces the underlying element. However, the concern is explicitly conditional ("If switching modes replaces the underlying DOM element") and the reviewer acknowledges that "If single/multiple modes always share the same textarea instance, this is fine as-is."
Without access to:
- The Sender component template to verify if
v-ifconditionals swap elements between single/multiple modes- The
setMultipleMode()implementation- The
insertNewLine()function's interaction withsetTimeoutI cannot definitively confirm or refute whether this is an actual issue or a theoretical edge case that does not occur in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
109-129: Clarify modifier handling for newline shortcuts (Ctrl/Shift vs Meta/Alt)
handleNewLinetreats onlyctrlKeyandshiftKeyas meaningful, whilemetaKey/altKeyare ignored here (andaltKeyis also ignored incheckSubmitShortcut). For consistency and to avoid surprises on different platforms, consider making the modifier intent explicit, e.g. by:
- Explicitly ruling out
metaKey/altKeyinisCtrlEnter/isShiftEnter, and/or- Documenting that Alt/Command + Enter are intentionally not treated as newline shortcuts when
submitType='enter'.This is small, but it helps keep behavior predictable across OSes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/src/sender/composables/useKeyboardHandler.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/src/sender/composables/useKeyboardHandler.ts (1)
packages/components/src/sender/index.type.ts (1)
SubmitTrigger(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (2)
packages/components/src/sender/composables/useKeyboardHandler.ts (2)
84-102: insertNewLine now correctly respects selection rangeUsing
selectionStart/selectionEndand replacing the selected text with'\n'matches native textarea behavior; caret repositioning and scroll handling viasetTimeoutalso look solid.
131-139: Verify Ctrl+Enter behavior when suggestions are visibleWith newline handling moved ahead of suggestion logic, when
submitType='enter'and suggestions are shown:
- Plain
Enterstill accepts the active suggestion (defaultactiveSuggestionKeys=['Enter','Tab']).Ctrl+Enter/Shift+Enternow insert a newline viahandleNewLineinstead of accepting the suggestion (previously, modifiers were ignored andEntertypically accepted the suggestion).If the intended UX is "only unmodified Enter accepts suggestions; Ctrl/Shift+Enter always mean newline in this mode", this is fine; otherwise, you may want to gate
handleNewLinebehind!showSuggestions.valueor refine the suggestion key handling.Also applies to: 183-186
Component
Docs
Preview
https://sonyleo.github.io/tiny-robot/alpha/components/sender.html#%E8%BE%93%E5%85%A5%E6%A8%A1%E5%BC%8F
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.