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

[FIX] Reveal SRP or Private Key wrong password error #4799

Merged
merged 16 commits into from
Aug 17, 2022
Merged

[FIX] Reveal SRP or Private Key wrong password error #4799

merged 16 commits into from
Aug 17, 2022

Conversation

gantunesr
Copy link
Member

@gantunesr gantunesr commented Aug 4, 2022 •
edited

Description

Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions,

1. What is the reason for the change?
While trying to reveal the SRP or Private Key, if the user enters the wrong password the app would still display the "Hold To Reveal" component.

2. What is the improvement/solution?
This PR validates the password before allowing the user to proceed.

Screenshots/Recordings

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Screen.Recording.2022-08-04.at.11.49.13.mov

Issue

Progresses #4781

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@gantunesr gantunesr added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Aug 4, 2022
@gantunesr gantunesr marked this pull request as ready for review August 4, 2022 03:09
@gantunesr gantunesr requested a review from a team as a code owner August 4, 2022 03:09
gantunesr
gantunesr commented Aug 4, 2022
View reviewed changes
package.json Outdated Show resolved Hide resolved
Cal-L
Cal-L reviewed Aug 4, 2022
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment •
edited

Choose a reason for hiding this comment

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

Was there another issue where you can still hit return on the keyboard, which will prompt the hold to reveal modal even with an incorrect password?
Reference to where this is mentioned - #4781

@gantunesr
Copy link
Member Author

In reference to #4799 (review)

You're right @Cal-L, I just added an improvement in 440cfa7

Cal-L
Cal-L approved these changes Aug 5, 2022
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@gantunesr gantunesr added needs-qa Any New Features that needs a full manual QA prior to being added to a release. Mobile QA board and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Aug 9, 2022
@gantunesr gantunesr added the release-5.6.0 PRs for v5.6.0 release label Aug 9, 2022
@cortisiko cortisiko added release-5.7.0 PRs for release 5.7.0 QA in Progress QA has started on the feature. and removed release-5.6.0 PRs for v5.6.0 release needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Aug 11, 2022
@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Aug 12, 2022
cortisiko
cortisiko approved these changes Aug 12, 2022
View reviewed changes
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

This is 🌮 🌮 🌮

To test follow the accepted behavior listed here

@cortisiko cortisiko merged commit 669782e into main Aug 17, 2022
@cortisiko cortisiko deleted the fix/reveal-credential-bug branch August 17, 2022 22:47
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Cal-L Cal-L Cal-L approved these changes

@cortisiko cortisiko cortisiko approved these changes

Assignees
No one assigned
Labels
QA Passed A successful QA run through has been done release-5.7.0 PRs for release 5.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gantunesr @Cal-L @cortisiko