Commit 301f366e authored by Mark Florian's avatar Mark Florian

Simplify clear logic for filtered search bar root

This changes the behaviour of the `FilteredSearchBarRoot` component
slightly. Previously, the `onFilter` event would fire when the user
pressed backspace repeatedly to clear all the tokens. This event usually
triggers a search to be performed, either as a redirect or an async
request.

Now, under those circumstances, that event is *not* fired. In order for
a user to initiate a new search when there are no search tokens, they
must either:

- Hit enter after removing all search tokens with backspace. This makes
  it more consistent with *adding* tokens: the search is only performed
  on hitting Enter.
- Click the "clear" button in `GlFilteredSearch`. This behaviour hasn't
  changed.

Without this commit, on visiting the Project Members page (and possibly
others), it would immediately reload, repeatedly, making the page was
unusable. Various tests also broke with uninformative `Net:ReadTimeout`
errors from Selenium.

It turned out this was due to some recent changes to `GlFilteredSearch`,
particularly
https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/2732 and
https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/2736, which
changed how and when `input` events are fired from `GlFilteredSearch`.

In debugging this, I encountered and wrote up
https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1789, which I suspect
is the reason why the `filterValue` watcher was written this way in the
first place. That bug should be fixed soon, since in Chrome, backspacing
to clear a segmented token removes focus from the search input, so
hitting Enter does not initiate a new search. This works correctly in
Firefox, however.
parent b84ccc1e
...@@ -163,33 +163,6 @@ export default { ...@@ -163,33 +163,6 @@ export default {
return undefined; return undefined;
}, },
}, },
watch: {
/**
* GlFilteredSearch currently doesn't emit any event when
* tokens are manually removed from search field so we'd
* never know when user actually clears all the tokens.
* This watcher listens for updates to `filterValue` on
* such instances. :(
*/
filterValue(newValue, oldValue) {
const [firstVal] = newValue;
if (
!this.initialRender &&
newValue.length === 1 &&
firstVal.type === 'filtered-search-term' &&
!firstVal.value.data
) {
const filtersCleared =
oldValue[0].type !== 'filtered-search-term' || oldValue[0].value.data !== '';
this.$emit('onFilter', [], filtersCleared);
}
// Set initial render flag to false
// as we don't want to emit event
// on initial load when value is empty already.
this.initialRender = false;
},
},
created() { created() {
if (this.recentSearchesStorageKey) this.setupRecentSearch(); if (this.recentSearchesStorageKey) this.setupRecentSearch();
}, },
...@@ -322,6 +295,10 @@ export default { ...@@ -322,6 +295,10 @@ export default {
return tokenOption.title; return tokenOption.title;
}, },
onClear() {
const cleared = true;
this.$emit('onFilter', [], cleared);
},
}, },
}; };
</script> </script>
...@@ -345,6 +322,7 @@ export default { ...@@ -345,6 +322,7 @@ export default {
:suggestions-list-class="suggestionsListClass" :suggestions-list-class="suggestionsListClass"
class="flex-grow-1" class="flex-grow-1"
@history-item-selected="handleHistoryItemSelected" @history-item-selected="handleHistoryItemSelected"
@clear="onClear"
@clear-history="handleClearHistory" @clear-history="handleClearHistory"
@submit="handleFilterSubmit" @submit="handleFilterSubmit"
> >
......
...@@ -26,7 +26,6 @@ import { ...@@ -26,7 +26,6 @@ import {
tokenValueMilestone, tokenValueMilestone,
tokenValueMembership, tokenValueMembership,
tokenValueConfidential, tokenValueConfidential,
tokenValueEmpty,
} from './mock_data'; } from './mock_data';
jest.mock('~/vue_shared/components/filtered_search_bar/filtered_search_utils', () => ({ jest.mock('~/vue_shared/components/filtered_search_bar/filtered_search_utils', () => ({
...@@ -207,33 +206,14 @@ describe('FilteredSearchBarRoot', () => { ...@@ -207,33 +206,14 @@ describe('FilteredSearchBarRoot', () => {
}); });
}); });
describe('watchers', () => { describe('events', () => {
describe('filterValue', () => { it('emits component event `onFilter` with empty array and true when initially selected filter value was cleared', async () => {
it('emits component event `onFilter` with empty array and false when filter was never selected', async () => { wrapper = createComponent({ initialFilterValue: [tokenValueLabel] });
wrapper = createComponent({ initialFilterValue: [tokenValueEmpty] });
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
initialRender: false,
filterValue: [tokenValueEmpty],
});
await nextTick();
expect(wrapper.emitted('onFilter')[0]).toEqual([[], false]);
});
it('emits component event `onFilter` with empty array and true when initially selected filter value was cleared', async () => { wrapper.find(GlFilteredSearch).vm.$emit('clear');
wrapper = createComponent({ initialFilterValue: [tokenValueLabel] });
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
initialRender: false,
filterValue: [tokenValueEmpty],
});
await nextTick(); await nextTick();
expect(wrapper.emitted('onFilter')[0]).toEqual([[], true]); expect(wrapper.emitted('onFilter')[0]).toEqual([[], true]);
});
}); });
}); });
......
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