-
Notifications
You must be signed in to change notification settings - Fork 448
fix(LWM): prevent retry button from redirecting to wallet page in receive flow #13497
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a navigation bug in the receive flow where clicking retry after a device error (e.g., locked device) would incorrectly redirect users to the wallet page instead of returning them to the device selection screen. The fix changes navigation from replace() to navigate() to preserve the device selection screen in the navigation stack, and introduces a navigationDepth parameter to properly close inline flows.
Key Changes
- Replaced
replace()withnavigate()in device selection to keep SelectDevice in the navigation stack for retry scenarios - Added
navigationDepthparameter to track how many screens to pop when closing inline flows - Updated retry logic to navigate back to device selection for inline flows with errors
- Added integration tests to validate retry behavior and inline flow closure
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
useSelectDeviceViewModel.ts |
Changed from replace() to navigate() to preserve device selection screen in stack for retry navigation |
useDeviceNavigation.ts |
Added navigationDepth calculation (value of 2 for inline flows) to track navigation stack depth |
types.ts (DeviceSelection) |
Added optional navigationDepth parameter to CommonParams type |
types.ts (AddAccount) |
Added optional navigationDepth parameter with documentation comment |
useScanDeviceAccountsViewModel.ts |
Updated retry logic to navigate back for inline errors, added closeInlineFlow function using navigationDepth |
addAccountFlow.test.tsx |
Added integration tests for device lock retry and navigationDepth-based flow closure |
ReanimatedSwipeable.js |
Added forwardRef support and displayName to mock component |
ninety-bags-taste.md |
Added changeset describing the fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...dger-live-mobile/src/newArch/features/ModularDrawer/__integrations__/addAccountFlow.test.tsx
Outdated
Show resolved
Hide resolved
apps/ledger-live-mobile/src/newArch/features/Accounts/screens/AddAccount/types.ts
Show resolved
Hide resolved
apps/ledger-live-mobile/src/newArch/features/ModularDrawer/hooks/useDeviceNavigation.ts
Outdated
Show resolved
Hide resolved
...e/src/newArch/features/Accounts/screens/ScanDeviceAccounts/useScanDeviceAccountsViewModel.ts
Outdated
Show resolved
Hide resolved
...e/src/newArch/features/Accounts/screens/ScanDeviceAccounts/useScanDeviceAccountsViewModel.ts
Show resolved
Hide resolved
...dger-live-mobile/src/newArch/features/ModularDrawer/__integrations__/addAccountFlow.test.tsx
Outdated
Show resolved
Hide resolved
...e/src/newArch/features/Accounts/screens/ScanDeviceAccounts/useScanDeviceAccountsViewModel.ts
Show resolved
Hide resolved
|
d175642 to
5898130
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...e/src/newArch/features/Accounts/screens/ScanDeviceAccounts/useScanDeviceAccountsViewModel.ts
Show resolved
Hide resolved
...dger-live-mobile/src/newArch/features/ModularDrawer/__integrations__/addAccountFlow.test.tsx
Show resolved
Hide resolved
5898130 to
4b84907
Compare
Desktop Bundle Checks
Mobile Bundle Checks✅ Previous issues have all been fixed. |
4b84907 to
9eef20e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ledger-live-mobile/src/newArch/features/ModularDrawer/hooks/useDeviceNavigation.ts
Show resolved
Hide resolved
apps/ledger-live-mobile/src/newArch/features/ModularDrawer/hooks/useDeviceNavigation.ts
Show resolved
Hide resolved
...e/src/newArch/features/Accounts/screens/ScanDeviceAccounts/useScanDeviceAccountsViewModel.ts
Show resolved
Hide resolved
9eef20e to
c7c40d8
Compare
c7c40d8 to
41f924b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please consider rebasing this PR with develop as the PR includes tests and we should ensure they still pass on Jest 30 (as some API may have changed in Jest #13396 ) |
41f924b to
744bc46
Compare
744bc46 to
8572a08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("should return to device selection on retry when device is locked in inline flow", async () => { | ||
| const { user, getByText, queryByText, getByTestId } = render( | ||
| <ModularDrawerSharedNavigator flow="not_add_account" />, | ||
| ); | ||
|
|
||
| // Navigate through the add account flow | ||
| expect(getByText(WITH_ACCOUNT_SELECTION)).toBeVisible(); | ||
| await user.press(getByText(WITH_ACCOUNT_SELECTION)); | ||
| advanceTimers(); | ||
|
|
||
| expect(getByText(/bitcoin/i)).toBeVisible(); | ||
| await user.press(getByText(/bitcoin/i)); | ||
| advanceTimers(); | ||
|
|
||
| expect(getByText(/add new or existing account/i)).toBeVisible(); | ||
| await user.press(getByText(/add new or existing account/i)); | ||
| advanceTimers(); | ||
|
|
||
| expect(getByText(/connect device/i)).toBeVisible(); | ||
| advanceTimers(); | ||
|
|
||
| const deviceItem = getByText(/ledger stax/i); | ||
| expect(deviceItem).toBeVisible(); | ||
| await user.press(deviceItem); | ||
| advanceTimers(); | ||
|
|
||
| // Wait for scanning to start | ||
| await waitFor(() => { | ||
| expect(getByText(/checking the blockchain/i)).toBeVisible(); | ||
| }); | ||
|
|
||
| // Trigger device locked error | ||
| await act(() => { | ||
| triggerError?.(new Error("Device locked")); | ||
| }); | ||
|
|
||
| // Wait for error modal to appear | ||
| await waitFor(() => { | ||
| expect(queryByText("Device locked")).toBeVisible(); | ||
| }); | ||
|
|
||
| // Click Retry button (only Retry has testID="proceed-button", not Cancel) | ||
| const retryButton = getByTestId("proceed-button"); | ||
| await user.press(retryButton); | ||
|
|
||
| // Should return to device selection screen (not to wallet) | ||
| await waitFor(() => { | ||
| expect(getByText(/connect device/i)).toBeVisible(); | ||
| expect(queryByText(/checking the blockchain/i)).not.toBeVisible(); | ||
| }); | ||
| }); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test suite only validates retry behavior for inline flows. Consider adding a test case for the retry behavior in non-inline flows (when inline is false) to ensure restartSubscription is called correctly in that scenario.
8572a08 to
dc6763e
Compare
|



✅ Checklist
npx changesetwas attached.📝 Description
This PR fixes a navigation bug where retrying after a device error (e.g., locked device) in the receive flow would redirect to the wallet page. The fix uses navigate() instead of replace() to preserve the device selection screen in the stack, allowing proper retry navigation. A navigationDepth parameter is introduced to correctly close inline flows, with integration tests added to validate the behavior.
Add
forwardRefsupport to ReanimatedSwipeable mock for proper ref handling in integration tests.Screen.Recording.2026-01-12.at.11.08.45.mov
❓ Context
🧐 Checklist for the PR Reviewers