-
Notifications
You must be signed in to change notification settings - Fork 555
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
Pre-populating the Deposit Max Amount #1875
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@kirksgithub is attempting to deploy a commit to the blockworks-foundation Team on Vercel. A member of the Team first needs to authorize it. |
@asktree this update fixes UI/UX issues with depositing tokens to a governance, although this button should have been pushed as an advanced menu option/deposit option in the "My Governance Power" and not exposed for regular users who would like to quickly deposit and participate in Governance:
This should improve the experience significantly so users are not guessing the amount they have as a balance while in this prompt |
@@ -76,6 +88,7 @@ export const DepositTokensButton = ({ | |||
await deposit(nativeAmount) | |||
setOpenModal(false) | |||
}} | |||
disabled={humanReadableMax !== undefined && (parseInt(amount) > humanReadableMax || parseInt(amount) > 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.
this seems wrong? if amount > 0
the button is disabled?
The intended UX is that the max is a placeholder, and hitting confirm without any value entered would use it. It's just an oversight in the code that this isn't the case. However, it seems like this intended UX wasn't very obvious. I'm going to fix this bug in another PR quickly. I think the ideal UI would be a 1% to 100% slider, with a text entry box. I hear you re: advanced option in another view. I think it makes sense, mind opening a thread in the feedback channel on the discord for further discussion? |
Default should be set to Max, this PR resolves the issue immediately until you push your changes |
@@ -30,12 +30,11 @@ export const DepositTokensButton = ({ | |||
const depositTooltipContent = !connected | |||
? 'Connect your wallet to deposit' | |||
: !hasTokensInWallet | |||
? "You don't have any governance tokens in your wallet to deposit." | |||
: undefined | |||
? "You don't have any governance tokens in your wallet to deposit." |
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.
it seems like you're using a different code formatter, any idea what's going on? FWIW I prefer this way of formatting ternaries
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.
using codespace
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.
as in, using the codespace associated with the repo? interesting. how did you find that btw? I'm new to codespaces and just set it up for personal use.
The Deposit button automatically allowed the Max amount to be deposited, nevertheless this changed causing issues for our DAO members, in specific we identified the following:
To improve this slow and confusing experience, the popup should be easier to navigate as specified above.
We also recommend that in the future this button is pushed as an advanced button either in the "My Governance View" or as a dropdown button as an additional option
The fixes here will be to improve the UI/UX and allow a default Max to be set along with any helper buttons to ease setting the Max deposit amount