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

fix: present hw pairing sheet during swap #6275

Merged
merged 9 commits into from
Dec 3, 2024
Merged

Conversation

derHowie
Copy link
Member

@derHowie derHowie commented Nov 20, 2024

Fixes APP-1928

What changed (plus any additional context for devs)

We have added the ledger bluetooth sheet back to the swap flow. Ledger transactions were working, however we were not displaying the various bluetooth connection states.

Screen recordings / screenshots

RPReplay_Final1732665403.MP4

What to test

Attempt a swap with a ledger device. Make sure you see the bluetooth connection sheet.

Copy link

linear bot commented Nov 20, 2024

@derHowie derHowie force-pushed the @chris/swap-ledger-sheet branch from 4239d6d to 71db231 Compare November 20, 2024 17:47
@derHowie derHowie marked this pull request as ready for review November 20, 2024 18:11
@brunobar79
Copy link
Member

Launch in simulator or device for bd764c1

@brunobar79
Copy link
Member

Launch in simulator or device for c873bef

@derHowie derHowie marked this pull request as draft November 21, 2024 18:09
@derHowie derHowie marked this pull request as ready for review November 22, 2024 23:02
@brunobar79
Copy link
Member

Launch in simulator or device for cc70300

@brunobar79
Copy link
Member

Launch in simulator or device for 09e1890

@brunobar79
Copy link
Member

Launch in simulator or device for 931b939

@brunobar79
Copy link
Member

Launch in simulator or device for afdb5a8

@brunobar79
Copy link
Member

Launch in simulator or device for b5511b9

@brunobar79
Copy link
Member

Launch in simulator or device for b8f0d32

Comment on lines 343 to 347
if (
index === 0 ||
routes[index - 1] === Routes.EXPANDED_ASSET_SHEET ||
activeRoute.name === Routes.PAIR_HARDWARE_WALLET_AGAIN_SHEET
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this wasn't introduced in your PR, but can we specify what these checks are? I'm a little confused what index === 0 implies. Something like:

const isOnlyRoute = index === 0;  // not sure if this is actually what this means?
const isFromExpandedSheet = ...
const isFromHardwareWalletSheet = ...

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a feeling that both of those cases do not fit the typical shape of routes. I hadn't yet had time to check out that expanded sheet flow yet.

I'll try to verify that either of these are necessary. Then make them more legible.

Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

nit but lgtm

@brunobar79 brunobar79 added the release for release blockers and release candidate branches label Dec 2, 2024
@derHowie derHowie merged commit e30b444 into develop Dec 3, 2024
8 of 9 checks passed
@derHowie derHowie deleted the @chris/swap-ledger-sheet branch December 3, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release for release blockers and release candidate branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants