-
Notifications
You must be signed in to change notification settings - Fork 3
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
2 tx flow, first remove votes and undelegate then delegate #186
Conversation
Deploying delegit with Cloudflare Pages
|
title: string | ||
message: string | ||
title?: string | ||
message: string | React.ReactNode |
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 is kinda scary.. We should probably split in 2 different optional variables
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.
Nah! this is pretty common and doesn't change anything in the code below. Having 2 variables makes it uglier, and doesn't change anything! For instance that's what we have in the ContentReveal
and what many common libraries have too!
@@ -147,9 +147,9 @@ export const MultiTransactionDialog = ({ | |||
<DialogDescription> | |||
<div className="my-4"> | |||
{step === 1 && | |||
'The delegation process is in 2 parts. First, please sign a transaction to remove current votes and delegations.'} | |||
'The delegation process is in 2 parts. First, please sign a transaction to remove previous votes or delegations.'} |
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 delegation process is in 2 parts. First, please sign a transaction to remove previous votes or delegations.'} | |
'Please sign a transaction to clear any previous votes or delegations before proceeding with the delegation process.'} |
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 find this long phrase that we had actually pretty bad, nobody reads such long things, it needs to be more actionable IMHO. What do you think about:
There will be 2 transactions:
1. Clear previous votes & delegations
2. Delegate
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.
edit, scratch that, I'll make a new proposal. We really don't need a long phrase.
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 went for something straight forward. No need to "Please" and what not, they are asking for it, not delegit!
- First, let's clear previous votes or delegations.
- Now, we are ready to delegate.
WDYT?
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.
Sounds better.
ping @wirednkod for another review please 🙏 |
closes #185
closes #167
Submission checklist:
Layout