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: busy states during governance transactions #5742

Merged

Conversation

nicole-obrien
Copy link
Contributor

Co-authored-by: Tuditi Tuditi@users.noreply.github.com

Summary

Please summarize your changes, describing what they are and why they were made.

...

Changelog

- Please write a list of changes

Relevant Issues

Please list any related issues (e.g. bug, task).
Fixes #5726

Testing

Platforms

Please select any platforms where your changes have been tested.

  • Desktop
    • MacOS
    • Linux
    • Windows
  • Mobile
    • iOS
    • Android

Instructions

Please describe the specific instructions, configurations, and/or test cases necessary to test and verify that your changes work as intended.

...

Checklist

Please tick all of the following boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or modified tests that prove my changes work as intended
  • I have verified that new and existing unit tests pass locally with my changes
  • I have verified that my latest changes pass CI workflows for testing and linting
  • I have made corresponding changes to the documentation

Co-authored-by: Tuditi <Tuditi@users.noreply.github.com>
Copy link
Contributor

@maxwellmattryan maxwellmattryan left a comment

Choose a reason for hiding this comment

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

Looks good! Added a few questions and requested changes 🙏

@maxwellmattryan
Copy link
Contributor

Some feedback from testing:

  • (Slightly unrelated but) I got a no unspent voting output found when trying to revote for proposals (after reducing voting power from 1000 to 500 SMR):
    No unspent voting output found
    
    Until I logged out and back in, my voting power went to 0 SMR even though it was reflected in my available balance (500 SMR). When I logged back in it displayed the correct voting power of 500 SMR.
    Since I didn't revote, my expectation was that voting on a proposal would only vote for that single one, but it seems that all my previous votes were remembered. This is convenient for the user since they aren't required to manually re-vote for everything, but it's not the expectation.
  • UX-wise, it would feel better if we also disabled the "Cancel" button in the ProposalDetailsView since we do it for the popups.

@nicole-obrien
Copy link
Contributor Author

  • UX-wise, it would feel better if we also disabled the "Cancel" button in the ProposalDetailsView since we do it for the popups.

There is a task to remove this redundant button and replace it with the unvote button so this PR is fine for now

Copy link
Contributor

@maxwellmattryan maxwellmattryan left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@maxwellmattryan maxwellmattryan merged commit f2986a1 into develop Feb 1, 2023
@maxwellmattryan maxwellmattryan deleted the fix/busy-state-during-governance-transactions branch February 1, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Disable voting button until governance transaction is included on the Tangle
2 participants