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

[Android] Update Account Picker Pane to V3 #7736

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Dec 21, 2023

Summary

Migrates account picker pane to v3.

(most of the file changes are screenshot test related updates)

Figma for reference

Motivation

📔  [Android] Update Account Picker Pane to V3
🌐  BANKCON-8367

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

image

Changelog

@carlosmuvi-stripe carlosmuvi-stripe added the work-cli Added to pull requests created with #work-cli for usage tracking. label Dec 21, 2023
@carlosmuvi-stripe carlosmuvi-stripe self-assigned this Dec 21, 2023
Comment on lines +55 to +58
when (attachPayment) {
is Loading,
is Uninitialized,
is Success -> FullScreenGenericLoading()
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 screen is now just a generic loading spinner with no text (or an error if something fails)

Comment on lines -84 to -97
onAsync(
AttachPaymentState::payload,
onFail = {
eventTracker.logError(
logger = logger,
pane = PANE,
extraMessage = "Error retrieving accounts to attach payment",
error = it
)
},
onSuccess = {
eventTracker.track(FinancialConnectionsAnalyticsEvent.PaneLoaded(PANE))
}
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no need to fetch account info since no text is rendered.

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review January 8, 2024 16:35
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners January 8, 2024 16:35
@carlosmuvi-stripe carlosmuvi-stripe requested review from awush-stripe and removed request for a team January 8, 2024 16:35
Comment on lines +121 to +122
payload()
?.takeIf { it.shouldSkipPane.not() }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this so that we don’t render a list with a single account for a split-second before continuing? If yes, can we move that logic entirely to the view model?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep! in some cases we auto select the accounts, and keep showing a loading until moving to the next pane.

Open to suggestions on how to move more to the viewmodel here! an option I've considered is keeping the Loading state, but we observe the Success state to run some logic.

Comment on lines +132 to +133
payload()
?.takeIf { it.shouldSkipPane.not() }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

}
}

@Suppress("MagicNumber")
Copy link
Collaborator

Choose a reason for hiding this comment

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

It’d be cool to pull out all dimension values into a proper theme object. Nothing for this pull request, but an improvement down the line.

}

enum class SelectionMode {
RADIO, CHECKBOXES
SINGLE, MULTIPLE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: The convention now is to use pascal case for enum cases, so Single and Multiple.

Comment on lines 47 to 48
private const val UNSELECTABLE_ALPHA = 0.5f
private const val SELECTABLE_ALPHA = 1f
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a couple alpha constants in the module. Should we align those, or at least bundle them in some place?

We could also consider using Compose’s ContentAlpha.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wasn't aware of ContentAlpha! Replaced it (validated by design)

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}
Spacer(modifier = Modifier.size(12.dp))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Can we achieve the same by removing the Spacer calls and using horizontalArrangement = Arrangement.spacedBy(12.dp)?

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 catch 549e008

@Composable
private fun readableListOfPermissions(permissionsReadable: List<Int>): String =
permissionsReadable
// TODO@carlosmuvi localize enumeration of permissions once more languages are supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a plan to eventually drive this from the backend?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes! we're pushing for it : )

.foldIndexed("") { index, current, arg ->
when {
index == 0 -> arg
permissionsReadable.lastIndex == index -> "$current and $arg"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Do we need to use an Oxford comma if it’s 3+ items?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we're mimicking what web does here, but we should in any case move this to backend and have proper unified logic there, I'll push for that!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Can you improve the legibility here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 - 9b66c20

@@ -83,7 +81,7 @@ internal fun AccountItem(
if (SDK_INT >= M) view.performHapticFeedback(CONTEXT_CLICK)
onAccountClicked(account)
}
.alpha(if (selectable) SELECTABLE_ALPHA else UNSELECTABLE_ALPHA)
.alpha(if (selectable) 1f else ContentAlpha.disabled)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: This could use ContentAlpha.high for the selectable case.

@carlosmuvi-stripe carlosmuvi-stripe merged commit 739dcca into carlosmuvi/i/v3-general-style-updates Jan 17, 2024
7 of 9 checks passed
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/BANKCON-8367/android-update-account-picker-pane-to-v3 branch January 17, 2024 19:03
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