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

Card-scanning in PaymentSheet #4804

Merged
merged 153 commits into from
Apr 25, 2022
Merged

Card-scanning in PaymentSheet #4804

merged 153 commits into from
Apr 25, 2022

Conversation

epan-stripe
Copy link
Contributor

@epan-stripe epan-stripe commented Mar 31, 2022

Summary

Add the ability for customers to enter credit card details into PaymentSheet by pointing their device’s camera at their physical card instead of manually typing them in.

Motivation

Card-scanning is one of the top requests from developers and already exists in iOS, but has not yet been implemented in the Android SDK.

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Button Screen
Screen Shot 2022-03-31 at 1 29 42 PM Screen Shot 2022-03-31 at 1 30 05 PM

…rm field values of it's elements. This simplifies the form quite a bit!
…that we don't have to iterate through section element fields to get the form field values.
val cardScanLauncher =
rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) {
it.data?.let {
controller.cardDetailsElement.controller.numberElement.controller.onCardScanResult(
Copy link
Contributor

Choose a reason for hiding this comment

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

This line feels a little weird. Maybe this logic should be moved to your controller class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how I would move this logic - the rememberLauncherForActivityResult method is a Composable method, and I'm calling the onCardScanResult function in the controller from the callback

Copy link
Contributor

Choose a reason for hiding this comment

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

oh fair enough. You can put a @composable method in the controller if you want. The bit that I'm feeling weird about is the controller.cardDetailsElement.controller.numberElement.controller.onCardScanResult but I think that's required weirdness right? We couldn't hide that away in the controller? I like not having a ton of logic in the compose views and making them as dumb as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative would be to add a function in the CardDetailsSectionElementController that then calls the cardDetailsElement.controller.numberElement.controller.onCardScanResult() function

@michelleb-stripe
Copy link
Contributor

Can you do this with a quick test when "Don't keep activities is turned on".

@michelleb-stripe
Copy link
Contributor

Can we use the built in android camera resource (android.R.drawable.ic_menu_camera)?

@stripe stripe deleted a comment from michelleb-stripe Apr 19, 2022
@stripe stripe deleted a comment from michelleb-stripe Apr 19, 2022
@@ -5,4 +5,7 @@
-->
<!-- Card details section title for card form entry -->
<string name="card_information" tools:ignore="ExtraTranslation">Card information</string>

<!-- Scan card button -->
<string name="scan_card" tools:ignore="ExtraTranslation">Scan card</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

I am looking up the translated string in localize and adding it.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, got it in localize, and it will update the next time we run the script. Best to wait until this other PR is merged to run the localize script again. #4892

@epan-stripe epan-stripe merged commit 4737d07 into master Apr 25, 2022
@epan-stripe epan-stripe deleted the elena/cardscanning branch April 25, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants