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] : WalletSync - Synchronize with LLD πŸ”„ #7023

Merged
merged 20 commits into from
Jun 10, 2024

Conversation

mcayuelas-ledger
Copy link
Contributor

@mcayuelas-ledger mcayuelas-ledger commented Jun 6, 2024

βœ… Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

πŸ“ Description

Synchronize Flow

  • QRCode Screen βœ…
  • PinCode Screen βœ…
Screen.Recording.2024-06-06.at.17.05.44.mov

❓ 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 Jun 6, 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 Jun 7, 2024 1:15pm
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2024 1:15pm
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2024 1:15pm
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2024 1:15pm
web-tools ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2024 1:15pm

Copy link
Member

@KVNLS KVNLS left a comment

Choose a reason for hiding this comment

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

Could we add a debug cta to start a specific flow in developer settings please? Because here we can't test the activation flow without changing the code.

Copy link
Contributor

@LucasWerey LucasWerey left a comment

Choose a reason for hiding this comment

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

LGTM !

{ element: t("walletSync.synchronize.qrCode.steps.step3") },
];

// TO CHANGE WHEN INTRAGRATION WITH TRUSTCHAIN
Copy link
Contributor

Choose a reason for hiding this comment

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

notes to anticipate a bit the integration:

this code may just disappear because we need to be able to "control" the steps, it's not the step that will do a call so some api and do something based on this because we will have a global function to call at the beginning and we will just hook to the various events.

so i think we don't need to have the "displayPinCode" injected as a prop neither, it's going to be a switch of step from the parent directly.

see the usage here for instance: https://github.com/LedgerHQ/ledger-live/blob/live-12627/apps/web-tools/trustchain/components/App.tsx#L468-L479 (generally speaking, this App.tsx file will act as general/canonical examples for all trustchain integrations)

createQRCodeHostInstance({
      onDisplayQRCode: url => {
        setUrl(url);
      },
      onDisplayDigits: digits => {
        setDigits(digits);
      },
      addMember: async member => {
        // this sdk will be somehow available as a react context when we do the trustchain integration
        const jwt = await sdk.liveAuthenticate(trustchain, liveCredentials);
        await sdk.addMember(jwt, trustchain, liveCredentials, member);
      },
    })
      .catch(e => {
        if (e instanceof InvalidDigitsError) {
          return; // in this case, we could display an error of invalid digital error (it can also be handled by the generic component)
        }
        setError(e); // in this case, we render the generic error (we have a generic component for handling of errors) , typically it can be Networking error, but there could be also protocol errors
      })
      .then(() => {
        // everything is finished. the other member was added βœ… 
      });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah nice, I just putted that to simulate a behavior to develop UI but I'll make another PR right now to start the job on this part :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are currently able to control which step we want to display :)

Copy link
Contributor

@gre gre left a comment

Choose a reason for hiding this comment

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

some notes to anticipate the integration, i let you decide if we fix this in a future PR, but the integration will have to modify a bunch of code (and not just the block of // INTEGRATION that you have defined) specifically for the qr code flow that needs to be one function call for the whole thing (since it's managing a websocket channel)

@mcayuelas-ledger mcayuelas-ledger requested a review from a team as a code owner June 7, 2024 13:12
@mcayuelas-ledger mcayuelas-ledger merged commit 008108c into feat/ws-activation-flow-lld Jun 10, 2024
40 of 41 checks passed
@mcayuelas-ledger mcayuelas-ledger deleted the feat/ws-synchronize-lld branch June 10, 2024 13:29
mcayuelas-ledger added a commit that referenced this pull request Jun 10, 2024
mcayuelas-ledger added a commit that referenced this pull request Jun 11, 2024
…h LLD flows (#6885)

* Initial setup on folders

* Fix key

* Reorg files using newArch folder

update imports

* [WIP]: WS activation flow (connect device)

* add router

* Generic Error/success screen

Reorg in new arch

* [WIP]: WS activation flow (connect device)

* add router

* Reorg in new arch

* Fix files

* Error/success screens

* use DeviceAction

* add loading state

* UI polish and fix

* Add loading screen for WS activation

* Add loader animation

* πŸ›οΈ  Regorg files

* ⚑[FEAT]: WS Manage your Backup in LLD (#6988)

* remove useless files

* remove useless files

* ⚑[FEAT]: WS Manage your Backup in LLD

* changeset

* Add Loading + error screen

* Add Integration test

* Reworks steps

* Add unsecured Error

* fix WS tests

* update flow

* fix path

* Fix tests

* transition between flows

* Add test for useFlow

* add ManageBackup analytics

* Fix reset flow

* Mock Analytics

---------

Co-authored-by: Maxime Aubanel <[email protected]>

* Rename steps

* ⚑️ [FEAT] : WalletSync - Synchronize with LLD πŸ”„ (#7023)

* ⚑[FEAT]: WS Manage your Backup in LLD

---------

Co-authored-by: Maxime Aubanel <[email protected]>
kallen-ledger pushed a commit that referenced this pull request Jun 26, 2024
…h LLD flows (#6885)

* Initial setup on folders

* Fix key

* Reorg files using newArch folder

update imports

* [WIP]: WS activation flow (connect device)

* add router

* Generic Error/success screen

Reorg in new arch

* [WIP]: WS activation flow (connect device)

* add router

* Reorg in new arch

* Fix files

* Error/success screens

* use DeviceAction

* add loading state

* UI polish and fix

* Add loading screen for WS activation

* Add loader animation

* πŸ›οΈ  Regorg files

* ⚑[FEAT]: WS Manage your Backup in LLD (#6988)

* remove useless files

* remove useless files

* ⚑[FEAT]: WS Manage your Backup in LLD

* changeset

* Add Loading + error screen

* Add Integration test

* Reworks steps

* Add unsecured Error

* fix WS tests

* update flow

* fix path

* Fix tests

* transition between flows

* Add test for useFlow

* add ManageBackup analytics

* Fix reset flow

* Mock Analytics

---------

Co-authored-by: Maxime Aubanel <[email protected]>

* Rename steps

* ⚑️ [FEAT] : WalletSync - Synchronize with LLD πŸ”„ (#7023)

* ⚑[FEAT]: WS Manage your Backup in LLD

---------

Co-authored-by: Maxime Aubanel <[email protected]>
kallen-ledger pushed a commit that referenced this pull request Jun 26, 2024
kallen-ledger pushed a commit that referenced this pull request Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop Has changes in LLD translations Translation files have been touched ui Has changes in the design system library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants