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

feat: wallet password wizard #146

Merged

Conversation

tomaszantas
Copy link

Description

This PR:

  • Adds react-hook-form package
  • Extends the Input component by adding forwardRef, so it plays nice with react-hook-form
  • Callout component - right now it supports only warning variant
  • Adds WalletPasswordWizard container and WalletPasswordForm component. The WalletPasswordForm is pure HTML with react-hook-form. The WalletPasswordWizard renders WalletPasswordForm and connects with Redux.
  • Adds WalletPasswordWizard to the Tari Mining to handle situation when the wallet is not configured yet.

Motivation and Context

#12

Missing:

How Has This Been Tested?

Callout component

image

Wallet Password Wizard

Screen.Recording.2022-05-09.at.21.29.55.mov

@tomaszantas tomaszantas requested review from corquaid and tarnas14 May 9, 2022 20:07
@@ -0,0 +1,4 @@
export interface WalletPasswordWizardProps {
submitBtnText?: string
onSuccess?: () => void

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand why the above is optional, you could have a default 'submit' text, but onSuccess looks like a required prop to me

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary in the Wizard, because on form submission, the container dispatches the action and changes the Redux state. So the parent component not necessary need to know whether form submit was successful, it can just observe the Redux state with some selector.

@@ -10,6 +10,7 @@ export const unlockWallet = createAsyncThunk<
},
WalletPassword
>('wallet/unlock', async password => {
console.log('thunk')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@@ -10,6 +10,7 @@ export const unlockWallet = createAsyncThunk<
},
WalletPassword
>('wallet/unlock', async password => {
console.log('thunk')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a note for this console.log()

Copy link

@corquaid corquaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tomaszantas tomaszantas merged commit 9c9d264 into launchpad_such_wow May 10, 2022
@tomaszantas tomaszantas deleted the launchpad/feature/wallet-password-wizard branch May 10, 2022 11:16
CjS77 pushed a commit that referenced this pull request May 12, 2022
* Add Icons and statuses to MiningBox, initialize WalletPasswordWizard

* Password Wizard
CjS77 pushed a commit that referenced this pull request May 13, 2022
* Add Icons and statuses to MiningBox, initialize WalletPasswordWizard

* Password Wizard
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.

3 participants