feat: add Ctrl+Enter / Cmd+Enter as send shortcut#698
Conversation
Support Ctrl+Enter (Windows/Linux) and Cmd+Enter (macOS) as additional send shortcuts alongside plain Enter. Shift+Enter continues to insert a newline. Changes: - ChatInput: expand Enter check to accept Ctrl/Cmd+Enter - ChatInput.test: add 4 tests covering plain Enter, Shift+Enter, Ctrl+Enter, and Cmd+Enter
Greptile SummaryThis PR adds Ctrl+Enter (Windows/Linux) and Cmd+Enter (macOS) as send shortcuts in the chat input, alongside an accompanying test suite. The boolean expression used to implement this has a subtle flaw that causes
Confidence Score: 3/5Not safe to merge as-is — the condition change introduces an unintended send trigger on Shift+Ctrl+Enter that contradicts the Shift-always-means-newline design. The original The single changed line in Important Files Changed
|
|
|
||
| if (e.key === "Enter" && !e.shiftKey) { | ||
| // Send: plain Enter (without Shift) or Ctrl/Cmd+Enter | ||
| if (e.key === "Enter" && (!e.shiftKey || e.ctrlKey || e.metaKey)) { |
There was a problem hiding this comment.
Logic bug: Shift+Ctrl+Enter and Shift+Cmd+Enter now unexpectedly send
The new condition (!e.shiftKey || e.ctrlKey || e.metaKey) evaluates to true when ctrlKey or metaKey is set, regardless of shiftKey. So holding Shift+Ctrl+Enter (or Shift+Cmd+Enter) now submits the message instead of inserting a newline — contradicting the stated invariant that Shift always produces a newline.
Also note: the original !e.shiftKey condition already handled Ctrl+Enter and Cmd+Enter correctly (since those combos have shiftKey=false). The net observable change here is only for the Shift+modifier combinations, which aren't mentioned in the PR description or covered by the new tests.
A missing test case Shift+Ctrl+Enter (expecting onSubmit NOT to have been called) would surface this regression.
| it("does not submit on Shift+Enter (newline)", () => { | ||
| const { onSubmit, textarea } = renderInput(); | ||
| fireEvent.change(textarea, { target: { value: "hello" } }); | ||
| fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true }); | ||
| expect(onSubmit).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
A test for
Shift+Ctrl+Enter is missing. With the new condition this combo now submits (because ctrlKey short-circuits !shiftKey), but the PR description says Shift+Enter should always produce a newline. Adding this case would immediately surface the regression.
| it("does not submit on Shift+Enter (newline)", () => { | |
| const { onSubmit, textarea } = renderInput(); | |
| fireEvent.change(textarea, { target: { value: "hello" } }); | |
| fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true }); | |
| expect(onSubmit).not.toHaveBeenCalled(); | |
| }); | |
| it("does not submit on Shift+Enter (newline)", () => { | |
| const { onSubmit, textarea } = renderInput(); | |
| fireEvent.change(textarea, { target: { value: "hello" } }); | |
| fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true }); | |
| expect(onSubmit).not.toHaveBeenCalled(); | |
| }); | |
| it("does not submit on Shift+Ctrl+Enter (newline takes precedence)", () => { | |
| const { onSubmit, textarea } = renderInput(); | |
| fireEvent.change(textarea, { target: { value: "hello" } }); | |
| fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true, ctrlKey: true }); | |
| expect(onSubmit).not.toHaveBeenCalled(); | |
| }); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Feature
Add Ctrl+Enter (Windows/Linux) and Cmd+Enter (macOS) as send shortcuts alongside the existing plain Enter.
Changes
!e.shiftKeyto!e.shiftKey || e.ctrlKey || e.metaKey, so Ctrl/Cmd+Enter also triggers sendBehavior
The CJK IME composition guard is unaffected — Enter is still swallowed while an IME is composing.