-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Version2: Add certificate search logic #167
base: donskov/v2
Are you sure you want to change the base?
Version2: Add certificate search logic #167
Conversation
import { CertificateProps } from "../../types"; | ||
|
||
export function useSearchList(certificates: CertificateProps[]) { | ||
const searchParams = new URLSearchParams(window.location.search); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use the searchParams
only for default state prefilling, but this code calls on each rerender. We have to fix this and parse parameters only 1 time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return { | ||
searchedText, | ||
list: searchedText | ||
? certificates.filter(({ label }) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same situation with multiple re-renders. We should try to make filtration only when the searchedText
or the certificates
have changed. Now, even when opening a dialog, we filter certificates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OleksandrSPV Not related to these changes, but is there any reason to have the search state in the search hook and two search states in the component? Can we use only a single source of truth? |
@donskov I needed to use delayed input, so I decided use this part near the input component not in search hook. |
Yeah, let's remove it. |
|
Do we also need the search input state ( |
Screen.Recording.2024-07-04.at.18.56.29.mov