Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add RPC selection to onboarding flow #868
Add RPC selection to onboarding flow #868
Changes from 17 commits
7b9ec42
d3fa1d0
8674da3
0d653d1
4b6e697
ab697d8
2c7d979
d4c0d4d
1705365
5113ed7
e9e1ed9
9d32e80
10efebb
a0378fe
412d3b3
539914f
ffed7de
d32ccab
2dc36af
c969590
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Note that I didn't make this change in the
RestorePassword
component. It doesn't seem to be part of the onboarding flow, and I actually am unclear on if that component is actually used anywhere?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.
Feel free to delete it. It's unused for like 8 months.
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.
What do you think about moving all this logic into zustand with an RPC slice? My hunch is perhaps we'd want to re-use this within our settings tab in the extension and moving the logic out likely makes this easier.
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 also reduces the redundancies with
setGrpcEndpointInZustand
&setGrpcEndpoint
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.
@turbocrime please confirm that this is correct (moving
OnboardComplete
) to this step, since this is the new last step of onboarding.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.
Can we do some validation like the way we do in settings right now? If it cannot connect to the grpc url, we do not allow it to be set.
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.
Going to take the liberty of doing this (and addressing your comment above about moving logic into Zustand) in a separate PR, since it looks like that'll require some (necessary) refactoring that I was anyway going to do when adding a similar RPC selection form to the extension's settings page.