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

Add new CustomerSheet playground #8362

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

samer-stripe
Copy link
Collaborator

@samer-stripe samer-stripe commented Apr 27, 2024

Summary

Adds a new version of the CustomerSheet playground using the architecture from the PaymentSheet playground.

Motivation

Makes it easier to add additional playground settings and create effective end-to-end tests.

Video

Playground.mp4

Copy link
Contributor

github-actions bot commented Apr 27, 2024

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.3 MiB │   4.3 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │   8.1 KiB │   8.1 KiB │  0 B 
      res │ 301.5 KiB │ 301.5 KiB │  0 B │   455 KiB │   455 KiB │  0 B 
   native │   7.3 MiB │   7.3 MiB │  0 B │  18.4 MiB │  18.4 MiB │  0 B 
    asset │   1.5 MiB │   1.5 MiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
    other │    87 KiB │    87 KiB │ +6 B │ 161.5 KiB │ 161.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.2 MiB │  12.2 MiB │ +6 B │  25.8 MiB │  25.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21683 │ 21683 │ 0 (+0 -0) 
   types │  6869 │  6869 │ 0 (+0 -0) 
 classes │  5634 │  5634 │ 0 (+0 -0) 
 methods │ 31447 │ 31447 │ 0 (+0 -0) 
  fields │ 18320 │ 18320 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3404 │ 3404 │  0
APK
   compressed    │   uncompressed   │                        
──────────┬──────┼───────────┬──────┤                        
 size     │ diff │ size      │ diff │ path                   
──────────┼──────┼───────────┼──────┼────────────────────────
 29.1 KiB │ +2 B │    64 KiB │  0 B │ ∆ META-INF/CERT.SF     
  1.2 KiB │ +2 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA    
 25.9 KiB │ +2 B │  63.9 KiB │  0 B │ ∆ META-INF/MANIFEST.MF 
──────────┼──────┼───────────┼──────┼────────────────────────
 56.1 KiB │ +6 B │ 129.1 KiB │  0 B │ (total)

@samer-stripe samer-stripe marked this pull request as ready for review April 29, 2024 02:10
@samer-stripe samer-stripe requested review from a team as code owners April 29, 2024 02:10
@samer-stripe samer-stripe force-pushed the samer/customer-sheet-playground branch from c49a1e7 to 78dc8c4 Compare April 29, 2024 17:32
…ate `CountrySettingsDefinition` comment.
Copy link
Collaborator

@jaynewstrom-stripe jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

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

The old playground had a button to launch payment sheet as well (with the same config) which made it easy to test the defaults go saved between the surfaces.

@@ -101,7 +101,7 @@ class MainActivity : AppCompatActivity() {
MenuItem(
titleResId = R.string.customersheet_playground_title,
subtitleResId = R.string.playground_subtitle,
klass = CustomerSheetPlaygroundActivity::class.java,
klass = NewCustomerSheetPlaygroundActivity::class.java,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like you replaced this menu item, but didn't delete the old playground.

}
}

@OptIn(ExperimentalCustomerSheetApi::class)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should opt-in for the class instead.

}

@OptIn(ExperimentalCustomerSheetApi::class)
fun fetchOption(customerSheet: CustomerSheet) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I took some time to dig into this. I think we did it this way because of the PaymentOptionFactory, and the only way we can supply that to create them is via CustomerSheet.

In hindsight, we likely could have hacked around that technical limitation, and made a more compelling public API. Maybe we should reconsider before GA.

retrievePaymentOptionSelection should probably live on the adapter. But how do we allow merchants to transform it to a PaymentOption? As of now, we don't have a way to do that.

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.

None yet

2 participants