-
Notifications
You must be signed in to change notification settings - Fork 74
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: cctp withdrawals: withdraw form [OTE-855][5/n] #1155
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
6cb13aa
to
c3ad6a0
Compare
f91234c
to
e3e08cf
Compare
c3ad6a0
to
5504986
Compare
e3e08cf
to
01d41ae
Compare
5504986
to
ed6d288
Compare
4ee597b
to
76a7484
Compare
9792997
to
3e09292
Compare
9486824
to
762e73e
Compare
ed6d288
to
8de27cd
Compare
762e73e
to
4f8cb71
Compare
testFlags.onboardingRewrite ? ( | ||
<span tw="row gap-[0.5ch]"> | ||
<Icon iconName={IconName.Usdc} size="1.5em" /> | ||
{stringGetter({ key: STRING_KEYS.WITHDRAW })} USDC |
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.
usdc needs to be a parameter, can't guarantee all languages put object after verb
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.
oh good point, this should have the localize TODO on it. i just haven't been localizing any of these strings yet.
return ( | ||
<$Form onSubmit={onSubmit}> | ||
<div tw="text-color-text-0"> | ||
Make sure everything looks correct with your withdrawal before confirming |
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.
localize
</span> | ||
} | ||
validationConfig={ | ||
toAddress && Boolean(exchange) && !isValidDestinationAddress |
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.
I prefer !!exchange but this works too
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.
hmmm okay i'm down. tbh i've always preferred Boolean
but i think !!
is more common at this company so it makes sense to match that pattern
/> | ||
} | ||
/> | ||
<ArrowContainer tw="z-1 mx-auto -mb-1.25 -mt-1.25 flex h-1.5 w-1.5 items-center justify-center rounded-0.25 border-color-layer-7 bg-color-layer-4"> |
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.
this one is probably at the point it's better as a styled component. Why the "-" in front of mb and mt?
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.
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.
still somewhat prefer this being a styled component but it's not a big deal
src/views/forms/AccountManagementFormsNew/WithdrawForm/WithdrawForm.tsx
Outdated
Show resolved
Hide resolved
393f229
to
1411437
Compare
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.
i didn't re-review, assuming the pr that just came in was reviewed
Adds the withdrawform component and hooks it up to the subcomponents in the stack
Adds ArrowIcon and USDCIcon
Adds backgroundColorOverride to formInput and Input components to enable dark themed
TODOS:
Figma https://www.figma.com/design/fMQodZKzn9B5aZTN5G0fLJ/dYdX-%E2%80%BA-Web?node-id=41402-168816&node-type=frame&t=HtXCnIXhOiY4jfOY-0
What it actually looks like
https://www.loom.com/share/c7904c9fd90a4bd6a921257cd81f5461?sid=66851029-b82e-4e93-a34e-f33822f1bdb5