Skip to content

Conversation

DeeJayElly
Copy link
Contributor

@DeeJayElly DeeJayElly commented Mar 15, 2021

This PR enables pasting of wallet recovery phase.

This PR will utilized changes from React Polymorph Autocomplete component from here:
input-output-hk/react-polymorph#163

Todos

  • Improve logic to parse pasted strings (words)
  • Handle wrong words from the passphrase and break parsing of strings
  • Fix js console error
  • Prevent dropdown opening on deletion of the word
  • Disable button on dialog opening with empty recovery phase
  • Fix opening/closing of dropdown on words copy/paste

Screenshots

Screen Shot 2021-03-18 at 7 48 55 AM


Testing Checklist

Test Cases

Scenario 1: Copy and paste wallet passphrase
Given I am restoring a wallet
And I am at Recovery Phrase Step in (Restore wallet|Verify wallet phrase)
And I have the correct wallet passphrase
And I have copied the entire wallet passphrase
When I paste the wallet passphrase into the text area
And I select ‘Check Recovery Phrase’
Then it is successful

Test Report

For build 17700

Fixed issues

Issues 1 to 4 were reported here.

1 - When deleting an already entered words by using "Backspace" key we should always hide the options dropdown in case it is shown (this is reproducible in develop too)

2 - When you enter the 2nd step of the "Wallet restoration" dialog, the button "Check recovery phrase" should be disabled by default (this is reproducible in develop too):

3 - When deleting already entered words by clicking on the "x" button we should prevent the options dropdown to show (this is reproducible in develop too)

4 - When pasting recovery phrase on the "Verification of the recovery phrase" dialog, there is a JS error thrown (this does not happen in the "Wallet restoration" dialog)

5 - I was able to paste more than maximum words. See screenshot

6 - After pasting words, dropdown didn't close automatically
See video

7 - When there're extra white spaces, it seemed the remaining words were not able to be pasted
See video

8 - Sometimes it is needed to click twice in the field to successfully paste values


Review Checklist

Basics

  • PR has been assigned and has appropriate labels (feature/bug/chore, release-x.x.x)
  • PR is updated to the most recent version of the target branch (and there are no conflicts)
  • PR has a good description that summarizes all changes
  • PR has default-sized Daedalus window screenshots or animated GIFs of important UI changes:
    • In English
    • In Japanese
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • Automated tests: All acceptance and unit tests are passing (yarn test)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in development build (yarn dev)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in production build (yarn package / CI builds)
  • There are no flow errors or warnings (yarn flow:test)
  • There are no lint errors or warnings (yarn lint)
  • There are no prettier errors or warnings (yarn prettier:check)
  • There are no missing translations (running yarn manage:translations produces no changes)
  • Text changes are proofread and approved (Jane Wild / Amy Reeve)
  • Japanese text changes are proofread and approved (Junko Oda)
  • UI changes look good in all themes (Alexander Rukin)
  • Storybook works and no stories are broken (yarn storybook)
  • In case of dependency changes yarn.lock file is updated

Code Quality

  • Important parts of the code are properly commented and documented
  • Code is properly typed with flow
  • React components are split-up enough to avoid unnecessary re-renderings
  • Any code that only works in main process is neatly separated from components

Testing

  • New feature/change is covered by acceptance tests
  • New feature/change is manually tested and approved by QA team
  • All existing acceptance tests are still up-to-date
  • New feature/change is covered by Daedalus Testing scenario
  • All existing Daedalus Testing scenarios are still up-to-date

After Review

  • Merge the PR
  • Delete the source branch
  • Move the ticket to done column on the YouTrack board
  • Update Slack QA thread by marking it with a green checkmark

@DeeJayElly DeeJayElly removed the WIP label Mar 29, 2021
@DeeJayElly
Copy link
Contributor Author

DeeJayElly commented Apr 20, 2021

@aleksandardjordjeviciohk great work! Your changes work great - still, I need you to focus on the following list of remaining issues (not all of them are caused by your changes - most of them are present in develop but I would like to have them fixed in this PR):

  1. When deleting an already entered words by using "Backspace" key we should always hide the options dropdown in case it is shown (this is reproducible in develop too):

1.mp4

  1. When you enter the 2nd step of the "Wallet restoration" dialog, the button "Check recovery phrase" should be disabled by default (this is reproducible in develop too):

2

  1. When deleting already entered words by clicking on the "x" button we should prevent the options dropdown to show (this is reproducible in develop too)

3.mp4

  1. When pasting recovery phrase on the "Verification of the recovery phrase" dialog, there is a JS error thrown (this does not happen in the "Wallet restoration" dialog):

4.mp4
4

Thanks for fixing those 🙏

cc @gabriela-ponce

All of the listed issues are fixed. They are part of this PR: input-output-hk/react-polymorph#168

@gabriela-ponce
Copy link

I tested this on build 17687 and on Netlify from this PR.
Regarding this list of issues:

1 - When deleting an already entered words by using "Backspace" key we should always hide the options dropdown in case it is shown (this is reproducible in develop too):

1 - Fixed on Netlify

2 - When you enter the 2nd step of the "Wallet restoration" dialog, the button "Check recovery phrase" should be disabled by default (this is reproducible in develop too):

2 - Can't test on Netlify, doesn't reproduce on build 17687

3 - When deleting already entered words by clicking on the "x" button we should prevent the options dropdown to show (this is reproducible in develop too)

3 - Still reproduces on Netlify and build 17687. Check Video.

4 - When pasting recovery phrase on the "Verification of the recovery phrase" dialog, there is a JS error thrown (this does not happen in the "Wallet restoration" dialog):

4 - Can't test on netlify, doesn't reproduce on build 17687

Copy link

@gabriela-ponce gabriela-ponce 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, great job

Copy link
Contributor

@nikolaglumac nikolaglumac left a comment

Choose a reason for hiding this comment

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

Great fixes @aleksandardjordjeviciohk 🎉
There is just one more bug left - it is specific to the "Wallet recovery phrase verification" dialog. When you type in first character in the autocomplete a JS error is thrown.
You can get it each time you delete and enter first character in the search as shown in the following video:

bug.mp4

@DeeJayElly
Copy link
Contributor Author

Great fixes @aleksandardjordjeviciohk 🎉
There is just one more bug left - it is specific to the "Wallet recovery phrase verification" dialog. When you type in first character in the autocomplete a JS error is thrown.
You can get it each time you delete and enter first character in the search as shown in the following video:

bug.mp4

There was an event error onChange() method, its fixed now with these updates -> input-output-hk/react-polymorph@26e5704

@DeeJayElly DeeJayElly requested a review from nikolaglumac April 22, 2021 09:25
@DeeJayElly DeeJayElly removed the WIP label Apr 22, 2021
Copy link
Contributor

@nikolaglumac nikolaglumac left a comment

Choose a reason for hiding this comment

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

Works great now! 💯

@nikolaglumac nikolaglumac merged commit d536fe9 into develop Apr 22, 2021
@iohk-bors iohk-bors bot deleted the feature/ddw-603-enable-pasting-of-wallet-recovery-phrase branch April 22, 2021 14:55
@nikolaglumac nikolaglumac added release-4.1.0 Daedalus Mainnet and removed ✈️ release-4.1.0-FC1 labels Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature release-4.1.0 Daedalus Mainnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants