-
Notifications
You must be signed in to change notification settings - Fork 228
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: add Earn amount inputs #1870
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
<div className={cn('flex flex-col', className)}> | ||
<TextInput | ||
className={cn( | ||
'w-full border-[none] bg-transparent font-display text-[2.5rem]', |
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.
any way to avoid the magic 2.5rem?
18df2f1
to
c73c8d4
Compare
<div className={cn('flex flex-col', className)}> | ||
<TextInput | ||
className={cn( | ||
'w-full border-[none] bg-transparent font-display text-5xl', |
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.
'w-full border-[none] bg-transparent font-display text-5xl', | |
'w-full border-none bg-transparent font-display text-5xl', |
Shouldn't need an arbitrary value
disabled, | ||
value, | ||
onChange, | ||
'aria-label': ariaLabel, |
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.
The only thing that's a little weird about this is exposing a random aria-label
to the developer but no other input attributes. Is there a way that we could handle it ourselves?
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.
well EarnAmountInput
shouldn't be used by developer -> they would use the wrapper DepositAmountInput
or WithdrawAmountInput
that doesn't expose those
b86a4c5
to
914ea0e
Compare
What changed? Why?
EarnAmountInput
and wrappers forEarn
componentNotes to reviewers
How has it been tested?