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

[DDW-409] New mnemonic input #2979

Merged
merged 53 commits into from
Aug 10, 2022
Merged

Conversation

przemyslaw-wlodek
Copy link
Contributor

@przemyslaw-wlodek przemyslaw-wlodek commented May 12, 2022

This PR implements a new design of a Mnemonic Input component.

Screenshots

Before:

image

image

After:

image

image

Testing Checklist


Test Scenarios

Scenario 1 - Validate Creation Wizard - Step 3

Given that I am on the step 3 of the wallet creation wizard
And I see the recovery phrase for the new wallet
Then I can see each word is numbered
And they are shown in 3 columns

Scenario 2 - Validate |Recovery phrase dialog|

Given that I am on the |Recovery phrase dialog|
When I validate the screen
Then I can see there is a dedicated numbered input for each word
And these are shown in 3 columns
And the amount of inputs matches the number of words for the type of wallet

Scenario 3 - Paste a valid recovery phrase

Given that I am on the |Recovery phrase dialog|
And I have copied a valid recovery phrase with the correct amount of words
When I position the cursor on any input
And I paste the recovery phrase
Then I can see that the recovery phrase is pasted 
And these are inserted on the corresponding order on each input
And over the recovery phrase I can see a message that informs all the words have been inserted

Scenario 4 - Paste a single word

Given that I am on the |Recovery phrase dialog|
And I have copied a single word from the recovery phrase
When I position the cursor on any input
And I paste the word
Then I can see the word is inserted correctly on the input
And the cursor moves to the next input

Scenario 5 - Paste a wrong recovery phrase

Given that I am on the |Recovery phrase dialog|
And I have copied an invalid recovery phrase with the correct amount of words
When I position the cursor on any input
And I paste the recovery phrase
Then I can see that the recovery phrase is pasted 
And these are inserted on the corresponding order on each input
And over the recovery phrase I can see a message that informs that the recovery phrase is invalid

Scenario 6 - Paste a short or long recovery phrase

Given that I am on the |Recovery phrase dialog|
And I have copied an invalid recovery phrase with the incorrect amount of words
When I position the cursor on any input
And I paste the recovery phrase
Then I can see that the recovery phrase is not pasted

Scenarion 7 - Insert a valid recovery phrase manually

Given that I am on the |Recovery phrase dialog|
And I have a valid recovery phrase with the correct amount of words
When I type each word on the inputs
Then I can see these are inserted correctly 
And each time I insert one valid word, the cursor moves to the next input

Scenario 8 - Insert an invalid recovery phrase manually

Given that I am on the |Recovery phrase dialog|
And I have a valid recovery phrase with the correct amount of words
When I type each word on the inputs
Then I can see these are inserted correctly 
And after all were entered, over the recovery phrase I can see a message that informs that the recovery phrase is invalid

Scenario 10 - Delete a single word from the recovery phrase

Given that I am on the |Recovery phrase dialog|
And I have entered a valid recovery phrase
When I delete a single word
Then I can see the counter over the recovery phrase indicates the actual value
And the submit button is disabled

Scenario 11 - Insert an invalid word

Given that I am on the |Recovery phrase dialog|
And I entered an invalid word 
And I switch to the next input
Then I can see the input with the invalid word is outlined in red

Scenario 12 - Validate dropdown

Given that I am on the |Recovery phrase dialog|
When I click on an input
Then I can see a dropdown with a list of valid words sorted alphabetically is displayed
And if I type a character the list is updated showing matching results

Scenario 13 - Pasting mnemonic words

Given that I am on the |Recovery phrase dialog|
And I have a valid recovery phrase with the correct amount of words
And the words are separated by only a space
And I have copied the recovery phrase into the clipboard
And I have clicked on an input field
When I paste the words
Then all of the pasted words appear according to the order they are written
And no input field is left empty

Scenario 14 - Single input error (underline)

Given that I am on the |Recovery phrase dialog|
And I have provided an incorrect word to one of the inputs
Then a red underline should be displayed under that input

Scenario 15 - Submission with wrong phrase

Given that I am on the |Recovery phrase dialog|
And I have provided an incorrect recovery phrase
And I submitted that phrase
Then error is displayed in top-right corner of the Mnemonic Input component
And I change one of the inputs to a different word
Then submission should be allowed again
|Recovery phrase dialog|
|Wallet Creation - Step 4|
|Wallet Restoration - Step 3|
|Wallet Recovery Phrase Verification - Step 2|

Review Checklist

Basics

  • PR assigned to the PR author(s)
  • input-output-hk/daedalus-dev and input-output-hk/daedalus-qa assigned as PR reviewers
  • If there are UI changes, Alexander Rukin assigned as an additional reviewer
  • All visual regression testing has been reviewed (assign run Chromatic label to PR to trigger the run)
  • PR has appropriate labels (release-vNext, feature/bug/chore, WIP)
  • PR link is added to a Jira ticket, ticket moved to In Review
  • 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 contains screenshots (in case of UI changes)
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • 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)
  • 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 typescript types
  • 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

  • Update Slack QA thread by marking it with a green checkmark

