Set gRPC endpoint during onboarding, part 2 #877
Conversation
jessepinho
force-pushed
the
jessepinho/rpc-endpoint-part-2-web-605
branch
from
cd7806e to
9057be9
Compare
| @@ -6,7 +6,7 @@ import { ImportSeedPhrase } from './import'; | |||
| import { OnboardingSuccess } from './success'; | |||
| import { SetPassword } from './set-password'; | |||
| import { pageIndexLoader } from '..'; | |||
| import { SetRpcEndpoint } from './set-rpc-endpoint'; | |||
| import { SetGrpcEndpoint } from './set-grpc-endpoint'; | |||
There was a problem hiding this comment.
Consolidating the naming
There was a problem hiding this comment.
would prefer to align on "rpc", it's a more generic term
There was a problem hiding this comment.
Ugh, same. I'll merge this and then discuss in Discord. The issue was, the local storage key is grpcEndpoint, so I figured we should standardize on that since it's already in a bunch of users' local storage.
| import { ServicesMessage } from '@penumbra-zone/types/src/services'; | ||
| import { GrpcEndpointForm } from '../../../shared/components/grpc-endpoint-form'; | ||
|
|
||
| export const SetGrpcEndpoint = () => { |
There was a problem hiding this comment.
This file used to be set-rpc-endpoint.tsx. When I renamed it to set-grpc-endpoint.tsx, and extracted the GrpcEndpointForm component, the file was so different that Git thought of it as an entirely different file.
| } | ||
| }; | ||
|
|
||
| export const useGrpcEndpointForm = () => { |
There was a problem hiding this comment.
This hook basically extracts all the business logic out of the SettingsRPC component
There was a problem hiding this comment.
a lot of this logic could alternatively go into zustand, it seems like that's generally preferred
There was a problem hiding this comment.
Yeah, you're right — I'll do a follow-up PR for that
jessepinho
changed the title
jessepinho
requested review from
turbocrime,
grod220,
Valentine1898 and
TalDerei
apps/extension/src/shared/components/grpc-endpoint-form/index.tsx
Outdated
Show resolved
Hide resolved
| ) => { | ||
| setIsSubmitButtonEnabled(false); | ||
| await setGrpcEndpoint(grpcEndpointInput); | ||
| // If the chain id has changed, our cache is invalid |
There was a problem hiding this comment.
on mainnet, this is a more serious condition than is typical in our experience as developers constantly flipping between testnet/preview. there should be a confirmation or other significant ui hurdle at this point.
There was a problem hiding this comment.
OK, I added a confirmation dialog in this commit.
Was the rest of this PR a ✅ ? If so, could you please approve if that latest commit looks good?
| rel='noreferrer' | ||
| className='block text-right text-xs text-muted-foreground' | ||
| > | ||
| Add to this list |
There was a problem hiding this comment.
clicking on this link in the RPC settings page of the extension takes you to this outdated link: https://github.com/penumbra-zone/web/blob/main/packages/constants/src/grpc-endpoints.ts
There was a problem hiding this comment.
@TalDerei that's actually the new link!
This PR takes the UI from #868 and uses it in the RPC settings page of the extension. It also does a bit of refactoring to better share the functionality between the RPC settings page and the RPC onboarding page. And lastly, it shows the selected chain ID (or error message) at the bottom of the form.
Screen.Recording.2024-04-03.at.5.41.13.PM.mov
Screen.Recording.2024-04-03.at.5.45.16.PM.mov
In this PR
<GrpcEndpointForm />component that's shared between the onboarding screen and the settings screen of the extension.<PaddingWrapper />component to ensure consistent padding in the various component popup screens.Closes #605