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

[FC-V3] Bottom sheet navigation support via Accompanist #7882

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Feb 5, 2024

Summary

  • Adds Accompanist navigation, that allows having navigation destinations as BottomSheets.
    • This requires wrapping the NavHost into a ModalBottomSheetLayout.
    • We already do that just for the exit modal, so converted it to a navigation destination. That way bottom sheet behavior is consistent across the AuthFlow.
  • PARTNER_AUTH (and its prepane) now can show both as full screen and within a bottom sheet.
    • To handle that, created two different destinations with the same content.
    • The UI slightly changes depending on if shown full-screen or modal, so a shownInModal parameter is passed to the screen composable to make the appropiate tweaks.

Next steps

  • Error screens still should show full screen. We'll follow web approach: An error navigation destination that unifies error logic across panes.
  • Investigate not depending on Accompanist while keeping the current functionality.
prepane-test.mp4

Motivation

📔  Bottom sheet navigation support
🌐  BANKCON-9190

Testing

  • Added tests
  • Modified tests
  • Manually verified

@carlosmuvi-stripe carlosmuvi-stripe added the work-cli Added to pull requests created with #work-cli for usage tracking. label Feb 5, 2024
@carlosmuvi-stripe carlosmuvi-stripe self-assigned this Feb 5, 2024
Comment on lines 42 to 45
DisposableEffect(Unit) {
onDispose { viewModel.onCloseDismiss() }
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ensures post-close logic is run even if exit modal is dismissed via clicking outside it.

sheetState = bottomSheetState,
sheetContent = {
state.dataAccess?.let {
DataAccessBottomSheetContent(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DataAccessBottomSheetContent does not exist in v3 anymore.

…KCON-9190/fc-v3-bottom-sheet-navigation-support-via-accompan

# Conflicts:
#	financial-connections/api/financial-connections.api
#	financial-connections/src/main/java/com/stripe/android/financialconnections/features/bankauthrepair/BankAuthRepairScreen.kt
#	financial-connections/src/main/java/com/stripe/android/financialconnections/features/common/SharedPartnerAuth.kt
#	financial-connections/src/main/java/com/stripe/android/financialconnections/features/partnerauth/PartnerAuthViewModel.kt
#	financial-connections/src/main/java/com/stripe/android/financialconnections/navigation/Destination.kt
#	financial-connections/src/main/java/com/stripe/android/financialconnections/navigation/DestinationMappers.kt
#	financial-connections/src/main/java/com/stripe/android/financialconnections/ui/FinancialConnectionsSheetNativeActivity.kt
#	financial-connections/src/test/java/com/stripe/android/financialconnections/features/institutionpicker/InstitutionPickerViewModelTest.kt
) { /*TODO*/ }
onWebAuthFlowFinished = { /*TODO*/ },
onViewEffectLaunched = { /*TODO*/ },
inModal = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feature is not yet available in the SDK, but once it is, it'll display the partner auth pane full-screen.

Comment on lines +83 to +88
private fun restoreOrCreateAuthSession() = suspend {
// A session should have been created in the previous pane and set as the active
// auth session in the manifest.
// if coming from a process kill, we'll fetch the current manifest from network,
// that should contain the active auth session.
val sync: SynchronizeSessionResponse = getOrFetchSync()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

key change: The Auth Session is now created on the pane that launches the partner_auth pane, rather than within the pane itself.

This mimics what web does, and it's also a requirement since we need to know if the session is OAuth or not in order to decide if the pane will be bottom sheet or full screen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replicated from Accompanist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replicated from Accompanist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replicated from Accompanist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replicated from Accompanist.

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review February 8, 2024 18:52
@carlosmuvi-stripe carlosmuvi-stripe requested review from samer-stripe and tillh-stripe and removed request for a team and samer-stripe February 8, 2024 18:52
Comment on lines 42 to 44
DisposableEffect(Unit) {
onDispose { viewModel.onCloseDismiss() }
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this cause onDispose to be called on configuration changes? If so, we should find a different solution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point! ended up moving the exit logic to its own viewmodel. That prevents parent viewmodel communication, dispose logic should not be needed anymore:

2769755

Comment on lines 51 to 52
onExit: () -> Unit = { },
onCancel: () -> Unit = { },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I feel like these shouldn’t be optional. It makes the Preview code noisier, but I’d rather lean on the safe side and have the compiler complain if we forget to provide a parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

made them required

@carlosmuvi-stripe carlosmuvi-stripe merged commit 803332f into carlosmuvi/i/v3-navigation-changes Feb 13, 2024
6 of 8 checks passed
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/BANKCON-9190/fc-v3-bottom-sheet-navigation-support-via-accompan branch February 13, 2024 17:57
carlosmuvi-stripe added a commit that referenced this pull request Feb 13, 2024
* [FC-V3] Support display errors as navigation destinations (#7892)

* [skip ci] Start PR

* Adds error handler to simplify usage.

* Reverts changes.

* Properly handles reset and close cases.

* Reverts institution picker changes.

* Removes function.

* Re-enables back handler.

* Makes error handler executable.

* Adds test error.

* Makes function private

* Uses factory.

* Regenerates API.

* [FC-V3] Bottom sheet navigation support via Accompanist (#7882)

* [skip ci] Start PR

* First commit - adds accompanist lib and opens prepane in drawer.

* Moves exit pane to a destination.

* Removes modal.

* Handles modal UI within partner auth pane.

* Removes unused method.

* Updates screenshots.

* Updates dependencies file.

* Replaces Accompanist dependency by local replica.

* Regenerates API.

* Merge fixes.

* Adds comment.

* Adds tests.

* Regenerates screenshots and adds more.

* Regenerates API.

* Updates tests.

* Adds accompanist attribution.

* Moves exit logic to its own viewmodel.

* Makes callbacks non-optional.

* Uses financial connections modal for navigation.
carlosmuvi-stripe added a commit that referenced this pull request Mar 13, 2024
* [FC-V3] Add V3 typography and colors (#7612)

* [skip ci] Start PR

* Adds new typefaces.

* Updates screenshots.

* Style fixes.

* Adds new colors.

* Generates screenshots.

* Updates colors.

* Adds background to previews.

* api dump.

* [FC-V3] Update Consent Pane (#7621)

* [skip ci] Start PR

* WIP

* WIP

* Updates loading.

* Removes resources.

* Adds screenshots.

* Reverts loading.

* Style fixes.

* Removes resource.

* [FC-V3] Update Data Access and Legal dialogs (#7634)

* [skip ci] Start PR

* Updates models.

* Updates UI.

* Adds legal details and data access styles.

* Updates layout to support modal mode.

* Updates list items.

* Updates screenshots.

* Ktlint.

* Detekt.

* Uses items indexed and last index.

* Removes row.

* Uses animated content to transition between image states.

* Adds debug resources.

* Ktlint fixes.

* Adds divider on content scroll and elevation in consent.

* Adds divider on content scroll and elevation in consent.

* Adds top shadow when content is scrollable.

* Documents Layout.

* Ktlint fixes.

* [FC-V3] Update Institution Picker Pane to v3 (#7641)

* [skip ci] Start PR

* WIP

* Adds loading / disabled states.

* Removes search mode.

* Removes search mode.

* Extracts institutionIcon and uses plain lazy column.

* Updates shadows on institution icons.

* Updates api dump.

* Adds clear search button.

* Ktlint fixes.

* Disable "search more" when loading.

* [FC-V3] Update Partner Auth and Prepane to V3 (#7696)

* [skip ci] Start PR

* Updates screen.

* Adds preview.

* Adds loading shimmer screenshots.

* Updates screenshots.

* [FC-V3] Add Exit bottom sheet dialog (#7769)

* [skip ci] Start PR

* Updates exit style.

* Adds icon.

* Generates screenshots.

* Generates screenshots.

* Updates bottom padding.

* [FC-V3] Update Manual Entry Pane to V3 (#7780)

* [skip ci] Start PR

* Updates manual entry content.

* Updates API.

* [Android] Update Account Picker Pane to V3 (#7736)

* [skip ci] Start PR

* Updates account view.

* Updates merchant data access box.

* Updates loading and error screens.

* Updates screenshots.

* Adds haptic feedback.

* Updates balance badge.

* Regenerates screenshots.

* Pascal case for enum.

* ContentAlpha.

* Adds spacedBy.

* Improves preview.

* Uses content alpha.

* Detekt fixes.

* Reverts content alpha.

* Updates API.

* [FC-V3] Update Networkng Link Signup to v3  (#7802)

* [skip ci] Start PR

* Updates signup pane.

* Adds legal details notice.

* Updates selected color and elevation.

* Updates screenshots.

* Ktlint fixes.

* [Android] Update Success Pane to V3 (#7761)

* [skip ci] Start PR

* Adds success pane.

* Improves animation.

* Adds animation constants.

* Updates texts.

* Adds preview parameter provider.

* Adds preview parameter provider.

* Updates booleans for readability.

* Removes api field.

* Apidump.

* (WIP) updates manual entry success.

* Uses success content in manual entry success.

* Ktlint fixes.

* Updates screenshots.

* Uses delegation and method reference.

* uses graphic layer.

* removes count.

* Adds comment.

* Uses last4 directly.

* Moves preview parameter to composable.

* Updates screenshots.

* Updates api.

* Adds link messaging.

* [Android] Update link account picker pane to v3 (#7804)

* [skip ci] Start PR

* Uses institution icon from partner account response.

* Uses layout.

* Updates loading view

* Updates screenshots.

* Adds missing screenshots.

* Adds click handler.

* Use clickable handler in Consent.

* Unifies mainContent.

* Updates API.

* Uses shared modal bottom sheet.

* Updates screenshots.

* Just navigates on successful attach.

* Update release notes for 20.37.0 (#7803)

* Bump version to 20.37.0 (#7805)

* Generate dokka for 20.37.0 (#7808)

* Improve stability of end to end tests. (#7811)

* Add CVC to `confirm` call in Link passthrough mode. (#7812)

* Show Link email & fix dark mode issues with `LinkButton`. (#7813)

* Make tweaks to Link inline signup (#7801)

* Make tweaks to Link inline signup

* Address code review feedback

- Make names consistent
- Add test for case where prefill is desired

* Update screenshots (#7815)

Co-authored-by: runner <runner@Mac-1706055070232.local>

* Use a more stable emulator. (#7818)

* [FC-V3] Update networking login warmup pane (#7830)

* [skip ci] Start PR

* Updates warmup screen.

* Regenerates screenshots.

* Exposes contentDescription.

* Updates spacing.

* Ktlint fix.

* Uses emoji

* Use @.

* [FC-V3] Update error screens (#7845)

* [skip ci] Start PR

* Regenerates screenshots.

* Detekt fixes.

* Regenerates API.

* Updates buttons.

* [FC-V3] Update networking verification panes (#7834)

* [skip ci] Start PR

* Exposes OTP customization fields.

* Updates verification panes.

* Uses alpha.

* Updates screenshots.

* Updates styles.

* Adds single task to sample app.

* Updates screenshots.

* Ktlint fixes.

* Fixes bottom sheet borders.

* Adds error feedback.

* Shows loading on prepane CTA while authorizing account.

* Hides institution icons on account picker.

* Formats urls.

* Updates screenshots.

* Updates line height.

* Slows down consent animation.

* Updates prepane, adds CTA and image style.

* Updates shadows.

* Fixes success text saving account to link for new user.

* Always shows footer in signup page.

* Updates verification title.

* Uses redacted formatted phone number.

* Improves shimmer.

* Adds email address label and updates shared colors.

* Updates label.

* Removes shimmer.

* [V3] Navigation changes (#7922)

* [FC-V3] Support display errors as navigation destinations (#7892)

* [skip ci] Start PR

* Adds error handler to simplify usage.

* Reverts changes.

* Properly handles reset and close cases.

* Reverts institution picker changes.

* Removes function.

* Re-enables back handler.

* Makes error handler executable.

* Adds test error.

* Makes function private

* Uses factory.

* Regenerates API.

* [FC-V3] Bottom sheet navigation support via Accompanist (#7882)

* [skip ci] Start PR

* First commit - adds accompanist lib and opens prepane in drawer.

* Moves exit pane to a destination.

* Removes modal.

* Handles modal UI within partner auth pane.

* Removes unused method.

* Updates screenshots.

* Updates dependencies file.

* Replaces Accompanist dependency by local replica.

* Regenerates API.

* Merge fixes.

* Adds comment.

* Adds tests.

* Regenerates screenshots and adds more.

* Regenerates API.

* Updates tests.

* Adds accompanist attribution.

* Moves exit logic to its own viewmodel.

* Makes callbacks non-optional.

* Uses financial connections modal for navigation.

* Adds optin.

* Updates screenshots.

* Make FC playground scrollable (#7927)

* Merges master.

* Make sure back button isn't shown on first screen (#7932)

* [FC-V3] Show networking Warmup pane in bottom sheet (#7923)

* [skip ci] Start PR

* Move login pane to bottom sheet.

* Adds tests.

* Regenerates screenshots.

* Removes loading state.

* Remove wrapping box.

* Revert.

* Fixes styling.

* Dismiss software keyboard on navigation (#7925)

* Dismiss software keyboard on navigation

* Add comment about using deprecated method

* Dismiss keyboard on forward navigation

* [FC-V3] Fixes pre-pane loading states (#7943)

* Adds action type.

* Keeps existing payload to prevent showing full-screen loading.

* Updates action.

* Updates screenshots.

* [FC-V3] Remove v2/v3 references (#7958)

* Removes v2 and v3 references.

* Removes v2 and v3 references.

* Regenerates screenshots.

* Fix lint

* Update screenshots following text padding change

* Use content padding in account picker screen (#7967)

* Make button height stay consistent during load (#7945)

* Move warning suppressions to baseline file (#7977)

* Fix missing elevation for logos on consent screen (#7930)

* Update connected_account_notice object for v3 (#7975)

* [skip ci] Start PR

* Adds connected account section to modals.

* Regenerates screenshots.

* Adds connected account tests.

* Ktlint fixes.

* Screenshots updated.

* Tweak keyboard in institution picker search (#7989)

* Tweak keyboard in institution picker search

- Show appropriate IME action
- Clear focus upon search

* Update detekt baseline

* Fix small-screen issues in Networking signup screen (#7994)

* Rename `Layout` to `LazyLayout`

* Add new eager `Layout`

* Use `Layout` in Link signup screen to allow moving focus

This didn't work before because the phone number text field wasn't visible on small devices.

* Remove animation delay

* Add missing v3 analytics events (#7996)

* [skip ci] Start PR

* Adds events.

* Adds tests.

* Uses space separator.

* Make event reporting non-blocking in FC (#8010)

* Make event reporting non-blocking in FC

* Remove irrelevant `suspend` modifiers

* Update tests

* [FC-V3] Adds test pill to toolbar when on testmode.  (#7960)

* Adds testmode field.

* Renames field.

* Screenshots updated.

* Updates baseline.

* Updates baseline.

* Fix detekt baseline issue

* Use hyphen in manual entry instructions (#8023)

* [V3] Fix Success animation transition (#8017)

* Removes optional rendering.

* Regenerates screenshots.

* use named parameter.

* [V3] Fix modal spaces (#8020)

* Fix modal spaces

* Updates screenshots.

* Fix clickable tests on Link screen.

* Centers disclaimer.

* Makes screenshots more v3.

* Don't count bottom sheets towards the back stack (#8022)

* Don't count bottom sheets towards the back stack

* Add Maestro test for returning user flow

The test checks the back button visibility

* Address code review feedback

Rename `collectCanGoBackAsState` to `collectCanShowBackIconAsState`, and rename `showBack` to `allowBackNavigation`. The latter is meant to convey that it’s more about business logic rather than back stack state.

* Add autofill for phone number and OTP fields (#7995)

* Extract autofill logic into modifier

* Add autofill for phone number and OTP fields

* Add test mode button

* Use banner instead

* Reduce diff

* [V3] Manual entry: Add autofill on testmode (#8027)

* Adds autofill on manual entry.

* Disable fields while loading.

* Auto submits form.

* Uses column to wrap composables.

* Regenerates screenshots.

* Fix disabled `Save to Link` button issue (#8025)

* Fix disabled `Save to Link` button issue

* Add tests

* Persist email in FC playground (#8034)

* Add gray placeholders for consent header logos (#8031)

* [skip ci] Start PR

* Adds grey loader.

* Simplifies logic.

* Removes load method.

* Regenerates API.

* Waits for all images to load.

* Simplifies code.

* Fix consent header for icons without background.

* Fixes wrong icon.

* Disable back navigation after `Not now` in Link warmup pane (#8033)

* Initial approach

* Revert "Initial approach"

This reverts commit e06d860.

* Use NavController-based approach

Changed `tryNavigateTo` from taking `popUpToCurrent: Boolean` to taking `popUpTo: PopUpToBehavior?)`. For situations where we want to pop two destinations (the bottom sheet and its referrer), we can now use `PopUpToBehavior.Route(String)`.

* Fix rebase issue

* Address code review feedback

Add comments for clarity

* Align exit modal confirm label with Web and iOS (#8038)

* Align exit modal confirm label with Web

* Update screenshots

* Use non-breaking spaces in `manually enter details` text (#8040)

* Add `TimeZoneRule` (#8045)

* [V3] Adds padding to inst picker search bar on scroll (#8036)

* [skip ci] Start PR

* Add 8dp padding underneath the institution picker search bar.

* Removes comment.

* Regenerates screenshots

* Regenerates screenshots

* Regenerates screenshots

* Regenerates screenshots

* Extracts to method.

* Inverts consent logo dots colors.

* Send correct pane in `onCloseClick` on two Link screens (#8055)

* Show manual entry CTA instead of close on unclassified errors if allowed (#8062)

* [skip ci] Start PR

* Adds allow manual entry check on error screen.

* Regenerates screenshots.

* Adds new screenshots.

* Expose underlying `StripeException` instead of `FinancialConnectionsError` (#8058)

* Expose underlying `StripeException` instead of `FinancialConnectionsError`

* Add UI test to cover close-with-error scenario

* [V3][Manual Entry] Extracts form fields from Mavericks state (#8060)

* WIP>

* Adds tests.

* Style fixes.

* Fixes preview.

* Fixes preview.

* Renames to state

* Updates state.

* Adds tests.

* PR feedback.

* Replace more en-dashes with hyphens (#8077)

* Update button arrangement in `ExitModal` (#8073)

* Update button arrangement in `ExitModal`

* Update screenshots

* Updates skip logic.

* [V3] customSuccessMessage on Success pane. (#8070)

* Resolves custom success message before landing on success pane.

* Adds test double.

* Regenerates API.

* Style fixes.

* Style fixes.

* [V3] logic bug bash udpates (Networking) (#8079)

* Hide not now button when auto navigating from signup pane.

* Stop calling networked accounts on verification pane.

* Updates tests.

* Adds submitted event on link account picker.

* Updates tests.

* [V3] Update Maestro tests (#8050)

* [skip ci] Start PR

* Update Testmode-Data-TestOauthInstitution.yaml

* Update Testmode-PaymentIntent-TestInstitution.yaml

* Update Testmode-PaymentIntent-TestInstitution-Networking.yaml

* Update Testmode-Token-ManualEntry.yaml

* Updates livemode tests.

* Test execution.

* Updates web tests.

* Updates maestro.

* Update web test.

* Updates manual entry test.

* Updates web test.

* Accounts auto select.

* Reset bitrise.

* Adds back not visible check.

* Updates test.

* Add drag handle to bottom sheets (#8043)

* Remove single `NoArgumentsDestination` subclass in `Destination` (#8099)

* Keyboard keeps showing up after browser (#8102)

* [skip ci] Start PR

* Remove focus after selecting institution.

* Move defaultBillingDetails and shippingDetails to PaymentMethodMetadata. (#8094)

* [Identity] Butter M2 phase 1 - Support microblink detector (#7883)

* Add ErrorReporter for sending error analytics (#8097)

* Create ErrorReporter and use it to send analytics on one error

* Remove unnecessary RestrictTo annotations

* Use errorReporter in more tests

* Add param name to make it more obvious what is being tested

* Fix testing issues with FakeErrorReporter

* Move FakeErrorReporter to payments-core-testing

* Move field extraction from StripeException into an extension function

* Move ErrorReporter extension to EventReporter file + fix imports

* Update ErrorReporter API

* fix test (#8100)

* remove io dispatcher

* inject global context

---------

Co-authored-by: Samer Alabi <141707240+samer-stripe@users.noreply.github.com>
Co-authored-by: Jay Newstrom <jaynewstrom@stripe.com>
Co-authored-by: Till Hellmund <tillh@stripe.com>
Co-authored-by: runner <runner@Mac-1706055070232.local>
Co-authored-by: Chen Cen <79880926+ccen-stripe@users.noreply.github.com>
Co-authored-by: amk-stripe <160939932+amk-stripe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-cli Added to pull requests created with #work-cli for usage tracking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants