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
Show proposal vote block heights #2701
Show proposal vote block heights #2701
Conversation
Ready for review @lukebp |
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.
Looks good but it won't show on mobile since we aren't showing blocks left too.
@tiagoalvesdulce How do you think? Should we display blocks left on mobile? |
src/components/Proposal/Proposal.jsx
Outdated
<> | ||
<Tooltip | ||
placement="bottom" | ||
content={`Block heights: ${startblockheight} - ${endblockheight}`} |
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.
Let's update this to just:
content={`Block heights: ${startblockheight} - ${endblockheight}`} | |
content={`Block ${startblockheight} to ${endblockheight}`} |
src/components/Proposal/Proposal.jsx
Outdated
styles.blocksLeft | ||
)} | ||
size="small"> | ||
{`duration: ${votingBlocks} block${ |
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.
Let's replace this with the text Block ${startblockheight} to ${endblockheight}
and remove the tooltip here since it's redundant.
No, I think it's fine as it. It's ok if mobile doesn't have the tooltip info. |
Yeah, I think it's fine as well. |
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 more UX tweeks.
-
Let's not capitalize
Block
inBlock xxxx to xxxx
.block xxxx to xxxx
is more consistent with the other text. -
After seeing this, I think it's too cluttered.
For proposals that have finished voting, can we display the block xxxx to xxxx
as a second line in the exiting tooltip that shows the date. i.e. the tooltip would display:
Mon, 10 Jan 2022 18:38:09 GMT
block xxxx to xxxx
Lets see how that looks. I think it will be cleaner.
…ith finished proposal
src/components/Proposal/Proposal.jsx
Outdated
{/*{isVotingFinished && ( | ||
<Text | ||
className={classNames( | ||
"hide-on-mobile", | ||
styles.blocksLeft | ||
)} | ||
size="small"> | ||
{`Block ${startblockheight} to ${endblockheight}`} | ||
{`block ${startblockheight} to ${endblockheight}`} | ||
</Text> | ||
)} | ||
)}*/} |
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.
comment
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.
Sorry. My bad
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 do like it better this way. What do you think @tiagoalvesdulce?
I agree. |
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.
Code looks good and works as expected.
Closes #2699
This diff add the block start and end heights on the gui for proposal
votes.