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

[LIVE-10526][LLM] Fix navigation to firmware update #6536

Merged
merged 11 commits into from
Apr 5, 2024

Conversation

ofreyssinet-ledger
Copy link
Contributor

@ofreyssinet-ledger ofreyssinet-ledger commented Mar 25, 2024

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests. Too much refactoring needed (mocking etc.) to test the full navigation flows here, for now, unfortunately.
  • Impact of the changes:
    • New firmware update flow (Stax):
      • Navigation to firmware update from: (and navigation back, when closing the screen or when finishing the update)
        • Wallet screen
        • My Ledger screen
        • Stax onboarding (early security checks)
          • Non onboarded LL
          • Onboarded LL
    • Old firmware update flow (Nano S, SP and X, via USB, Android)
      • Navigation to firmware update (and back) from:
        • Wallet screen
        • My Ledger screen

📝 Description

Issue: There is an issue sometimes when navigating to the (new) firmware update flow: the Tab Bar at the bottom of the screen doesn't hide properly.
Reason: This is because we have done something wrong with the navigation setup for the FirmwareUpdate and it was inside the MainNavigator which is the navigator that has the Tab Bar. We have done a hacky workaround to hide the TabBar when the Firmware update screen is displayed but it's not the proper way to do it.
Solution: Moving the FirmwareUpdate screen out of the MainNavigator, directly in BaseNavigator, and also in SyncOnboardingNavigator (for the "early security checks" flow of the onboarding).

On top of that, I've done some refactoring of the navigator/screen names and directory names for the My Ledger section because it was very confusing. (Manager is now called My Ledger in the UI, and the naming for the two screens, Manager and ManagerMain was just confusing and impossible to understand on the first try).
The 2 screens of MyLedgerNavigator are now called: MyLedgerChooseDevice and MyLedgerDevice, I hope it is now self explanatory.

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

Copy link

vercel bot commented Mar 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-docs ⬜️ Ignored (Inspect) Visit Preview Apr 4, 2024 2:51pm
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Apr 4, 2024 2:51pm
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Apr 4, 2024 2:51pm
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Apr 4, 2024 2:51pm
web-tools ⬜️ Ignored (Inspect) Visit Preview Apr 4, 2024 2:51pm

@live-github-bot live-github-bot bot added the mobile Has changes in LLM label Mar 25, 2024
@ofreyssinet-ledger ofreyssinet-ledger changed the title LIVE-10526 [LIVE-10526][LLM] Fix navigation to firmware update Mar 26, 2024
@ofreyssinet-ledger ofreyssinet-ledger force-pushed the bugfix/LIVE-10526-fwupdate-footer branch 2 times, most recently from 0ab8bf1 to b3ab4be Compare April 2, 2024 08:17
@ofreyssinet-ledger ofreyssinet-ledger marked this pull request as ready for review April 4, 2024 13:53
@ofreyssinet-ledger ofreyssinet-ledger requested review from a team as code owners April 4, 2024 13:53
@ofreyssinet-ledger ofreyssinet-ledger force-pushed the bugfix/LIVE-10526-fwupdate-footer branch from b3ab4be to 54f1b5d Compare April 4, 2024 14:28
Copy link
Contributor

@mcayuelas-ledger mcayuelas-ledger left a comment

Choose a reason for hiding this comment

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

LGTM for Hub part

@ofreyssinet-ledger ofreyssinet-ledger merged commit 4ae85cb into develop Apr 5, 2024
39 checks passed
@ofreyssinet-ledger ofreyssinet-ledger deleted the bugfix/LIVE-10526-fwupdate-footer branch April 5, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Has changes in LLM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants