Onboarding: Add TokensSettings Screen #517
Conversation
|
@pavloMandryk is attempting to deploy a commit to the 1hive Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Nice job @pavloMandryk
Left some corrections and suggestions. Let me know wdyt!
There was a problem hiding this comment.
Awesome work @pavloMandryk. I appreciate you kept receptive to our feedback and that you have addressed our suggestions super fast. Despite we drop the ball and took longer for us to fully review your work. It is great to have you on the team 🙌
| ) | ||
| }, []) | ||
|
|
||
| const handleNext = useCallback( |
There was a problem hiding this comment.
One thing that I notice with a final review is that when we do onBack we are not storing the config changes if they were any. I don't think this is particularly bad behavior. Just wanted to bring it up to have your oppinions.
There was a problem hiding this comment.
Regardless I think this PR is adding a lot of value. We can create a new one with that specific change on every onBack across screens if we decided to.
There was a problem hiding this comment.
That's right. I think we'll need to create a PR to fix it on every onBack. It is not such a rare practice, when using a installation wizard, moving backwards and performing changes. It would be nice to have support for it.
This PR contains the TokensSettings screen for the Onboarding, it consists in:
A folder with the Tokens icon for the Header.
The Header component used in every wizard screen.
The KnownAppBadge component to display the icons in the Header.
The TokenSettings main screen.
Adding changes to the useToken hook to fetch the token symbol, set loading only when an address is provided and treat an exception.