diff --git a/client/src/hooks/Input/useTextarea.ts b/client/src/hooks/Input/useTextarea.ts index 04f93c6bf6..58bfac9910 100644 --- a/client/src/hooks/Input/useTextarea.ts +++ b/client/src/hooks/Input/useTextarea.ts @@ -8,6 +8,7 @@ import type { KeyboardEvent } from 'react'; import { parseBinding, isMacPlatform, + bindingsMatch, bindingFromEvent, resolveSubmitOverrideAction, } from '~/utils/shortcuts'; @@ -213,10 +214,15 @@ export default function useTextarea({ return; } // Before the bare Ctrl/Cmd branch below, which would otherwise - // swallow the shifted chord. Yields to a rebound submit shortcut for - // the same reason that branch does: a user who bound submit to this - // chord must keep getting submit. - if ((e.ctrlKey || e.metaKey) && e.shiftKey && submitOverride === undefined) { + // swallow the shifted chord. Yields only to a submit shortcut rebound + // to THIS chord — unlike the default Ctrl/Cmd+Enter branch below, an + // unrelated rebinding (or an explicit unbind) leaves this chord free, + // so it must keep the meaning the hovercard advertises. + if ( + (e.ctrlKey || e.metaKey) && + e.shiftKey && + !bindingsMatch(bindingFromEvent(e.nativeEvent), submitOverride) + ) { e.preventDefault(); onDuringRunModifier('preempt'); return; diff --git a/client/src/utils/__tests__/shortcuts.spec.ts b/client/src/utils/__tests__/shortcuts.spec.ts new file mode 100644 index 0000000000..5f38ad03c9 --- /dev/null +++ b/client/src/utils/__tests__/shortcuts.spec.ts @@ -0,0 +1,76 @@ +import { parseBinding, bindingsMatch, resolveSubmitOverrideAction } from '~/utils/shortcuts'; + +/** + * The composer's during-run Ctrl/Cmd+Shift+Enter (Interrupt & steer) must yield + * to a rebound `submitMessage` shortcut only when submit is bound to THAT chord. + * Yielding to any override at all silently removes the shortcut the hovercard + * advertises for users who rebound submit to something unrelated. + */ +describe('bindingsMatch', () => { + const preemptChord = parseBinding('Ctrl+Shift+Enter'); + + test('matches the same chord', () => { + expect(bindingsMatch(preemptChord, parseBinding('Ctrl+Shift+Enter'))).toBe(true); + }); + + test('is insensitive to the order modifiers are written in', () => { + expect(bindingsMatch(parseBinding('Shift+Ctrl+Enter'), preemptChord)).toBe(true); + }); + + test('does not match a submit shortcut rebound to an unrelated chord', () => { + expect(bindingsMatch(preemptChord, parseBinding('Ctrl+J'))).toBe(false); + }); + + test('does not match the same key held with different modifiers', () => { + expect(bindingsMatch(preemptChord, parseBinding('Ctrl+Enter'))).toBe(false); + expect(bindingsMatch(preemptChord, parseBinding('Cmd+Shift+Enter'))).toBe(false); + }); + + test('treats an explicitly unbound shortcut as no match', () => { + expect(bindingsMatch(preemptChord, null)).toBe(false); + expect(bindingsMatch(preemptChord, parseBinding(''))).toBe(false); + }); + + test('treats an unset shortcut as no match', () => { + expect(bindingsMatch(preemptChord, undefined)).toBe(false); + }); + + test('never matches when no chord was pressed', () => { + expect(bindingsMatch(null, preemptChord)).toBe(false); + expect(bindingsMatch(null, null)).toBe(false); + }); +}); + +describe('resolveSubmitOverrideAction', () => { + const plainEnter = parseBinding('Enter'); + + test('submits on the rebound chord', () => { + expect( + resolveSubmitOverrideAction( + parseBinding('Ctrl+Shift+Enter'), + parseBinding('Ctrl+Shift+Enter'), + false, + ), + ).toBe('submit'); + }); + + test('submits a bare Enter when enterToSend is on', () => { + expect(resolveSubmitOverrideAction(plainEnter, parseBinding('Ctrl+J'), true)).toBe('submit'); + }); + + test('inserts a newline for a bare Enter when enterToSend is off', () => { + expect(resolveSubmitOverrideAction(plainEnter, parseBinding('Ctrl+J'), false)).toBe('newline'); + }); + + test('leaves Shift+Enter and non-Enter keys to the browser', () => { + expect(resolveSubmitOverrideAction(parseBinding('Shift+Enter'), plainEnter, true)).toBe('none'); + expect(resolveSubmitOverrideAction(parseBinding('Ctrl+J'), parseBinding('Ctrl+J'), true)).toBe( + 'none', + ); + }); + + test('an unbound submit shortcut still allows bare Enter to send', () => { + expect(resolveSubmitOverrideAction(plainEnter, null, true)).toBe('submit'); + expect(resolveSubmitOverrideAction(plainEnter, null, false)).toBe('newline'); + }); +}); diff --git a/client/src/utils/shortcuts.ts b/client/src/utils/shortcuts.ts index 0051a21718..561025c8a7 100644 --- a/client/src/utils/shortcuts.ts +++ b/client/src/utils/shortcuts.ts @@ -145,6 +145,18 @@ export function bindingHash(binding: ShortcutBinding): string { return `${flags}|${binding.key}`; } +/** + * Whether a pressed chord is the one a shortcut is bound to. Absent on either + * side means no match: an unset (`undefined`) or explicitly unbound (`null`) + * shortcut is not something a keypress can match. + */ +export function bindingsMatch( + a: ShortcutBinding | null | undefined, + b: ShortcutBinding | null | undefined, +): boolean { + return a != null && b != null && bindingHash(a) === bindingHash(b); +} + export function hasModifier(binding: ShortcutBinding): boolean { return binding.meta || binding.ctrl || binding.alt; } @@ -185,10 +197,7 @@ export function resolveSubmitOverrideAction( if (!eventBinding || eventBinding.key !== 'Enter') { return 'none'; } - const matchesChord = - submitOverride != null && - submitOverride.key === 'Enter' && - bindingHash(eventBinding) === bindingHash(submitOverride); + const matchesChord = bindingsMatch(eventBinding, submitOverride); const isPlainEnter = !eventBinding.meta && !eventBinding.ctrl && !eventBinding.alt && !eventBinding.shift; if (matchesChord || (isPlainEnter && enterToSend)) {