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

feat: ledger account selection screen add hd options to sync with extension #10755

Merged
merged 61 commits into from
Sep 10, 2024

Conversation

dawnseeker8
Copy link
Contributor

@dawnseeker8 dawnseeker8 commented Aug 23, 2024

Description

This PR is to add hd path selection drop down to Ledger Account selector screen to sync with Extension ledger features.
this PR has added following changes:

  1. add the hd path selection drop down list into LedgerSelectAccount.ts
  2. styling the relative components to make it match what Alex want.
  3. add locale message for the new components.

Related issues

Fixes: #10678

Manual testing steps

  1. Connect your ledger to your laptop usb
  2. Open both Metamask mobile and Metamask extension.
  3. firstly connect your ledger to metamask extension and open the account selection screen. and select ledger live path in drop down in metamask extension
  4. Connect your ledger again in metamask moble via bluetooth, and make sure you see the same account as step 3 list.
  5. in both metamask mobile and mestamask extension, click Next or Previous button and cnofirm that the account lists are same in both mobile and extensions.
  6. Change the drop down to BIP44 or legacy address list in both extensions and mobile. make sure mobile list matched the extensions list in both selection.
  7. Finally tick the accounts you want to import in metamask mobile, and click unlock to make sure all accounts can be imported correctly.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@dawnseeker8 dawnseeker8 added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-hardware-wallets labels Aug 23, 2024
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@dawnseeker8 dawnseeker8 added the Run Smoke E2E Triggers smoke e2e on Bitrise label Aug 27, 2024
Copy link
Contributor

github-actions bot commented Aug 27, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: b709db9
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/107d571e-5220-4497-93f6-39f49e4201ec

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@dawnseeker8 dawnseeker8 marked this pull request as ready for review August 27, 2024 09:41
@dawnseeker8 dawnseeker8 requested a review from a team as a code owner August 27, 2024 09:41
@dawnseeker8 dawnseeker8 added the release-7.31.0 Issue or pull request that will be included in release 7.31.0 label Aug 27, 2024
@dawnseeker8 dawnseeker8 marked this pull request as draft August 28, 2024 06:31
@dawnseeker8 dawnseeker8 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 3ac093f
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3d882f3d-d072-4d82-a96f-4466aa891cae

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@dawnseeker8 dawnseeker8 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 10, 2024
Copy link
Contributor

github-actions bot commented Sep 10, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 5119e9b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bdb78a05-e331-428f-8216-1cbef053a912

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@vivek-consensys vivek-consensys added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 10, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 61.53846% with 45 lines in your changes missing coverage. Please review.

Project coverage is 52.92%. Comparing base (8dcdd3c) to head (1901f58).
Report is 65 commits behind head on main.

Files with missing lines Patch % Lines
app/components/Views/LedgerSelectAccount/index.tsx 22.64% 40 Missing and 1 partial ⚠️
...p/components/UI/SelectOptionSheet/OptionsSheet.tsx 88.23% 1 Missing and 1 partial ⚠️
app/core/Ledger/Ledger.ts 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10755      +/-   ##
==========================================
+ Coverage   52.75%   52.92%   +0.16%     
==========================================
  Files        1534     1549      +15     
  Lines       36777    37025     +248     
  Branches     4335     4376      +41     
==========================================
+ Hits        19403    19596     +193     
- Misses      16058    16104      +46     
- Partials     1316     1325       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

NicolasMassart
NicolasMassart previously approved these changes Sep 10, 2024
Copy link
Contributor

@NicolasMassart NicolasMassart 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 to me.

@dawnseeker8 dawnseeker8 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 10, 2024
Copy link
Contributor

github-actions bot commented Sep 10, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 562f9f2
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/28532ea4-db0c-41dc-915d-4f8b61b6ada9

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Copy link

Copy link
Contributor

@NicolasMassart NicolasMassart 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 to me.

@dawnseeker8 dawnseeker8 merged commit 54a6b03 into main Sep 10, 2024
42 checks passed
@dawnseeker8 dawnseeker8 deleted the feat/10678-hd-options-sync-with-extension branch September 10, 2024 09:23
@github-actions github-actions bot locked and limited conversation to collaborators Sep 10, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Sep 10, 2024
@metamaskbot metamaskbot added the release-7.32.0 Issue or pull request that will be included in release 7.32.0 label Sep 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed A successful QA run through has been done release-7.31.0 Issue or pull request that will be included in release 7.31.0 release-7.32.0 Issue or pull request that will be included in release 7.32.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-accounts team-hardware-wallets
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Ledger - Hd options sync with extension
5 participants