-
Notifications
You must be signed in to change notification settings - Fork 2
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
UI: Make Deposit step #21
Conversation
Added setTimeout utility function
Applied minor styling corrections
Fixed QR code size and spacing, address text color, uncommented code line
This commit is cherry picked from the `ui/initiate-minting-step` branch for consistency between changes. Moved ModalHeader rendering to higher order component with additional label parameter. Restyled Spinner component. Added onError callback when deposit transaction is failed with storing error data (entry point for Toast component rendering).
`Toast` component will be introduced with `ui/initiate-minting-step` branch. It's reverted from this branch. This reverts commit b14b805.
This reverts commit 4b10dd5.
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.
Left some comments to look at before the merge
</BodyMd> | ||
</VStack> | ||
<Flex as={Badge} align="center"> | ||
{/* TODO: Adjust styles when Badge changes are merged. */} |
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 you tell me in wchich PR the Badge changes are addressed?
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.
Actually it's LabeledBadge
introduced in this PR.
I will complete this TODO in separate PR.
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.
Ok, I will leave this conversation open then
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.
Addressed review comments
</BodyMd> | ||
</VStack> | ||
<Flex as={Badge} align="center"> | ||
{/* TODO: Adjust styles when Badge changes are merged. */} |
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.
Actually it's LabeledBadge
introduced in this PR.
I will complete this TODO in separate PR.
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.
Left two small comments.
Overall looking good 🔥
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.
Two additional comments:
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 🔥
Ref: #4
Depends on: #12This pull request introduces redesigned Make Deposit minting flow step.
Additional changes:
GenerateNewDepositAddress
modal redesign:Note: Changes currently doesn't fulfill designs requirements completely. It requires additional finetunes after #20 is merged - can be done with separate PR