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

Fix Governance Dashboard display of Over-Allocated proposals #346

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

JSKitty
Copy link
Member

@JSKitty JSKitty commented Apr 16, 2024

Abstract

This PR simply fixes (alongside making a little more obvious) when proposals are Over-Allocated, thus ineligible to receive their proposal payment for the given cycle.

MPW properly displays "Not Funded" on such proposals now, as well as displaying the "Passing" text in Orange to highlight the difference better.

To save a few CPU cycles and excess code, I removed the repetitive parseInt(MonthlyPayment) for a single conversion and re-used it in the same places.

These calculations are a 1:1 match with the PIVX.org website.

Before PR
image

After PR
image


Testing

To test this PR, it's suggested to attempt these user flows, or variations of these:

  • Open the "Governance" tab, ensure the "Not Funded" states (and general governance state) matches PIVX.org

If any errors are found, the PR works unexpectedly, or you have viable suggestions to improve the UX or functionality of the PR, let me know!


@JSKitty JSKitty added the Bug This is either a bugfix (PR) or a bug (issue). label Apr 16, 2024
@JSKitty JSKitty self-assigned this Apr 16, 2024
Copy link

netlify bot commented Apr 16, 2024

Deploy Preview for cheery-moxie-4f1121 ready!

Name Link
🔨 Latest commit fa644c6
🔍 Latest deploy log https://app.netlify.com/sites/cheery-moxie-4f1121/deploys/661fde51e860480008da3150
😎 Deploy Preview https://deploy-preview-346--cheery-moxie-4f1121.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@JSKitty JSKitty requested a review from panleone April 16, 2024 22:02
panleone
panleone previously approved these changes Apr 16, 2024
Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK on the changes

maybe saying that a proposal is both "passing" and "not funded" is a bit strange?

@JSKitty
Copy link
Member Author

JSKitty commented Apr 17, 2024

Perhaps we could add a third translation variable for "Over Budget" / "Over Allocated"?

Trying to show the user that the proposal is valid and successfully passing, but too large to receive from the budget at current vote priority.

@Duddino
Copy link
Member

Duddino commented Apr 17, 2024

a proposal is both "passing" and "not funded" is a bit strange?

To me it makes sense, and that's what pivx.org uses as well
image

Duddino
Duddino previously approved these changes Apr 17, 2024
Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

tACK

@JSKitty JSKitty dismissed stale reviews from Duddino and panleone via 9700ef4 April 17, 2024 14:33
@JSKitty
Copy link
Member Author

JSKitty commented Apr 17, 2024

I've done two changes:

  • Aligned with the PIVX Core wallet by displaying "Over Budget" rather than "Not Funded".
  • De-capitalised all translations regarding proposal states, it makes re-using them elsewhere hard, I'm using CSS text-transform: uppercase; as a replacement.

New Display:
image

@JSKitty
Copy link
Member Author

JSKitty commented Apr 17, 2024

BTW, this built fine on Netlify, GitHub is just being weird.
https://deploy-preview-346--cheery-moxie-4f1121.netlify.app/

Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

tACK

Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

re-tACK fa644c6

@panleone panleone merged commit 76a6dfa into master Apr 17, 2024
6 of 7 checks passed
@JSKitty JSKitty deleted the fix-governance-overallocation-display branch April 22, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is either a bugfix (PR) or a bug (issue).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants