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.

Sign up for GitHub

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

Jump to bottom

Improve Proposal Creation #199

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Improve Proposal Creation #199

merged 2 commits into from
Sep 19, 2023

Conversation

JSKitty
Copy link
Member

@JSKitty JSKitty commented Sep 18, 2023 •
edited
Loading

Abstract

This PR adds various small improvements to the Proposal Creation dialog:

  • Add ability for Advanced Mode users to use a custom Proposal Address (external, etc) for payouts to arrive to.
  • Add HTML sanity checks using min/max/maxlength properties.
  • Use .trim() on all inputs.
  • Fix a case where unencrypted wallet users could create proposals (since finalisation data is stored in accounts).

image

Testing

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

  • Switch to Testnet (and load a wallet with tPIV).
  • Try to create a proposal without an encrypted wallet. (It should disallow you).
  • Create a proposal without Advanced Mode.
  • Create a proposal with Advanced Mode, and set a custom address.
    • Try a valid address.
    • Try a malformed/invalid address.

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 Bug This is either a bugfix (PR) or a bug (issue). Enhancement New feature or request labels Sep 18, 2023
@JSKitty JSKitty self-assigned this Sep 18, 2023
@JSKitty JSKitty changed the title Add custom addresses, checks, trims, input min/max Improve Proposal Creation Sep 18, 2023
panleone
panleone previously approved these changes Sep 18, 2023
View reviewed changes
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 aed3ac8

Duddino
Duddino previously approved these changes Sep 18, 2023
View reviewed changes
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 4516123 September 19, 2023 11:02
panleone
panleone approved these changes Sep 19, 2023
View reviewed changes
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 4516123, tested again and the git diff is good

Duddino
Duddino approved these changes Sep 19, 2023
View reviewed changes
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 merged commit dff304d into master Sep 19, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Duddino Duddino Duddino approved these changes

@panleone panleone panleone approved these changes

@Liquid369 Liquid369 Awaiting requested review from Liquid369

Assignees

@JSKitty JSKitty

Labels
Bug This is either a bugfix (PR) or a bug (issue). Enhancement New feature or request
Projects
My PIVX Wallet v2.0
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JSKitty @panleone @Duddino