Commit 54e47a49 authored by Nathan Friend's avatar Nathan Friend Committed by Denys Mishunov

Prevent form submission on Enter in dropdown

This commit updates two dropdown components to not submit their parent
forms when Enter is pressed inside the dropdown search boxes.
parent 0757395c
......@@ -89,6 +89,14 @@ export default {
return this.requestCount !== 0;
},
},
created() {
// This method is defined here instead of in `methods`
// because we need to access the .cancel() method
// lodash attaches to the function, which is
// made inaccessible by Vue. More info:
// https://stackoverflow.com/a/52988020/1063392
this.debouncedSearchMilestones = debounce(this.searchMilestones, 100);
},
mounted() {
this.fetchMilestones();
},
......@@ -108,7 +116,7 @@ export default {
this.requestCount -= 1;
});
},
searchMilestones: debounce(function searchMilestones() {
searchMilestones() {
this.requestCount += 1;
const options = {
search: this.searchQuery,
......@@ -133,7 +141,14 @@ export default {
.finally(() => {
this.requestCount -= 1;
});
}, 100),
},
onSearchBoxInput() {
this.debouncedSearchMilestones();
},
onSearchBoxEnter() {
this.debouncedSearchMilestones.cancel();
this.searchMilestones();
},
toggleMilestoneSelection(clickedMilestone) {
if (!clickedMilestone) return [];
......@@ -186,7 +201,8 @@ export default {
v-model.trim="searchQuery"
class="gl-m-3"
:placeholder="this.$options.translations.searchMilestones"
@input="searchMilestones"
@input="onSearchBoxInput"
@keydown.enter.prevent="onSearchBoxEnter"
/>
<gl-new-dropdown-item @click="onMilestoneClicked(null)">
......
......@@ -87,6 +87,15 @@ export default {
},
},
created() {
// This method is defined here instead of in `methods`
// because we need to access the .cancel() method
// lodash attaches to the function, which is
// made inaccessible by Vue. More info:
// https://stackoverflow.com/a/52988020/1063392
this.debouncedSearch = debounce(function search() {
this.search(this.query);
}, SEARCH_DEBOUNCE_MS);
this.setProjectId(this.projectId);
this.search(this.query);
},
......@@ -95,9 +104,13 @@ export default {
focusSearchBox() {
this.$refs.searchBox.$el.querySelector('input').focus();
},
onSearchBoxInput: debounce(function search() {
onSearchBoxEnter() {
this.debouncedSearch.cancel();
this.search(this.query);
}, SEARCH_DEBOUNCE_MS),
},
onSearchBoxInput() {
this.debouncedSearch();
},
selectRef(ref) {
this.setSelectedRef(ref);
this.$emit('input', this.selectedRef);
......@@ -129,6 +142,7 @@ export default {
class="gl-m-3"
:placeholder="i18n.searchPlaceholder"
@input="onSearchBoxInput"
@keydown.enter.prevent="onSearchBoxEnter"
/>
<div class="gl-flex-grow-1 gl-overflow-y-auto">
......
......@@ -140,7 +140,7 @@ export default {
class="form-control"
/>
</gl-form-group>
<gl-form-group class="w-50" data-testid="milestones-field" @keydown.enter.prevent.capture>
<gl-form-group class="w-50" data-testid="milestones-field">
<label>{{ __('Milestones') }}</label>
<div class="d-flex flex-column col-md-6 col-sm-10 pl-0">
<milestone-combobox
......
---
title: Prevent form submission in search boxes on New Release and Edit Release pages
merge_request: 40011
author:
type: changed
......@@ -8,4 +8,15 @@
// [2]: https://gitlab.com/gitlab-org/gitlab/-/issues/213378
// Further reference: https://github.com/facebook/jest/issues/3465
export default fn => fn;
export default fn => {
const debouncedFn = jest.fn().mockImplementation(fn);
debouncedFn.cancel = jest.fn();
debouncedFn.flush = jest.fn().mockImplementation(() => {
const errorMessage =
"The .flush() method returned by lodash.debounce is not yet implemented/mocked by the mock in 'spec/frontend/__mocks__/lodash/debounce.js'.";
throw new Error(errorMessage);
});
return debouncedFn;
};
......@@ -2,10 +2,12 @@ import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import { shallowMount } from '@vue/test-utils';
import { GlNewDropdown, GlLoadingIcon, GlSearchBoxByType } from '@gitlab/ui';
import { ENTER_KEY } from '~/lib/utils/keys';
import MilestoneCombobox from '~/milestones/project_milestone_combobox.vue';
import { milestones as projectMilestones } from './mock_data';
const TEST_SEARCH_ENDPOINT = '/api/v4/projects/8/search';
const TEST_SEARCH = 'TEST_SEARCH';
const extraLinks = [
{ text: 'Create new', url: 'http://127.0.0.1:3000/h5bp/html5-boilerplate/-/milestones/new' },
......@@ -21,6 +23,8 @@ describe('Milestone selector', () => {
const findNoResultsMessage = () => wrapper.find({ ref: 'noResults' });
const findSearchBox = () => wrapper.find(GlSearchBoxByType);
const factory = (options = {}) => {
wrapper = shallowMount(MilestoneCombobox, {
...options,
......@@ -63,7 +67,7 @@ describe('Milestone selector', () => {
describe('before results', () => {
it('should show a loading icon', () => {
const request = mock.onGet(TEST_SEARCH_ENDPOINT, {
params: { search: 'TEST_SEARCH', scope: 'milestones' },
params: { search: TEST_SEARCH, scope: 'milestones' },
});
expect(wrapper.find(GlLoadingIcon).exists()).toBe(true);
......@@ -85,9 +89,9 @@ describe('Milestone selector', () => {
describe('with empty results', () => {
beforeEach(() => {
mock
.onGet(TEST_SEARCH_ENDPOINT, { params: { search: 'TEST_SEARCH', scope: 'milestones' } })
.onGet(TEST_SEARCH_ENDPOINT, { params: { search: TEST_SEARCH, scope: 'milestones' } })
.reply(200, []);
wrapper.find(GlSearchBoxByType).vm.$emit('input', 'TEST_SEARCH');
findSearchBox().vm.$emit('input', TEST_SEARCH);
return axios.waitForAll();
});
......@@ -116,7 +120,7 @@ describe('Milestone selector', () => {
web_url: 'http://127.0.0.1:3000/h5bp/html5-boilerplate/-/milestones/6',
},
]);
wrapper.find(GlSearchBoxByType).vm.$emit('input', 'v0.1');
findSearchBox().vm.$emit('input', 'v0.1');
return axios.waitForAll().then(() => {
items = wrapper.findAll('[role="milestone option"]');
});
......@@ -147,4 +151,36 @@ describe('Milestone selector', () => {
expect(findNoResultsMessage().exists()).toBe(false);
});
});
describe('when Enter is pressed', () => {
beforeEach(() => {
factory({
propsData: {
projectId,
preselectedMilestones,
extraLinks,
},
data() {
return {
searchQuery: 'TEST_SEARCH',
};
},
});
mock
.onGet(TEST_SEARCH_ENDPOINT, { params: { search: 'TEST_SEARCH', scope: 'milestones' } })
.reply(200, []);
});
it('should trigger a search', async () => {
mock.resetHistory();
findSearchBox().vm.$emit('keydown', new KeyboardEvent({ key: ENTER_KEY }));
await axios.waitForAll();
expect(mock.history.get.length).toBe(1);
expect(mock.history.get[0].url).toBe(TEST_SEARCH_ENDPOINT);
});
});
});
......@@ -5,6 +5,7 @@ import MockAdapter from 'axios-mock-adapter';
import { GlLoadingIcon, GlSearchBoxByType, GlNewDropdownItem, GlIcon } from '@gitlab/ui';
import { trimText } from 'helpers/text_helper';
import { sprintf } from '~/locale';
import { ENTER_KEY } from '~/lib/utils/keys';
import RefSelector from '~/ref/components/ref_selector.vue';
import { X_TOTAL_HEADER, DEFAULT_I18N } from '~/ref/constants';
import createStore from '~/ref/stores/';
......@@ -83,6 +84,8 @@ describe('Ref selector component', () => {
const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
const findSearchBox = () => wrapper.find(GlSearchBoxByType);
const findBranchesSection = () => wrapper.find('[data-testid="branches-section"]');
const findBranchDropdownItems = () => findBranchesSection().findAll(GlNewDropdownItem);
const findFirstBranchDropdownItem = () => findBranchDropdownItems().at(0);
......@@ -120,7 +123,7 @@ describe('Ref selector component', () => {
// Convenience methods
//
const updateQuery = newQuery => {
wrapper.find(GlSearchBoxByType).vm.$emit('input', newQuery);
findSearchBox().vm.$emit('input', newQuery);
};
const selectFirstBranch = () => {
......@@ -244,6 +247,23 @@ describe('Ref selector component', () => {
});
});
describe('when the Enter is pressed', () => {
beforeEach(() => {
createComponent();
return waitForRequests({ andClearMocks: true });
});
it('requeries the endpoints when Enter is pressed', () => {
findSearchBox().vm.$emit('keydown', new KeyboardEvent({ key: ENTER_KEY }));
return waitForRequests().then(() => {
expect(branchesApiCallSpy).toHaveBeenCalledTimes(1);
expect(tagsApiCallSpy).toHaveBeenCalledTimes(1);
});
});
});
describe('when no results are found', () => {
beforeEach(() => {
branchesApiCallSpy = jest.fn().mockReturnValue([200, [], { [X_TOTAL_HEADER]: '0' }]);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment