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.

Sign up for GitHub

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

Jump to bottom

[FC] Fix - Back press navigation on first screen of the flow #7290

Merged
merged 2 commits into from
Sep 12, 2023
Merged

[FC] Fix - Back press navigation on first screen of the flow #7290

merged 2 commits into from
Sep 12, 2023

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Sep 12, 2023 •
edited

Summary

  • After navigation refactor, clicking back when the back stack is empty triggers onBackPressedDispatcher
  • That enters in a loop that triggers the back handler again, resulting in a crash
  • Fix: just call viewModel.onBackPressed when the back stack is empty. That'll trigger the close logic properly.

Testing

  • Test clicking back on the first screen (consent)

@carlosmuvi-stripe carlosmuvi-stripe changed the title [FC] Fix - Back press navigation [FC] Fix - Back press navigation on first screen of the flow Sep 12, 2023
carlosmuvi-stripe
carlosmuvi-stripe commented Sep 12, 2023
View reviewed changes
...-connections/src/main/java/com/stripe/android/financialconnections/navigation/Destination.kt
@@ -160,7 +160,7 @@ internal sealed class Destination(

object ManualEntrySuccess : Destination(
route = Pane.MANUAL_ENTRY_SUCCESS.value,
paramKeys = listOf(KEY_MICRODEPOSITS, KEY_LAST4),
paramKeys = listOf(KEY_REFERRER, KEY_MICRODEPOSITS, KEY_LAST4),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also added this missing navigation key on a PR merged into master.

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review September 12, 2023 01:12
@carlosmuvi-stripe carlosmuvi-stripe enabled auto-merge (squash) September 12, 2023 01:18
@carlosmuvi-stripe carlosmuvi-stripe merged commit a3d9b3e into master Sep 12, 2023
8 checks passed
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/add-missing-referrer-key branch September 12, 2023 01:22
@samer-stripe samer-stripe mentioned this pull request Sep 12, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samer-stripe samer-stripe samer-stripe approved these changes

@jaynewstrom-stripe jaynewstrom-stripe Awaiting requested review from jaynewstrom-stripe jaynewstrom-stripe is a code owner automatically assigned from stripe/android-sdk-reviewers

Assignees

@ccen-stripe ccen-stripe

@jameswoo-stripe jameswoo-stripe

@tillh-stripe tillh-stripe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@carlosmuvi-stripe @samer-stripe @ccen-stripe @jameswoo-stripe @tillh-stripe