Skip to content

Commit

Permalink
Update default FilterSearch behavior (#333)
Browse files Browse the repository at this point in the history
Update the default behavior of `FilterSearch`:
- On select, clobber other static filters in State that have a `fieldId` that matches with one of the search fields for `FilterSearch`. If there are multiple filters that were clobbered, log a console warning.
- If the static filters state is updated from outside of the component, log a console warning if there are multiple "matching" filters in State and there is no custom `onSelect` prop passed in.
  - And if there is no filter currently associated with the component, update the input text to the display name of the first "matching" filter that is selected in State.
    - If there are no matching filters in State, clear out the input and filter search response.

Note: We only look for field value filters in State for the "matching" filters. We don't prescribe how compound filters should be handled when comparing one to a search field. In the UCSD use case, a developer would pass in a custom `onSelect` function that would set compound static filters in State. In this case, the `currentFilter` may not be part of the `matchingFilters` array, which is why the concept of a `currentFilter` is still needed.

J=SLAP-2432
TEST=auto, manual

See that the added Jest tests pass. Spin up the test-site and test that the above situations match the expected behavior with different combinations of filters in State and passing an `onSelect` prop or not.
  • Loading branch information
nmanu1 authored Nov 15, 2022
1 parent 8422231 commit a2e8319
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 10 deletions.
53 changes: 48 additions & 5 deletions src/components/FilterSearch.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AutocompleteResult, FieldValueStaticFilter, FilterSearchResponse, SearchParameterField, StaticFilter, useSearchActions, useSearchState } from '@yext/search-headless-react';
import { AutocompleteResult, FieldValueStaticFilter, FilterSearchResponse, SearchParameterField, SelectableStaticFilter, StaticFilter, useSearchActions, useSearchState } from '@yext/search-headless-react';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useComposedCssClasses } from '../hooks/useComposedCssClasses';
import { useSynchronizedRequest } from '../hooks/useSynchronizedRequest';
Expand Down Expand Up @@ -109,6 +109,13 @@ export function FilterSearch({
const [currentFilter, setCurrentFilter] = useState<StaticFilter>();
const [filterQuery, setFilterQuery] = useState<string>();
const staticFilters = useSearchState(state => state.filters.static);
const matchingFilters: SelectableStaticFilter[] = useMemo(() => {
return staticFilters?.filter(({ filter, selected }) =>
selected
&& filter.kind === 'fieldValue'
&& searchFields.some(s => s.fieldApiName === filter.fieldId)
) ?? [];
}, [staticFilters, searchFields]);

const [
filterSearchResponse,
Expand All @@ -123,14 +130,36 @@ export function FilterSearch({
);

useEffect(() => {
if (matchingFilters.length > 1 && !onSelect) {
console.warn('More than one selected static filter found that matches the filter search fields: ['
+ searchFields.map(s => s.fieldApiName).join(', ')
+ ']. Please update the state to remove the extra filters.'
+ ' Picking one filter to display in the input.');
}

if (currentFilter && staticFilters?.find(f =>
isDuplicateStaticFilter(f.filter, currentFilter) && !f.selected
isDuplicateStaticFilter(f.filter, currentFilter) && f.selected
)) {
return;
}

if (matchingFilters.length === 0) {
clearFilterSearchResponse();
setCurrentFilter(undefined);
setFilterQuery('');
} else {
setCurrentFilter(matchingFilters[0].filter);
executeFilterSearch(matchingFilters[0].displayName);
}
}, [clearFilterSearchResponse, currentFilter, staticFilters]);
}, [
clearFilterSearchResponse,
currentFilter,
staticFilters,
executeFilterSearch,
onSelect,
matchingFilters,
searchFields
]);

const sections = useMemo(() => {
return filterSearchResponse?.sections.filter(section => section.results.length > 0) ?? [];
Expand All @@ -148,7 +177,7 @@ export function FilterSearch({
if (onSelect) {
if (searchOnSelect) {
console.warn('Both searchOnSelect and onSelect props were passed to the component.'
+ ' Using onSelect instead of searchOnSelect as the latter is deprecated.');
+ ' Using onSelect instead of searchOnSelect as the latter is deprecated.');
}
return onSelect({
newFilter,
Expand All @@ -159,6 +188,12 @@ export function FilterSearch({
});
}

if (matchingFilters.length > 1) {
console.warn('More than one selected static filter found that matches the filter search fields: ['
+ searchFields.map(s => s.fieldApiName).join(', ')
+ ']. Unselecting all existing matching filters and selecting the new filter.');
}
matchingFilters.forEach(f => searchActions.setFilterOption({ filter: f.filter, selected: false }));
if (currentFilter) {
searchActions.setFilterOption({ filter: currentFilter, selected: false });
}
Expand All @@ -171,7 +206,15 @@ export function FilterSearch({
searchActions.resetFacets();
executeSearch(searchActions);
}
}, [currentFilter, searchActions, executeFilterSearch, onSelect, searchOnSelect]);
}, [
currentFilter,
searchActions,
executeFilterSearch,
onSelect,
searchOnSelect,
matchingFilters,
searchFields
]);

const meetsSubmitCritera = useCallback(index => index >= 0, []);

Expand Down
164 changes: 159 additions & 5 deletions tests/components/FilterSearch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,38 @@ const mockedState: Partial<State> = {
}
};

const mockedStateWithSingleFilter: Partial<State> = {
...mockedState,
filters: {
static: [{
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'Real Person'
},
selected: true,
displayName: 'Real Person'
}]
}
};

const mockedStateWithMultipleFilters: Partial<State> = {
...mockedState,
filters: {
static: [...(mockedStateWithSingleFilter.filters?.static ?? []), {
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'Fake Person'
},
selected: true,
displayName: 'Fake Person'
}]
}
};

describe('search with section labels', () => {
it('renders the filter search bar, "Filter" label, and default placeholder text', () => {
renderFilterSearch({ searchFields: searchFieldsProp, label: 'Filter' });
Expand Down Expand Up @@ -179,6 +211,125 @@ describe('search with section labels', () => {
});
});

it('displays name of matching filter in state when no filter is selected from component', async () => {
renderFilterSearch(undefined, mockedStateWithSingleFilter);
const searchBarElement = screen.getByRole('textbox');
expect(searchBarElement).toHaveValue('Real Person');
});

it('logs a warning when multiple matching filters in state and no current filter selected', async () => {
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockImplementation();
renderFilterSearch(undefined, mockedStateWithMultipleFilters);
const searchBarElement = screen.getByRole('textbox');
expect(searchBarElement).toHaveValue('Real Person');
expect(consoleWarnSpy).toBeCalledWith(
'More than one selected static filter found that matches the filter search fields: [name].'
+ ' Please update the state to remove the extra filters.'
+ ' Picking one filter to display in the input.'
);
});

it('does not log a warning for multiple matching filters in state if onSelect is passed', async () => {
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockImplementation();
const mockedOnSelect = jest.fn();
renderFilterSearch(
{ searchFields: searchFieldsProp, onSelect: mockedOnSelect },
mockedStateWithMultipleFilters
);
const searchBarElement = screen.getByRole('textbox');
expect(searchBarElement).toHaveValue('Real Person');
expect(consoleWarnSpy).not.toHaveBeenCalled();
});

it('unselects single matching filter in state when a new filter is selected and doesn\'t log warning', async () => {
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockImplementation();
renderFilterSearch(undefined, mockedStateWithSingleFilter);
const executeFilterSearch = jest
.spyOn(SearchHeadless.prototype, 'executeFilterSearch')
.mockResolvedValue(labeledFilterSearchResponse);
const setFilterOption = jest.spyOn(SearchHeadless.prototype, 'setFilterOption');
const searchBarElement = screen.getByRole('textbox');

userEvent.clear(searchBarElement);
userEvent.type(searchBarElement, 'n');
await waitFor(() => expect(executeFilterSearch).toHaveBeenCalled());
await waitFor(() => screen.findByText('first name 1'));
userEvent.type(searchBarElement, '{enter}');
await waitFor(() => {
expect(setFilterOption).toBeCalledWith({
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'Real Person'
},
selected: false
});
});
expect(setFilterOption).toBeCalledWith({
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'first name 1'
},
displayName: 'first name 1',
selected: true
});

expect(consoleWarnSpy).not.toHaveBeenCalled();
});

it('unselects multiple matching filters in state when a new filter is selected and logs warning', async () => {
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockImplementation();
renderFilterSearch(undefined, mockedStateWithMultipleFilters);
const executeFilterSearch = jest
.spyOn(SearchHeadless.prototype, 'executeFilterSearch')
.mockResolvedValue(labeledFilterSearchResponse);
const setFilterOption = jest.spyOn(SearchHeadless.prototype, 'setFilterOption');
const searchBarElement = screen.getByRole('textbox');

userEvent.clear(searchBarElement);
userEvent.type(searchBarElement, 'n');
await waitFor(() => expect(executeFilterSearch).toHaveBeenCalled());
await waitFor(() => screen.findByText('first name 1'));
userEvent.type(searchBarElement, '{enter}');
await waitFor(() => {
expect(setFilterOption).toBeCalledWith({
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'Real Person'
},
selected: false
});
});
expect(setFilterOption).toBeCalledWith({
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'Fake Person'
},
selected: false
});
expect(setFilterOption).toBeCalledWith({
filter: {
kind: 'fieldValue',
fieldId: 'name',
matcher: Matcher.Equals,
value: 'first name 1'
},
displayName: 'first name 1',
selected: true
});
expect(consoleWarnSpy).toBeCalledWith(
'More than one selected static filter found that matches the filter search fields: [name].'
+ ' Unselecting all existing matching filters and selecting the new filter.'
);
});

