search filters #23
search filters #23
Conversation
fly in the search for providers - badges to represent filters - overlay for filter selection
| // onUpdateSelectedOptions | ||
| }: Props<T>) => { | ||
| const { setActiveFilter, updateFilter } = useProviderFilter(); | ||
| const isActive = filter.selectedOptions && filter.selectedOptions.length > 0; |
There was a problem hiding this comment.
To prevent unnecessary recalculations during re-renders, you could wrap this in useMemo
const isActive = useMemo(() => filter.selectedOptions && filter.selectedOptions.length > 0, [filter.selectedOptions]);| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| const updatedFilter = { | ||
| ...filter, | ||
| selectedOptions: [] | ||
| } as unknown as FilterType<FilterEnum>; | ||
| updateFilter(updatedFilter); | ||
| }} |
There was a problem hiding this comment.
For readability, I'd suggest putting this in a separate function
const handleBadgeClick = useCallback(
(e) => {
e.stopPropagation();
const updatedFilter = {
...filter,
selectedOptions: []
} as unknown as FilterType<FilterEnum>;
updateFilter(updatedFilter);
},
[filter, updateFilter]
);<SVG
onClick={handleBadgeClick}
...
/>| // onUpdateSelectedOptions: (filter: FilterType<T>) => void; | ||
| } | ||
|
|
||
| const FilterBadge = <T extends FilterEnum>({ |
There was a problem hiding this comment.
Why not export const ...? Then we don't need this awkward export { FilterOverlay } at the bottom of the file + we use consistent way of exporting components throughout all the files.
| const FilterBadge = <T extends FilterEnum>({ | |
| export const FilterBadge:FC<Props> = ({filter}) => { | |
| ... | |
| } |
| const onTextFilteredOptionsChange = (value: string) => { | ||
| const opts = filter.options.filter( | ||
| (option) => | ||
| String(option.label).toLowerCase().includes(value.toLowerCase()) || | ||
| String(option.value).toLowerCase().includes(value.toLowerCase()) | ||
| ); | ||
| setTextFilteredOptions(opts); | ||
| }; |
There was a problem hiding this comment.
Could benefit from useCallback
| const onSelectOption = (value: string) => { | ||
| if (filter.selectType === 'single') { | ||
| updateFilter({ | ||
| ...filter, | ||
| selectedOptions: [value] | ||
| }); | ||
| } else { | ||
| updateFilter({ | ||
| ...filter, | ||
| selectedOptions: filter.selectedOptions.includes(value) | ||
| ? filter.selectedOptions.filter((v) => v !== value) | ||
| : [...filter.selectedOptions, value] | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
could benefit from useCallback
| const updateFilters = debounce((newFilters: FilterType<FilterEnum>[]) => { | ||
| const updatedPreferences = updatePreferencesWithFilters( | ||
| initialPreferences, | ||
| newFilters | ||
| ); | ||
| onProviderPreferencesChange(updatedPreferences); | ||
| }, 500); |
| const updateFilter = (filter: FilterType<FilterEnum>) => { | ||
| const updatedFilters = filters.map((f) => { | ||
| if (f.key === filter.key) { | ||
| return filter; | ||
| } | ||
| return f; | ||
| }); | ||
| setFilters(updatedFilters); | ||
| updateFilters(updatedFilters); |
| const changeActiveFilter = ( | ||
| newFilterKey: keyof GetProvidersInputType | null | ||
| ) => { | ||
| if (newFilterKey === null || newFilterKey === activeFilter) { | ||
| setActiveFilter(null); | ||
| } else if (newFilterKey !== null) { | ||
| setActiveFilter(newFilterKey); | ||
| } | ||
| }; |
| const getActiveFilter = () => { | ||
| return filters.find( | ||
| (f) => f.key === activeFilter | ||
| ) as FilterType<FilterEnum>; | ||
| }; |
| const [providerPrefs, setProviderPrefs] = useState<GetProvidersInputType>( | ||
| args.providerPreferences as GetProvidersInputType | ||
| ); | ||
|
|
||
| const handlePrefsChange = (prefs: GetProvidersInputType) => { | ||
| console.log('Prefs changed', prefs); | ||
| setProviderPrefs(prefs); | ||
| fetchProvidersFn(); | ||
| }; | ||
|
|
||
| const fetchProvidersFn = useCallback(async () => { | ||
| const { data, ...rest } = await args.fetchProviders(); | ||
| console.log('fetched providers'); | ||
| return { | ||
| data: data.filter((p) => { | ||
| if ( | ||
| providerPrefs.clinicalFocus && | ||
| providerPrefs.clinicalFocus.length > 0 | ||
| ) { | ||
| return some( | ||
| providerPrefs.clinicalFocus.map((f) => | ||
| p.clinicalFocus | ||
| ?.map((cf) => cf.toLowerCase()) | ||
| .includes(f.toLowerCase()) | ||
| ) | ||
| ); | ||
| } else { | ||
| return true; | ||
| } | ||
| }), | ||
| ...rest | ||
| }; | ||
| }, [providerPrefs]); |
There was a problem hiding this comment.
Challenge: IMO this logic should live inside the SchedulingActivity component and not in the Story/Hosted Pages? I think this would require us to parameterize the `fetchProviders function so it includes the preferences to make that happen?
That way, we avoid business logic inside the story and Hosted Pages.
const [providerPrefs, setProviderPrefs] = useState<GetProvidersInputType>(
args.providerPreferences as GetProvidersInputType
);
// When preferences change, we force the component to call `fetchProviders` again
// This is logic we can add in SchedulingActivity.tsx
const handlePrefsChange = useCallback(() => args.onProviderPreferencesChange(), []);
// providerPrefs becomes a parameter of the fetch function
const fetchProvidersFn = useCallback((providerPrefs) => args.fetchProviders(providerPrefs), []);There was a problem hiding this comment.
i wondered about this myself. we can actually reduce the number of props to the activity if we do that. so, 👍
|
thanks for the feedback! i also need to move the close button from the selector to the overlay. re: useCallback, i'm not excited about putting it in a bunch of places because (1) it reduces readability; and (2) we're not making any external calls anywhere so the benefits are quite minimal. so, on the whole, i don't think it adds value. when we move the fetchProvidersFn so it handles prefs, i'll add useCallback for that. 👍 |
support search filters with multi-select and single-select