Commit 4b7be85b authored by Brandon Labuschagne's avatar Brandon Labuschagne

Merge branch '331029-improve-jira-connect-search-ux' into 'master'

Improve Jira Connect search UX

See merge request gitlab-org/gitlab!78994
parents eb2d5760 3f22e424
...@@ -30,7 +30,8 @@ export default { ...@@ -30,7 +30,8 @@ export default {
page: 1, page: 1,
totalItems: 0, totalItems: 0,
errorMessage: null, errorMessage: null,
searchTerm: '', userSearchTerm: '',
searchValue: '',
}; };
}, },
computed: { computed: {
...@@ -45,16 +46,11 @@ export default { ...@@ -45,16 +46,11 @@ export default {
}, },
methods: { methods: {
loadGroups() { loadGroups() {
// fetchGroups returns no results for search terms 0 < {length} < 3.
// The desired UX is to return the unfiltered results for searches {length} < 3.
// Here, we set the search to an empty string if {length} < 3
const search = this.searchTerm?.length < MINIMUM_SEARCH_TERM_LENGTH ? '' : this.searchTerm;
this.isLoadingMore = true; this.isLoadingMore = true;
return fetchGroups(this.groupsPath, { return fetchGroups(this.groupsPath, {
page: this.page, page: this.page,
perPage: this.$options.DEFAULT_GROUPS_PER_PAGE, perPage: this.$options.DEFAULT_GROUPS_PER_PAGE,
search, search: this.searchValue,
}) })
.then((response) => { .then((response) => {
const { page, total } = parseIntPagination(normalizeHeaders(response.headers)); const { page, total } = parseIntPagination(normalizeHeaders(response.headers));
...@@ -69,12 +65,24 @@ export default { ...@@ -69,12 +65,24 @@ export default {
this.isLoadingMore = false; this.isLoadingMore = false;
}); });
}, },
onGroupSearch(searchTerm) { onGroupSearch(userSearchTerm = '') {
// keep a copy of the search term for pagination this.userSearchTerm = userSearchTerm;
this.searchTerm = searchTerm;
// reset the current page // fetchGroups returns no results for search terms 0 < {length} < 3.
// The desired UX is to return the unfiltered results for searches {length} < 3.
// Here, we set the search to an empty string '' if {length} < 3
const newSearchValue =
this.userSearchTerm.length < MINIMUM_SEARCH_TERM_LENGTH ? '' : this.userSearchTerm;
// don't fetch new results if the search value didn't change.
if (newSearchValue === this.searchValue) {
return;
}
// reset the page.
this.page = 1; this.page = 1;
return this.loadGroups(); this.searchValue = newSearchValue;
this.loadGroups();
}, },
}, },
DEFAULT_GROUPS_PER_PAGE, DEFAULT_GROUPS_PER_PAGE,
...@@ -92,7 +100,7 @@ export default { ...@@ -92,7 +100,7 @@ export default {
debounce="500" debounce="500"
:placeholder="__('Search by name')" :placeholder="__('Search by name')"
:is-loading="isLoadingMore" :is-loading="isLoadingMore"
:value="searchTerm" :value="userSearchTerm"
@input="onGroupSearch" @input="onGroupSearch"
/> />
......
...@@ -101,6 +101,8 @@ describe('GroupsList', () => { ...@@ -101,6 +101,8 @@ describe('GroupsList', () => {
}); });
createComponent(); createComponent();
// wait for the initial loadGroups
// to finish.
await waitForPromises(); await waitForPromises();
}); });
...@@ -137,6 +139,8 @@ describe('GroupsList', () => { ...@@ -137,6 +139,8 @@ describe('GroupsList', () => {
describe('while groups are loading', () => { describe('while groups are loading', () => {
beforeEach(async () => { beforeEach(async () => {
fetchGroups.mockClear(); fetchGroups.mockClear();
// return a never-ending promise to make test
// deterministic.
fetchGroups.mockReturnValue(new Promise(() => {})); fetchGroups.mockReturnValue(new Promise(() => {}));
findSearchBox().vm.$emit('input', mockSearchTeam); findSearchBox().vm.$emit('input', mockSearchTeam);
...@@ -173,7 +177,7 @@ describe('GroupsList', () => { ...@@ -173,7 +177,7 @@ describe('GroupsList', () => {
describe('when group search finishes loading', () => { describe('when group search finishes loading', () => {
beforeEach(async () => { beforeEach(async () => {
fetchGroups.mockResolvedValue({ data: [mockGroup1] }); fetchGroups.mockResolvedValue({ data: [mockGroup1] });
findSearchBox().vm.$emit('input'); findSearchBox().vm.$emit('input', mockSearchTeam);
await waitForPromises(); await waitForPromises();
}); });
...@@ -184,32 +188,48 @@ describe('GroupsList', () => { ...@@ -184,32 +188,48 @@ describe('GroupsList', () => {
}); });
}); });
it.each` describe.each`
userSearchTerm | finalSearchTerm previousSearch | newSearch | shouldSearch | expectedSearchValue
${'gitl'} | ${'gitl'} ${''} | ${'git'} | ${true} | ${'git'}
${'git'} | ${'git'} ${'g'} | ${'git'} | ${true} | ${'git'}
${'gi'} | ${''} ${'git'} | ${'gitl'} | ${true} | ${'gitl'}
${'g'} | ${''} ${'git'} | ${'gi'} | ${true} | ${''}
${''} | ${''} ${'gi'} | ${'g'} | ${false} | ${undefined}
${undefined} | ${undefined} ${'g'} | ${''} | ${false} | ${undefined}
${''} | ${'g'} | ${false} | ${undefined}
`( `(
'searches for "$finalSearchTerm" when user enters "$userSearchTerm"', 'when previous search was "$previousSearch" and user enters "$newSearch"',
async ({ userSearchTerm, finalSearchTerm }) => { ({ previousSearch, newSearch, shouldSearch, expectedSearchValue }) => {
beforeEach(async () => {
fetchGroups.mockResolvedValue({ fetchGroups.mockResolvedValue({
data: [mockGroup1], data: [mockGroup1],
headers: { 'X-PAGE': 1, 'X-TOTAL': 1 }, headers: { 'X-PAGE': 1, 'X-TOTAL': 1 },
}); });
// wait for initial load
createComponent(); createComponent();
await waitForPromises(); await waitForPromises();
const searchBox = findSearchBox(); // set up the "previous search"
searchBox.vm.$emit('input', userSearchTerm); findSearchBox().vm.$emit('input', previousSearch);
await waitForPromises();
expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, { fetchGroups.mockClear();
});
it(`${shouldSearch ? 'should' : 'should not'} execute fetch new results`, () => {
// enter the new search
findSearchBox().vm.$emit('input', newSearch);
if (shouldSearch) {
expect(fetchGroups).toHaveBeenCalledWith(mockGroupsPath, {
page: 1, page: 1,
perPage: DEFAULT_GROUPS_PER_PAGE, perPage: DEFAULT_GROUPS_PER_PAGE,
search: finalSearchTerm, search: expectedSearchValue,
});
} else {
expect(fetchGroups).not.toHaveBeenCalled();
}
}); });
}, },
); );
...@@ -227,7 +247,13 @@ describe('GroupsList', () => { ...@@ -227,7 +247,13 @@ describe('GroupsList', () => {
await waitForPromises(); await waitForPromises();
const paginationEl = findPagination(); const paginationEl = findPagination();
paginationEl.vm.$emit('input', 2);
// mock the response from page 2
fetchGroups.mockResolvedValue({
headers: { 'X-TOTAL': totalItems, 'X-PAGE': 2 },
data: mockGroups,
});
await paginationEl.vm.$emit('input', 2);
}); });
it('should load results for page 2', () => { it('should load results for page 2', () => {
...@@ -238,18 +264,23 @@ describe('GroupsList', () => { ...@@ -238,18 +264,23 @@ describe('GroupsList', () => {
}); });
}); });
it('resets page to 1 on search `input` event', () => { it.each`
const mockSearchTerm = 'gitlab'; scenario | searchTerm | expectedPage | expectedSearchTerm
${'preserves current page'} | ${'gi'} | ${2} | ${''}
${'resets page to 1'} | ${'gitlab'} | ${1} | ${'gitlab'}
`(
'$scenario when search term is $searchTerm',
({ searchTerm, expectedPage, expectedSearchTerm }) => {
const searchBox = findSearchBox(); const searchBox = findSearchBox();
searchBox.vm.$emit('input', searchTerm);
searchBox.vm.$emit('input', mockSearchTerm);
expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, { expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, {
page: 1, page: expectedPage,
perPage: DEFAULT_GROUPS_PER_PAGE, perPage: DEFAULT_GROUPS_PER_PAGE,
search: mockSearchTerm, search: expectedSearchTerm,
});
}); });
},
);
}); });
}); });
......
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