it('executes onSelect function when a filter is selected', async () => {
const mockedOnSelect = jest.fn();
const setFilterOption = jest.spyOn(SearchHeadless.prototype, 'setFilterOption');
Expand Down Expand Up @@ -329,7 +480,7 @@ describe('search with section labels', () => {
expect(setFilterOption).not.toBeCalled();
expect(mockExecuteSearch).not.toBeCalled();
expect(consoleWarnSpy).toBeCalledWith('Both searchOnSelect and onSelect props were passed to the component.'
+ ' Using onSelect instead of searchOnSelect as the latter is deprecated.');
+ ' Using onSelect instead of searchOnSelect as the latter is deprecated.');
});
});
});
Expand Down Expand Up @@ -460,8 +611,11 @@ describe('screen reader', () => {
});
});

function renderFilterSearch(props: FilterSearchProps = { searchFields: searchFieldsProp }): RenderResult {
return render(<SearchHeadlessContext.Provider value={generateMockedHeadless(mockedState)}>
function renderFilterSearch(
props: FilterSearchProps = { searchFields: searchFieldsProp },
state = mockedState
): RenderResult {
return render(<SearchHeadlessContext.Provider value={generateMockedHeadless(state)}>
<FilterSearch {...props} />
</SearchHeadlessContext.Provider>);
}
Expand Down Expand Up @@ -490,15 +644,15 @@ it('clears input when old filters are removed', async () => {
};
return (
<button onClick={handleClickDeselectFilter}>
Deselect Filter
Deselect Filter
</button>
);

}

render(<SearchHeadlessContext.Provider value={generateMockedHeadless(mockedState)}>
<FilterSearch searchFields={searchFieldsProp} />
<DeselectFiltersButton/>
<DeselectFiltersButton />
</SearchHeadlessContext.Provider>);

const searchBarElement = screen.getByRole('textbox');
Expand Down

0 comments on commit a2e8319

Please sign in to comment.