UI improvement: searchable list of networks #157

RevokeCash / revoke.cash

❌ Revoke or update your token approvals

https://revoke.cash

MIT License

674 stars 238 forks source link

UI improvement: searchable list of networks #157 #180

Closed sabalpoudel closed 9 months ago

sabalpoudel commented 9 months ago

Issue #157 UI improvement: searchable list of networks Issue#157

Hey there! This is my first pull request. I used react-select components to make text filtering work. I have added new components in the same file. Can you check if there's anything else I should improve? Thanks!

revokeCashIssue157.webm

vercel[bot] commented 9 months ago

@sabalpoudel is attempting to deploy a commit to the Kalis Software Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
revoke-cash	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 21, 2024 11:00am

rkalis commented 9 months ago

Hey @sabalpoudel this is great, thank you for opening this PR! I have a few comments:

In the old version, I can use the arrow keys to control the selection, and I can press ESC to close the select again. In the new version, the arrow keys and ESC key don't do anything. Is there a way we can combine the old keyboard interactions with the new searchability.
For some reason, when entering the search strings "e", "u", "i", "d", "f" or "n", the filtering doesn't apply. This isn't a huge issue, since it works when adding more than 1 letter, but it seems a bit weird.
Finally, would it be possible to abstract the functionality out of the ChainSelect into a SearchableSelect component, so that this functionality could be reused in other selects in the future. This is not too important, we can merge it in regardless, but it could be nice to have it abstracted into a separate component.

sabalpoudel commented 9 months ago

Hi @rkalis , Thank you for your feedback on the PR! I appreciate your thorough review. Here's my response:

I'll work on incorporating the old keyboard interactions into the new version. Restoring the functionality of arrow keys for selection and ESC to close select
Hmm that's odd the filtering is from react-select itself. I'll investigate the issue and try and improve it. Could you share how you observed filtering not working for the specific single-letter alphabets?
😬

rkalis commented 9 months ago

Thanks again @sabalpoudel, this is very nice. I made some very small changes and I applied your suggestion of moving the isMounted check to ChainLogo. 🫡

rkalis commented 9 months ago

I ended up extracting the functionality you added into a SearchableSelect component (see https://github.com/RevokeCash/revoke.cash/commit/7b8c3a8fb2394071e9e74f0054fa36cd8924b2c7), which can be used in place of the Select component if search functionality is needed. Thanks again for adding this!

sabalpoudel commented 9 months ago

Hmm, that's great @rkalis . Taking a look at (https://github.com/RevokeCash/revoke.cash/commit/7b8c3a8fb2394071e9e74f0054fa36cd8924b2c7), I now understand what you were asking of me earlier. I appreciate your update.