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

task(recovery): Redo seed enter recovery page #1915

Merged
merged 12 commits into from
Mar 12, 2024

Conversation

Flemmli97
Copy link
Collaborator

@Flemmli97 Flemmli97 commented Mar 8, 2024

What this PR does 📖

  • Redo the seed recovery page. This PR changes the page to match the style of the other seed display pages.
    The phrases are now split into their own input. Typing enter will go to the next input unless the user is at the end where it will use act as the recovery button.
    Copy pasting an input with the phrases split into new lines will auto paste them into their correct places.
    E.g.
suspect
number
enemy
liar
estate
usual
state
total
dizzy
alley
eyebrow
spatial

Having too many or incomplete phrases will only paste the relevant bits in.

Screenshot 2024-03-08 at 17 56 40
  • This PR also makes copying the seed phrases easier by disabling selection on the seed number so the user can simply mouse drag select over the phrases to select them
  • This PR also adds an button to directly copy the seed for both the create account seed page and the settings show seed section

Which issue(s) this PR fixes 🔨

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev labels Mar 8, 2024
Copy link
Contributor

github-actions bot commented Mar 8, 2024

UI Automated Test Results Summary for MacOS/Windows

527 tests   476 ✅  3h 0m 56s ⏱️
 44 suites   47 💤
  3 files      2 ❌  2 🔥

For more details on these failures and errors, see this check.

Results for commit b2c171c.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Mar 8, 2024

UI Automated Tests execution is complete! You can find the test results report here

@phillsatellite
Copy link
Contributor

Worked very well on MacOS but on Windows I noticed an issue (I had Sheldon verify that this happens on his side as well) when you paste your entire seed on Windows each phrase has \r behind it

image

Copy link
Member

@stavares843 stavares843 left a comment

Choose a reason for hiding this comment

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

what phil mentioned above

@phillsatellite phillsatellite added the Changes requested When other dev or QA request a change label Mar 8, 2024
@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Mar 11, 2024
@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Mar 11, 2024
@luisecm
Copy link
Contributor

luisecm commented Mar 11, 2024

@Flemmli97 Automated tests failures on import account tests are expected on this PR since there is a change in the info text from the screen and also the layout is changing. I will push the corresponding test updates for this screen once that this PR is merged
image

@stavares843
Copy link
Member

thanks @luisecm 🔨

@stavares843
Copy link
Member

Gravacao.do.ecra.2024-03-11.as.17.44.06.mov

if the seed is invalid, error appears as expected but the buttons are hidden unless you maximise the window

@stavares843
Copy link
Member

Gravacao.do.ecra.2024-03-11.as.17.40.43.mov

the toast is a bit cut off as well

@stavares843
Copy link
Member

macOS, m1

@stavares843 stavares843 added the Changes requested When other dev or QA request a change label Mar 11, 2024
@Flemmli97
Copy link
Collaborator Author

@Flemmli97 Automated tests failures on import account tests are expected on this PR since there is a change in the info text from the screen and also the layout is changing. I will push the corresponding test updates for this screen once that this PR is merged image

ye i updated a translation text too to better reflect what the user now has to do

@Flemmli97
Copy link
Collaborator Author

Gravacao.do.ecra.2024-03-11.as.17.40.43.mov

the toast is a bit cut off as well

hmm toast is supposed to look like that. but i guess it doesnt look too good

@stavares843
Copy link
Member

yes i mean we can ignore the toast for now

is possible to add a scroll or to make the default window size bigger? because when the error appears the user does not see the buttons as they are hidden

cc @Flemmli97

@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Mar 12, 2024
@github-actions github-actions bot added the linter failing Cargo Workflow (linter) failed on this PR label Mar 12, 2024
@github-actions github-actions bot removed the linter failing Cargo Workflow (linter) failed on this PR label Mar 12, 2024
@stavares843 stavares843 added the QA Tested QA has tested and approved label Mar 12, 2024
@dariusc93 dariusc93 added Ready to Merge This PR is ready to merge and removed Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE labels Mar 12, 2024
@stavares843 stavares843 added Waiting for CI Waiting for at least one CI job to complete before merging and removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Mar 12, 2024
@stavares843 stavares843 merged commit c02112f into dev Mar 12, 2024
5 of 7 checks passed
@stavares843 stavares843 deleted the rework_recovery_seed_page branch March 12, 2024 22:57
@github-actions github-actions bot added Failed Automated Test This PR is failing Luis's Appium test and needs revised and removed Waiting for CI Waiting for at least one CI job to complete before merging labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Failed Automated Test This PR is failing Luis's Appium test and needs revised
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task(importAccount): Show seed words entered by user in a list
5 participants