From afc2e40ad8819973b96dbf73049ea17153b9dd63 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 3 Feb 2025 11:22:52 -0700 Subject: [PATCH 1/2] Capture enter & tab in related MR field These keystrokes are almost always intended to "tokenize" the input field rather than move out of it (tab) or submit the form (enter). Allowing the default behavior in that situation is significantly worse than preventing it. Changelog: fixed EE: true --- .../components/related_issuable_input.vue | 9 +++++ .../merge_requests/blocking_mr_input_root.vue | 7 ++++ .../blocking_mr_input_root_spec.js | 39 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/app/assets/javascripts/related_issues/components/related_issuable_input.vue b/app/assets/javascripts/related_issues/components/related_issuable_input.vue index 28f9f8109880cd..97ebebfe97e935 100644 --- a/app/assets/javascripts/related_issues/components/related_issuable_input.vue +++ b/app/assets/javascripts/related_issues/components/related_issuable_input.vue @@ -167,6 +167,14 @@ export default { onFocus() { this.isInputFocused = true; }, + onKeydown(event) { + // eslint-disable-next-line @gitlab/require-i18n-strings + if (['Enter', 'Tab'].includes(event.key)) { + const { value } = this.$refs.input; + + this.$emit('addIssuableFinishEntry', { value, event }); + } + }, setupAutoComplete() { const $input = $(this.$refs.input); @@ -231,6 +239,7 @@ export default { @input="onInput" @focus="onFocus" @blur="onBlur" + @keydown="onKeydown" @keyup.escape.exact="$emit('addIssuableFormCancel')" /> diff --git a/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue b/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue index 3d2c7be84f9be9..db0bf4a3933c9c 100644 --- a/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue +++ b/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue @@ -60,6 +60,12 @@ export default { this.inputValue = ''; } }, + onKeyFinish({ value, event }) { + event.preventDefault(); + event.stopPropagation(); + + this.onBlur(value); + }, }, }; @@ -75,6 +81,7 @@ export default { @addIssuableFormInput="onAddIssuable" @pendingIssuableRemoveRequest="removeReference" @addIssuableFormBlur="onBlur" + @addIssuableFinishEntry="onKeyFinish" /> { expect(wrapper.vm.references).toHaveLength(0); }); + describe('"finish" keystrokes (Enter or Tab)', () => { + const ev = { preventDefault: jest.fn(), stopPropagation: jest.fn() }; + + beforeEach(() => { + ev.preventDefault.mockReset(); + ev.stopPropagation.mockReset(); + }); + + it.each` + description | event + ${'tabs'} | ${ev} + ${'enters'} | ${ev} + `('prevent the default event behavior for $description', ({ event }) => { + createComponent(); + + getInput().vm.$emit('addIssuableFinishEntry', { value: 'x', event }); + + expect(event.preventDefault).toHaveBeenCalledTimes(1); + expect(event.stopPropagation).toHaveBeenCalledTimes(1); + }); + + it('do not add empty references', () => { + createComponent(); + + getInput().vm.$emit('addIssuableFinishEntry', { value: '', event: ev }); + + expect(wrapper.vm.references).toHaveLength(0); + }); + + it('add new tokens', () => { + createComponent(); + + getInput().vm.$emit('addIssuableFinishEntry', { value: '!1', event: ev }); + getInput().vm.$emit('addIssuableFinishEntry', { value: '!2', event: ev }); + + expect(wrapper.vm.references).toEqual(['!1', '!2']); + }); + }); + describe('hidden inputs', () => { const createHiddenInputExpectation = (selector) => (bool) => { expect(wrapper.find(selector).element.value).toBe(`${bool}`); -- GitLab From 8992bc413d9c6a2fc8a9857957712940c96dd4ba Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 4 Feb 2025 09:23:21 -0700 Subject: [PATCH 2/2] Skip the special handler when Cmd/Ctrl-Enter is used --- .../components/related_issuable_input.vue | 4 +- .../merge_requests/blocking_mr_input_root.vue | 13 ++++- .../blocking_mr_input_root_spec.js | 56 ++++++++++++++++--- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/related_issues/components/related_issuable_input.vue b/app/assets/javascripts/related_issues/components/related_issuable_input.vue index 97ebebfe97e935..f13eb399fd1442 100644 --- a/app/assets/javascripts/related_issues/components/related_issuable_input.vue +++ b/app/assets/javascripts/related_issues/components/related_issuable_input.vue @@ -8,6 +8,7 @@ import { inputPlaceholderConfidentialTextMap, inputPlaceholderTextMap, } from '../constants'; +import { ENTER_KEY, TAB_KEY } from '../../lib/utils/keys'; import IssueToken from './issue_token.vue'; const SPACE_FACTOR = 1; @@ -168,8 +169,7 @@ export default { this.isInputFocused = true; }, onKeydown(event) { - // eslint-disable-next-line @gitlab/require-i18n-strings - if (['Enter', 'Tab'].includes(event.key)) { + if ([ENTER_KEY, TAB_KEY].includes(event.key)) { const { value } = this.$refs.input; this.$emit('addIssuableFinishEntry', { value, event }); diff --git a/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue b/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue index db0bf4a3933c9c..a5f79e0505569b 100644 --- a/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue +++ b/ee/app/assets/javascripts/projects/merge_requests/blocking_mr_input_root.vue @@ -1,4 +1,5 @@