@przemyslaw-wlodek przemyslaw-wlodek self-assigned this May 12, 2022
@przemyslaw-wlodek przemyslaw-wlodek marked this pull request as draft May 12, 2022 15:13
@przemyslaw-wlodek przemyslaw-wlodek requested review from a team June 1, 2022 15:52
@przemyslaw-wlodek przemyslaw-wlodek marked this pull request as ready for review June 1, 2022 15:53
@DominikGuzei DominikGuzei self-requested a review June 2, 2022 09:57
@przemyslaw-wlodek przemyslaw-wlodek marked this pull request as draft June 2, 2022 13:37
@przemyslaw-wlodek przemyslaw-wlodek marked this pull request as ready for review June 2, 2022 14:35
@miorsufianiohk
Copy link

Hi @przemyslaw-wlodek ,

This is the findings for build 21997.

  • I was able to reproduce all of the issues found on build 21968 as reported here

  • I also found an issue during recovery phrase verification. When I typed an invalid phrase, upon clicking "Verify" button, spinner continued spinning and console displayed error unable to "...retrieve the walletid". It should have displayed an error to the user indicating the phrase was invalid (ps: For further info, Create new wallet and Restore wallet worked as expected). Please see video for more info.

@przemyslaw-wlodek
Copy link
Contributor Author

Hi @przemyslaw-wlodek ,

This is the findings for build 21997.

  • I was able to reproduce all of the issues found on build 21968 as reported here
  • I also found an issue during recovery phrase verification. When I typed an invalid phrase, upon clicking "Verify" button, spinner continued spinning and console displayed error unable to "...retrieve the walletid". It should have displayed an error to the user indicating the phrase was invalid (ps: For further info, Create new wallet and Restore wallet worked as expected). Please see video for more info.

Thank you @miorsufianiohk !

  1. I am not able to reproduce them. Let's have a call.
  2. Good catch! It's fixed now. The issue wasn't produced by my changes but it was revealed by them. 😅

@miorsufianiohk
Copy link

Hi @przemyslaw-wlodek ,

These are the findings for build 22021

Overall, great job so far 👍 .

As discussed in our call:

  • When I tried to create a new wallet, upon pasting a valid recovery phrase which was not from the newly created wallet, the “Confirm” button was still disabled. See video. (not yet fixed)
  • I am still able to enter symbols and numbers. See screenshot (this is expected behaviour)
  • I also found an issue during recovery phrase verification. When I typed an invalid phrase, upon clicking "Verify" button, spinner continued spinning and console displayed error unable to "...retrieve the walletid". It should have displayed an error to the user indicating the phrase was invalid (ps: For further info, Create new wallet and Restore wallet worked as expected). Please see video for more info. (fixed)

@przemyslaw-wlodek
Copy link
Contributor Author

Hi @przemyslaw-wlodek ,

These are the findings for build 22021

Overall, great job so far 👍 .

As discussed in our call:

  • When I tried to create a new wallet, upon pasting a valid recovery phrase which was not from the newly created wallet, the “Confirm” button was still disabled. See video. (not yet fixed)
  • I am still able to enter symbols and numbers. See screenshot (this is expected behaviour)
  • I also found an issue during recovery phrase verification. When I typed an invalid phrase, upon clicking "Verify" button, spinner continued spinning and console displayed error unable to "...retrieve the walletid". It should have displayed an error to the user indicating the phrase was invalid (ps: For further info, Create new wallet and Restore wallet worked as expected). Please see video for more info. (fixed)

Thank you @miorsufianiohk :) I've pushed a fix. Let's catch up tomorrow for a round of live testing.

@miorsufianiohk
Copy link

miorsufianiohk commented Jul 8, 2022

Hi @przemyslaw-wlodek ,

These are the findings for build 22026.

Great jobs on fixing the issue in the previous report

When I tried to create a new wallet, upon pasting a valid recovery phrase which was not from the newly created wallet, the “Confirm” button was still disabled. See video. (fixed)

However, as discussed in our chat, I found an issue that might have regressed in the wallet recovery verification

  • For Yoroi Byron and Yoroi Shelley. Number of words were 15 but on recovery phrase dialog displayed 24

I also found an issue that potentially exists in production which I am verifying at the moment. If this is the case, I will create a separate card once verified OR should be fixed in this PR, lets discuss (Note: confirmed exists on production see this screenshot and another screenshot)

  • Byron Paper Wallet. Number of words were 27 but on recovery phrase dialog displayed 12

Please watch video here for further info

Copy link

@miorsufianiohk miorsufianiohk left a comment

Choose a reason for hiding this comment

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

LGTM. Great work @przemyslaw-wlodek 🎉 . Tested on 22080

Copy link
Contributor

@renanvalentin renanvalentin left a comment

Choose a reason for hiding this comment

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

Great job @przemyslaw-wlodek !

mouseIsOverOptions: false,
blurred: false,
}),
[]
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can keep it as it is

@danielmain danielmain merged commit bdd6ccd into develop Aug 10, 2022
@iohk-bors iohk-bors bot deleted the feature/ddw-409-new-mnemonic-input branch August 10, 2022 13:07
@danielmain danielmain added release-5.0.0 Daedalus version 5.0.0 and removed ⏳release-vNext labels Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature release-5.0.0 Daedalus version 5.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants