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

[Connect SDK] Update API based on feedback #9480

Merged
merged 20 commits into from
Nov 6, 2024
Merged

Conversation

simond-stripe
Copy link
Contributor

@simond-stripe simond-stripe commented Oct 18, 2024

Summary

This PR updates the SDK's API based on API review. The review is still in progress so there may be some more changes in the future, but this should be significantly closer to what will be live in production. If you have any more feedback on the API, please let me know!

There are a lot of changes due to refactoring of the package name. I recommend reviewing commit-by-commit.

My next PR will make significant changes to the Example App UI to bring it to parity with the iOS example app. API and general feedback is appreciated, and I recommend holding off on UI-specific feedback until the next PR.

Changes:

  1. Rename the modules to connect and connect-example
  2. Update EmbeddedComponentManager to be a singleton
  3. Components are exposed as Fragments
  4. Incorporate PR feedback from [Connect SDK] Integrate with backend calls for client secret #9287

Motivation

Updating the API based on review

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

demo-1729293503.mp4

@simond-stripe simond-stripe requested review from a team as code owners October 18, 2024 23:06
Copy link
Contributor

Risky Change

This is considered a risky change because it adjusts the sample app build.gradle, please review carefully.
We've seen issues in the past which resulted in failed builds for merchants. Please make sure the build.gradle change is intended.

By adding the label accept-risky-change to this PR, I acknowledge that I'm changing an example app and have verified that the SDK remains in a shippable state.

@simond-stripe simond-stripe added the accept-risky-change accept-risky-change label Oct 21, 2024
Copy link
Contributor

github-actions bot commented Oct 21, 2024

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: V1, V2)
NEW: paymentsheet-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   3.8 MiB │   3.8 MiB │  0 B │   8.4 MiB │   8.4 MiB │  0 B 
     arsc │   2.3 MiB │   2.3 MiB │  0 B │   2.3 MiB │   2.3 MiB │  0 B 
 manifest │     5 KiB │     5 KiB │  0 B │  24.7 KiB │  24.7 KiB │  0 B 
      res │ 931.2 KiB │ 931.2 KiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
   native │   2.6 MiB │   2.6 MiB │  0 B │     6 MiB │     6 MiB │  0 B 
    asset │   1.6 MiB │   1.6 MiB │  0 B │   1.6 MiB │   1.6 MiB │  0 B 
    other │ 200.9 KiB │ 200.9 KiB │ +5 B │ 442.4 KiB │ 442.4 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  11.5 MiB │  11.5 MiB │ +5 B │  20.3 MiB │  20.3 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 39655 │ 39655 │ 0 (+0 -0) 
   types │ 13666 │ 13666 │ 0 (+0 -0) 
 classes │ 11364 │ 11364 │ 0 (+0 -0) 
 methods │ 58386 │ 58386 │ 0 (+0 -0) 
  fields │ 38724 │ 38724 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  242 │  242 │  0   
 entries │ 6247 │ 6247 │  0
APK
    compressed    │   uncompressed   │                        
───────────┬──────┼───────────┬──────┤                        
 size      │ diff │ size      │ diff │ path                   
───────────┼──────┼───────────┼──────┼────────────────────────
  53.5 KiB │ +3 B │ 118.7 KiB │  0 B │ ∆ META-INF/CERT.SF     
  50.3 KiB │ +2 B │ 118.6 KiB │  0 B │ ∆ META-INF/MANIFEST.MF 
───────────┼──────┼───────────┼──────┼────────────────────────
 103.8 KiB │ +5 B │ 237.3 KiB │  0 B │ (total)

Copy link

emerge-tools bot commented Oct 31, 2024

3 builds decreased size

Name Version Download Change Install Change Approval
Stripe Identity Example
com.stripe.android.identity.example.theme1
20.53.0-theme1 (20) 3.8 MB ⬇️ 2.5 kB (-0.07%) 8.9 MB ⬇️ 5.8 kB (-0.06%) N/A
Financial Connections Example
com.stripe.android.financialconnections.example
20.53.0 (205300) 4.2 MB ⬇️ 346 B 9.4 MB ⬇️ 116 B N/A
PaymentSheet Example
com.stripe.android.paymentsheet.example
20.53.0 (11) 7.1 MB ⬇️ 1.5 MB (-17.35%) 14.4 MB ⬇️ 1.9 MB (-11.38%) N/A

Stripe Identity Example 20.53.0-theme1 (20)
com.stripe.android.identity.example.theme1

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 5.8 kB (-0.06%)
Total download size change: ⬇️ 2.5 kB (-0.07%)

Largest size changes

Item Install Size Change Download Size Change
🗑 androidx.compose.ui.node.IntrinsicsPolicy ⬇️ -26.8 kB ⬇️ -12.9 kB
📝 androidx.compose.ui.node.DepthSortedSetsForDifferentPasses ⬆️ 26.2 kB ⬆️ 12.6 kB
🗑 com.stripe.android.camera.framework.AnalyzerLoop$startWorker$2 ⬇️ -4.4 kB ⬇️ -2.1 kB
📝 androidx.camera.camera2.internal.MeteringRepeatingSession$1 ⬆️ 3.7 kB ⬆️ 1.8 kB
🗑 androidx.constraintlayout.core.PriorityGoalRow$GoalVariableAccess... ⬇️ -3.4 kB ⬇️ -1.7 kB
View Treemap

Image of diff

Financial Connections Example 20.53.0 (205300)
com.stripe.android.financialconnections.example

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 116 B
Total download size change: ⬇️ 346 B

Largest size changes

Item Install Size Change Download Size Change
Other ⬇️ -116 B ⬇️ -346 B

PaymentSheet Example 20.53.0 (11)
com.stripe.android.paymentsheet.example

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 1.9 MB (-11.38%)
Total download size change: ⬇️ 1.5 MB (-17.35%)

Largest size changes

Item Install Size Change Download Size Change
🗑 ux_0_5_23_16.tflite ⬇️ -1.4 MB ⬇️ -1.3 MB
🗑 androidx.camera.camera2.internal ⬇️ -149.2 kB ⬇️ -68.1 kB
🗑 androidx.camera.core.impl.utils ⬇️ -126.8 kB ⬇️ -58.0 kB
🗑 androidx.camera.core.AspectRatio ⬇️ -79.7 kB ⬇️ -36.4 kB
📝 com.nimbusds.jose.shaded.asm.DefaultConverter ⬆️ 77.2 kB ⬆️ 35.3 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

@simond-stripe
Copy link
Contributor Author

There are a lot of miscellaneous changes here due to the refactor of the module name from stripe-connect -> connect.

I recommend focusing on this commit in particular: 8167a93. Otherwise, focus on EmbeddedComponentManager, PayoutsFragment, and AccountOnboardingFragment.

The example app is going to change quite a bit in a follow-up PR, so feel free to review but note that I'll likely seek to make changes to the example app in that PR rather than this one (barring any major concerns).

}

// private methods

@OptIn(PrivateBetaConnectSDK::class)

Choose a reason for hiding this comment

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

do you still need this if it's private?

Copy link

@seankenkeremath-stripe seankenkeremath-stripe left a comment

Choose a reason for hiding this comment

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

Looks good!

@simond-stripe simond-stripe requested review from awush-stripe and amk-stripe and removed request for awush-stripe November 5, 2024 19:21
@@ -305,6 +305,7 @@
| | | | +--- androidx.core:core:1.0.0 -> 1.13.0 (*)
| | | | \--- androidx.customview:customview:1.0.0 (*)
| | | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
| | | +--- androidx.fragment:fragment-compose:1.8.4 (c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know where this compose version is coming from? We are using older versions in the rest of the SDK (1.6.8 generally and I think 1.7.4 in paymentsheet-example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the Androidx fragment library. It's actually not related to the compose version used in the SDK, and matches the other versions of fragments in use elsewhere: https://developer.android.com/jetpack/androidx/releases/fragment#1.8.4

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah gotcha, thanks for the context!

@simond-stripe simond-stripe merged commit 3839c7f into master Nov 6, 2024
18 checks passed
@simond-stripe simond-stripe deleted the simond/api-update branch November 6, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accept-risky-change accept-risky-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants