-
Notifications
You must be signed in to change notification settings - Fork 308
feat: update search on feeds table #3018
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
| return matchSorter(items, search, { | ||
| keys: ["displaySymbol", "symbol", "description", "key"], | ||
| }); |
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.
Is there any chance this call would be blocking? Perhaps the array is small enough that this isn't a problem
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 a few thousands items so I don't think it's going to impact too much
| predicate: (item: T, term: string) => boolean, | ||
| predicate: (item: T, search: string) => boolean, | ||
| doSort: (a: T, b: T, descriptor: SortDescriptor) => number, | ||
| doMutate: (items: T[], search: string) => T[], |
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.
Could probably have deleted predicate and doSort at this point since doMutate is a more open/powerful API anyway
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.
That's fair and I think at some point we can remove that. I didn't want to do a big refactor this time as it's used in a few places.
Summary
Changed the search behaviour for the price feeds card/search box to match the one in the search bar.
Rationale
How has this been tested?