-
Notifications
You must be signed in to change notification settings - Fork 39
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
[HUM-159]: refactor component sign up component refactor #3157
base: develop
Are you sure you want to change the base?
[HUM-159]: refactor component sign up component refactor #3157
Conversation
@mpblocky is attempting to deploy a commit to the HUMAN Protocol Team on Vercel. A member of the Team first needs to authorize it. |
@dnechay ready! |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
LGTM overall
Some minor style comments
@@ -342,7 +342,7 @@ | |||
}, | |||
"connectWallet": { | |||
"title": "Setup Operator", | |||
"description": "Please use the button bellow to connect your wallet and sign up as an operator.", | |||
"description": "Please use the button below to connect your wallet and sign up as an operator.", |
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.
Nice catch!
keysData: GetEthKVStoreValuesSuccessResponse; | ||
}>) { | ||
const hasSomeNotEmptyKeys = | ||
Object.values(keysData).filter(Boolean).length > 0; |
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.
Object.values(keysData).filter(Boolean).length > 0; | |
Object.values(keysData).some(Boolean); |
Object.values(keysData).filter((value) => { | ||
/** | ||
* This check is necessary because TS can't infer | ||
* "undefined" from optional object's property | ||
*/ | ||
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
if (value === undefined) { | ||
return false; | ||
} | ||
|
||
return value.length === 0; | ||
}).length > 0; |
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.
Object.values(keysData).filter((value) => { | |
/** | |
* This check is necessary because TS can't infer | |
* "undefined" from optional object's property | |
*/ | |
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | |
if (value === undefined) { | |
return false; | |
} | |
return value.length === 0; | |
}).length > 0; | |
Object.values(keysData).some((value) => { | |
/** | |
* This check is necessary because TS can't infer | |
* "undefined" from optional object's property | |
*/ | |
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | |
if (value === undefined) { | |
return false; | |
} | |
return value.length === 0; | |
}); |
useHMTokenDecimals, | ||
} from '../hooks'; | ||
import { StakeForm, Buttons } from '../components/add-stake'; | ||
import { stakedAmountFormatter } from '../utils'; |
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.
A bit below in getAlert
, let's maybe do switch based on addStakeMutationState?.status
instead of true
?
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.
Also let's do something similar in ConnectWalletOperatorPage
component
|
||
type AmountField = z.infer<AmountValidation>; | ||
|
||
export const addStakeAmountCallArgumentsSchema = ( |
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.
schema
is not a util
, it's actually a schema
, so let's either move them to some signup/operator/schemas
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.
Yes, I was a bit unsure about how we want to store that, will do!
Issue tracking
HUM-159
Context behind the change
homepage/signup/operator
How has this been tested?
Release plan
normal
Potential risks; What to monitor; Rollback plan
n/a