Skip to content
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

improve preboostable logic, feedback and layout #1482

Merged
merged 7 commits into from
Mar 13, 2020
Merged

Conversation

dkent600
Copy link
Contributor

@dkent600 dkent600 commented Mar 6, 2020

Resolves: #739
...hopefully.

Refer to the logic described in the code comments added in this client lib PR: https://github.com/daostack/client/pull/416/files

  • eliminates some unnecessary constructions of BN
  • adds '=' (for ">=") in text referring to downStakeNeededToQueue
  • improves layout of amount-needed feedback in StakeButtons.scss
  • doesn't show amount-needed feedback unless it is possible to boost, in StakeButtons.tsx
  • in PreTransactionModal, when downstaking, don't show amount-to-boost, rather show amount-to-stake-to-unboost

@jellegerbrandy jellegerbrandy temporarily deployed to alchemy-fixpreboostable-3vn4uc March 6, 2020 17:52 Inactive
@dkent600 dkent600 changed the title fixPreBoostableLogic [WIP] improve preboostable logic and feedback Mar 6, 2020
@dkent600 dkent600 changed the title [WIP] improve preboostable logic and feedback [WIP] improve preboostable logic, feedback and layout Mar 6, 2020
@dkent600 dkent600 changed the title [WIP] improve preboostable logic, feedback and layout improve preboostable logic, feedback and layout Mar 6, 2020
@dkent600
Copy link
Contributor Author

dkent600 commented Mar 6, 2020

@jellegerbrandy I am premising my findings here based on the logic given in the new comments in #739. Until you've approved those comments, I don't think we should approve this PR. So I'm blocking this one (calling it WIP) until that point.

@dkent600 dkent600 changed the title improve preboostable logic, feedback and layout [WIP] improve preboostable logic, feedback and layout Mar 6, 2020
@dkent600 dkent600 changed the title [WIP] improve preboostable logic, feedback and layout improve preboostable logic, feedback and layout Mar 9, 2020
@dkent600 dkent600 self-assigned this Mar 9, 2020
@dkent600 dkent600 removed the Blocked label Mar 9, 2020
@@ -230,13 +230,13 @@ class ActionButton extends React.Component<IProps, IState> {
/> : ""
}

{ proposalState.stage === IProposalStage.Queued && proposalState.upstakeNeededToPreBoost.lt(new BN(0)) ?
{ proposalState.stage === IProposalStage.Queued && proposalState.upstakeNeededToPreBoost.ltn(0) ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure this works? didn't use to, maybe they fixed it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has always worked for me. testing the functionality seems to work


return (
<div className={wrapperClass}>
{
proposal.stage === IProposalStage.Queued && !expired && proposal.upstakeNeededToPreBoost.gte(new BN(0)) ?
!expired ? ((proposal.stage === IProposalStage.Queued && proposal.upstakeNeededToPreBoost.gten(0)) ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this is right, cant you still boost/unboost a pre-boosted proposal that is expired? or do we want to turn that off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was misinterpreting what "expired" meant (forgetting that it often means expired only within the context of the current phase, but not necessarily that the proposal is dead, and not necessarily that the expired state is even represented yet in the blockchain).

So good catch. I have reverted the change.

<span className={css.boostedAmount}>
<b>
{detailView ? <img src="/assets/images/Icon/Boost-slate.svg" /> : ""}
&gt; {formatTokens(proposal.downStakeNeededToQueue.abs(), "GEN")} Pass to stay boosted
&gt; {formatTokens(proposal.downStakeNeededToQueue.abs(), "GEN")} on Pass to stay boosted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this fit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@jellegerbrandy jellegerbrandy temporarily deployed to alchemy-fixpreboostable-3vn4uc March 9, 2020 22:29 Inactive
@jellegerbrandy jellegerbrandy temporarily deployed to alchemy-fixpreboostable-3vn4uc March 11, 2020 14:24 Inactive
@orenyodfat orenyodfat temporarily deployed to alchemy-fixpreboostable-cshzng March 13, 2020 23:30 Inactive
@dkent600 dkent600 merged commit a6c3774 into dev Mar 13, 2020
@dkent600 dkent600 deleted the fixPreBoostableLogic branch March 15, 2020 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong "pre boost" button on queued proposal.
4 participants