-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
multiple personas clickable [Fixes #12645] #12753
base: dev
Are you sure you want to change the base?
multiple personas clickable [Fixes #12645] #12753
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks @bhargavkakadiya! First glance looks like a great start, but per original issue (#12645):
Switch logic to "AND" and allow users to select multiple personas.
Looks like the logic here results in an "OR" result where it'll filter wallets that match a persona OR filters that match another persona (widening results with more checks)...
@konopkja Am I correct that we want to narrow the results as more checkboxes are added?
Thanks for the contribution!!! yeas our goal is to narrow down |
… into multi-select-user-persona
@wackerow I have updated to show narrowed results, can you please review it again? |
ping @corwintines @wackerow |
@bhargavkakadiya in this example, end result should be 4 wallets or less, as it was narrowed to 4 when clicking the mobile filter |
… into multi-select-user-persona
… into multi-select-user-persona
So close! This is definitely nuanced logic... @bhargavkakadiya Replicated Nico's steps above and it kept narrowing down appropriately, but when I uncheck the "Finance" persona, the filters do not broaden again, and the results stay filtered to only three: Screen.Recording.2024-05-07.at.14.17.15.mov |
… into multi-select-user-persona
In this current version, the broadening is working upon deselecting persona. Also, based on the example below, when mobile is checked, it checks Android and iOS, which is fine. Screencast.from.05-14-2024.09.46.36.AM.webm |
Description
Allows multi select for personas
For display of wallet tables, when multiple personas are selected, filters are applied after applying AND on all persona preset filters from useWalletPersonas.tsx
The current filtering logic for displaying results is open for review. I welcome any suggestions or requirements for adjustments to ensure the functionality meets our needs
Preview link
https://deploy-preview-12753--ethereumorg.netlify.app/en/wallets/find-wallet
Related issue