[Issue #1491]: Setup states to hide clear all and select all in filter accordion #1495
Conversation
rylew1
requested review from
andycochran,
acouch and
SammySteiner
as code owners
| @@ -10,7 +10,7 @@ import SearchCallToAction from "../../components/search/SearchCallToAction"; | |||
| import { SearchForm } from "./SearchForm"; | |||
| import { convertSearchParamsToProperTypes } from "../../utils/convertSearchParamsToStrings"; | |||
| import { cookies } from "next/headers"; | |||
| import { getSearchFetcher } from "../../services/searchfetcher/SearchFetcherUtil"; | |||
| import { getSearchFetcher } from "../../services/search/searchfetcher/SearchFetcherUtil"; | |||
There was a problem hiding this comment.
I also moved the SearchFetcher classes from /services/searchfetcher to services/search/searchfetcher/ so there's a bunch of updates in this PR for that
rylew1
requested review from
andycochran,
acouch,
coilysiren and
chouinar
and removed request for
andycochran,
acouch and
SammySteiner
| : undefined, | ||
| })); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Start the creation of a SearchFilterManager class (currently not used). I think we should start to move some of the management of checked options here (how many total checked, how many in a section are checked, if all in a section are checked are not) - to make it easy to just grab these values instead of looping through all the options everytime. We still want useSearchFilter I think for react things like mounted, possibly calling the setter functions of useStates'
It probably won't lead to a ton of performance improvement though - just more for our own sanity :)
There was a problem hiding this comment.
Might try this in a part 2 PR on this #1491 ticket
There was a problem hiding this comment.
Cool. Should we put a TODO in this file noting as much (that it's not being used yet and a link to a ticket to implement/explore)?
There was a problem hiding this comment.
yea good catch - will add that!
| const [isSectionNoneSelected, setIsSectionNoneSelected] = useState<{ | ||
| [key: string]: boolean; | ||
| }>(initialSelectionStates.isSectionNoneSelected); | ||
|
|
There was a problem hiding this comment.
loads of logic to manage the {select,clear} all disabled state for both sections and non-sections
| @@ -84,6 +90,8 @@ export function SearchFilterAccordion({ | |||
| mounted={mounted} | |||
| updateCheckedOption={toggleOptionChecked} | |||
| toggleSelectAll={toggleSelectAll} | |||
| isSectionAllSelected={isSectionAllSelected[option.id]} | |||
| isSectionNoneSelected={isSectionNoneSelected[option.id]} | |||
| /> | |||
There was a problem hiding this comment.
Yea the logic to setup those values is kind of a lot but once you have it it's very easy to use in the components
| : undefined, | ||
| })); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Cool. Should we put a TODO in this file noting as much (that it's not being used yet and a link to a ticket to implement/explore)?
Summary
Fixes #1491
Time to review: 10 min
Changes proposed
searchfetcherclasses to/services/search/searchfetcherfolderContext for reviewers
useSearchFilteras it's getting a little hard to follow - created another ticket [Task]: Refactor useSearchFilter logic #1498Demo
Screen.Recording.2024-03-18.at.2.11.50.PM.mov