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

[QT] Add password mismatch warnings #1607

Merged
merged 2 commits into from
May 9, 2020
Merged

[QT] Add password mismatch warnings #1607

merged 2 commits into from
May 9, 2020

Conversation

JSKitty
Copy link
Member

@JSKitty JSKitty commented May 7, 2020

This closes #1605 by adding a warning message & deactivating the "OK" button when the two passphrase fields do not match eachother.

image

As this warning is in the same area as the caps lock warning, the two have been merged into a single "passWarningLabel" which "stacks" warnings ontop of eachother, so both warnings can be shown independently, or both at once.

image

This PR ensures a user cannot proceed via the "OK" button until:

  1. The new passphrase fields are not empty.
  2. The new passphrase fields share identical texts.
  3. (If changing password only) The old passphrase field is not empty.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Sounds good, concept ACK. Left few comments.

src/qt/askpassphrasedialog.cpp Outdated Show resolved Hide resolved
src/qt/askpassphrasedialog.cpp Outdated Show resolved Hide resolved
@JSKitty
Copy link
Member Author

JSKitty commented May 9, 2020

Added suggestions and updated the PR description with new images of the compiled PR.

@JSKitty JSKitty requested a review from furszy May 9, 2020 14:27
@random-zebra
Copy link

Good job 👍 works as intended and code looks good. Only minor non-blocking nit would be rename getWarnings(), since it doesn't return the warning string, but rather update the label (maybe updateWarningsLabel() ?).

@JSKitty
Copy link
Member Author

JSKitty commented May 9, 2020

Renamed getWarnings() to updateWarningsLabel()

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 8797114

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Working good 👌 . For the next time, would be good to squash the commits, the last commit change is purely over the new introduced code.

ACK 8797114 and merging..

@furszy furszy merged commit 749d42a into PIVX-Project:master May 9, 2020
@JSKitty JSKitty deleted the passphrase-ux-improvements branch May 9, 2020 21:12
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.

[Feature request] Prevent going next step if initial wallet encryption passphrases do not match
3 participants