Skip to content
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

Update network selector #62

Merged
merged 11 commits into from
Feb 16, 2024
Merged

Update network selector #62

merged 11 commits into from
Feb 16, 2024

Conversation

Pabl0cks
Copy link
Member

Description

Switching regular selector for networks with react-select. Tried using react-tailwindcss-select but did not find an easy way to add network icons to the selector. The main benefits of upgrading the selector:

  • Improved customizable UI
  • Searchable
  • Grouped networks (local, mainnets, testnets). If we see local too granular and prefer to add it to the testnets group, lmk!

A few considerations to check:

  • Added a different icon asset for each network, in case we want to set some visual change for testnets icons in the future, but at the moment they are using the same image as mainnets. Could just reuse the same asset file for now if we see it cleaner.
  • While testing in mobile, it was very annoying to have the keyboard opened because of the search input.
    That's the reason of 29a1eda. If it feels too hacky could just set isSearchable={false} to all devices.

Related Issues

Closes #60

Copy link

vercel bot commented Feb 14, 2024

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

Name Status Preview Comments Updated (UTC)
abi-ninja-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 16, 2024 11:25am

@technophile-04
Copy link
Member

TYSM Pablo this is really great !!!!

Just pushed some commits,

  1. Moved NetworksDropDown to its own component, since it needed alot of specific stuff for initializing it and logic like isMobile was also bloating the index.tsx a bit
  2. Instead of marking each network with Mainnet our Testenet, relying on viems chains since they aready have testnet boolean to denote if its testnet

Could just reuse the same asset file for now if we see it cleaner.

Yup I think we should probably remove duplicate testnets icons nd reuse the mainnet one for them, I think its fine to have same icon for test and mainnet because we are already separating them with groups, we can always add different images for testnet if we feel so in future 🙌

That's the reason of 29a1eda. If it feels too hacky could just set isSearchable={false} to all devices.

Hmm yup makes sense ! I love the feature of search on desktop and we should keep it, yup on mobile its annoying :( not sure if its only for IOS but its also zoom's in the screen on mobile so makes sense to disable it for mobile, we can also keep an eye on this discussion JedWatson/react-select#3526

@technophile-04 technophile-04 merged commit f4d3b60 into main Feb 16, 2024
3 checks passed
@Pabl0cks
Copy link
Member Author

Thanks A LOT for the awesome feedback and cleaning + upgrading the code quality. Learn a lot from checking your reviews 🙌♥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a better <select> for the main network selector
2 participants