[MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard #10129

tianzhao-stripe · 2025-02-11T00:04:10Z

Summary

Adding Set As Default Payment Method Element

Looking for feedback on my approach

Motivation

https://jira.corp.stripe.com/browse/MOBILESDK-2683

Testing

N.A.

Screenshots

N.A.

Changelog

N.A.

github-actions · 2025-02-11T00:10:15Z

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 │     4 MiB │     4 MiB │ +1.6 KiB │  8.8 MiB │  8.8 MiB │ +1.1 KiB 
     arsc │   2.3 MiB │   2.3 MiB │   +696 B │  2.3 MiB │  2.3 MiB │   +696 B 
 manifest │   5.1 KiB │   5.1 KiB │      0 B │ 25.7 KiB │ 25.7 KiB │      0 B 
      res │ 910.3 KiB │ 910.3 KiB │      0 B │  1.4 MiB │  1.4 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 │ -1.4 KiB │  1.6 MiB │  1.6 MiB │ -1.4 KiB 
    other │   1.4 MiB │   1.4 MiB │     +1 B │  1.6 MiB │  1.6 MiB │      0 B 
──────────┼───────────┼───────────┼──────────┼──────────┼──────────┼──────────
    total │  12.7 MiB │  12.7 MiB │   +881 B │ 21.7 MiB │ 21.7 MiB │   +362 B 

 DEX     │ old   │ new   │ diff             
─────────┼───────┼───────┼──────────────────
   files │     1 │     1 │  0               
 strings │ 41235 │ 41234 │ -1 (+26 -27)     
   types │ 14272 │ 14268 │ -4 (+19 -23)     
 classes │ 11923 │ 11921 │ -2 (+2 -4)       
 methods │ 60554 │ 60552 │ -2 (+1543 -1545) 
  fields │ 40760 │ 40765 │ +5 (+771 -766)   

 ARSC    │ old  │ new  │ diff       
─────────┼──────┼──────┼────────────
 configs │  243 │  243 │  0         
 entries │ 6248 │ 6249 │ +1 (+1 -0)

APK

     compressed      │     uncompressed     │                                           
──────────┬──────────┼───────────┬──────────┤                                           
 size     │ diff     │ size      │ diff     │ path                                      
──────────┼──────────┼───────────┼──────────┼───────────────────────────────────────────
    4 MiB │ +1.6 KiB │   8.8 MiB │ +1.1 KiB │ ∆ classes.dex                             
  6.4 KiB │ -1.4 KiB │   6.2 KiB │ -1.4 KiB │ ∆ assets/dexopt/baseline.prof             
  2.3 MiB │   +696 B │   2.3 MiB │   +696 B │ ∆ resources.arsc                          
 53.7 KiB │     -6 B │   119 KiB │      0 B │ ∆ META-INF/CERT.SF                        
  1,009 B │     -5 B │     877 B │     -5 B │ ∆ assets/dexopt/baseline.profm            
  1.2 KiB │     +3 B │   1.2 KiB │      0 B │ ∆ META-INF/CERT.RSA                       
 50.4 KiB │     +3 B │ 118.9 KiB │      0 B │ ∆ META-INF/MANIFEST.MF                    
    272 B │     +1 B │     120 B │      0 B │ ∆ META-INF/version-control-info.textproto 
──────────┼──────────┼───────────┼──────────┼───────────────────────────────────────────
  6.4 MiB │   +881 B │  11.3 MiB │   +362 B │ (total)

DEX

STRINGS:

   old   │ new   │ diff         
  ───────┼───────┼──────────────
   41235 │ 41234 │ -1 (+26 -27) 
  
  + , defaultPaymentMethodId=
  + CustomerMetadata(isPaymentMethodSetAsDefaultEnabled=
  + LS8/a2;
  + Lf6/c;
  + SET_AS_DEFAULT_PAYMENT_METHOD_TEST_TAG
  + SetAsDefaultPaymentMethodElement(initialValue=false, shouldShowElementFlow=
  + [LS8/O1;
  + [LS8/U1;
  + [LS8/Y1;
  + [LS8/a2;
  + [Lg7/E;
  + [Lg7/s;
  + [Lg8/M;
  + [Lg8/x;
  + [Lg8/y;
  + [Lg8/z;
  + [Lh6/a;
  + [Lh6/c;
  + [Ll3/d;
  + [Ln7/c;
  + [Lq6/k;
  + [Lu8/m;
  + [Lv8/c;
  + set_as_default_payment_method
  + shouldShowElementFlow
  + ~~R8{"backend":"dex","compilation-mode":"release","has-checksums":false,"min-api":21,"pg-map-id":"26581ad","r8-mode":"full","version":"8.7.14"}
  
  - CustomerMetadata(hasCustomerConfiguration=
  - Lg7/Q;
  - Lg8/V;
  - Lg8/W;
  - Lg8/X;
  - [LS8/N1;
  - [LS8/T1;
  - [LS8/X1;
  - [LS8/Z1;
  - [Lg7/F;
  - [Lg7/t;
  - [Lg8/C;
  - [Lg8/E;
  - [Lg8/G;
  - [Lg8/P;
  - [Lg8/d;
  - [Lg8/e;
  - [Lh6/b;
  - [Lh6/d;
  - [Ll3/f;
  - [Ln7/e;
  - [Lq6/m;
  - [Lu8/n;
  - [Lv8/d;
  - customerMetadata
  - defaultPaymentMethodState
  - ~~R8{"backend":"dex","compilation-mode":"release","has-checksums":false,"min-api":21,"pg-map-id":"80f189e","r8-mode":"full","version":"8.7.14"}
  

TYPES:

   old   │ new   │ diff         
  ───────┼───────┼──────────────
   14272 │ 14268 │ -4 (+19 -23) 
  
  + LS8/a2;
  + Lf6/c;
  + [LS8/O1;
  + [LS8/U1;
  + [LS8/Y1;
  + [LS8/a2;
  + [Lg7/E;
  + [Lg7/s;
  + [Lg8/M;
  + [Lg8/x;
  + [Lg8/y;
  + [Lg8/z;
  + [Lh6/a;
  + [Lh6/c;
  + [Ll3/d;
  + [Ln7/c;
  + [Lq6/k;
  + [Lu8/m;
  + [Lv8/c;
  
  - Lg7/Q;
  - Lg8/V;
  - Lg8/W;
  - Lg8/X;
  - [LS8/N1;
  - [LS8/T1;
  - [LS8/X1;
  - [LS8/Z1;
  - [Lg7/F;
  - [Lg7/t;
  - [Lg8/C;
  - [Lg8/E;
  - [Lg8/G;
  - [Lg8/P;
  - [Lg8/d;
  - [Lg8/e;
  - [Lh6/b;
  - [Lh6/d;
  - [Ll3/f;
  - [Ln7/e;
  - [Lq6/m;
  - [Lu8/n;
  - [Lv8/d;
  

METHODS:

   old   │ new   │ diff             
  ───────┼───────┼──────────────────
   60554 │ 60552 │ -2 (+1543 -1545) 
  
  + A5.H0 <init>(Locale, a, d, O, I, j)
  + A5.c0 <init>(j, M, d, a)
  + B6.h0 <init>(a1, b, a, o, P0, W)
  + C.D0 <init>(e, g, Application, P, g0, e)
  + C5.c <init>(e, c, U, Q, c, s, c, p, int)
  + D1.f j(C, t0)
  + D5.e <init>(b, X, b, E, w, P0, d)
  + E7.f <init>(h, List, a, c, a, int)
  + G5.v <init>(n1, h, a, boolean, int, a, c, a, int)
  + I9.b p(a, B1, C, p, int)
  + K0.d0 B0(q, Q1)
  + K5.d <init>(a, N0, String, boolean)
  + L6.h <init>(k, O1, int, int, boolean, b, int)
  + M6.m <init>(l3, l0, boolean, boolean, List, c, String, i0, a, List, List, j, boolean, b, r, K0, y, e, boolean, a)
  + M6.t a(W0)
  + M6.u c(O1) → h
  + M6.u f(O1, e) → a
  + M6.u i(m, O1, x) → ArrayList
  + N6.b c(O1) → h
  + N6.b f(O1, e) → a
  + N6.b i(m, O1, x) → ArrayList
  + P5.s <init>(c, l, z, a, I0, d)
  + S6.b A(g, int, a, Object)
  + S6.b B(long)
  + S6.b C(g, int) → b
  + S6.b D(U, int) → d
  + S6.b E(char)
  + S6.b F(g, int, a, Object)
  + S6.b G(String)
  + S6.b H(boolean, a, p, int)
  + S6.b I(boolean, i, p, p, int)
  + S6.b J(int, int, p, p, boolean, boolean)
  + S6.b K(String, e, K, e, e, e, e, boolean, boolean, boolean, k, W, d3, p, int, int)
  + S6.b L(B, c, p, boolean, boolean, K, e, e, e, e, boolean, K, e0, d0, boolean, int, int, k, P, d3, p, int, int)
  + S6.b M(long, K, Float, e, p, int, int)
  + S6.b N(Context) → d
  + S6.b O(String, p, boolean, p, int, int)
  + S6.b P(j, g, a, p, int)
  + S6.b Q(int, p, String, p)
  + S6.b R(e, c, p, int)
  + S6.b S(int, p)
  + S6.b T(boolean, c, p, int)
  + S6.b U(boolean, D0, Set, M, p, int, int, p, int)
  + S6.b V(o, e, p, a, a, p, int)
  + S6.b W(int, int) → long
  + S6.b X(int, int)
  + S6.b Y(g, a, p, int)
  + S6.b Z(int, int, int)
  + S6.b a0(long)
  + S6.b b(g)
  + S6.b b0(int, long) → long
  + S6.b c(g) → b
  + S6.b c0(Throwable) → g
  + S6.b d0(int, int, String) → String
  + S6.b e(g) → boolean
  + S6.b e0(g, int)
  + S6.b f(U, int, char)
  + S6.b f0(Object)
  + S6.b g0(s0, e, String, p0, c) → n0
  + S6.b h(U, int, byte)
  + S6.b h0(int, int, TextPaint, CharSequence) → Rect
  + S6.b i()
  + S6.b i0(double) → long
  + S6.b j(g, int, String)
  + S6.b j0() → boolean
  + S6.b k(g, int, float)
  + S6.b k0(double) → long
  + S6.b l(g, int)
  + S6.b l0(int) → long
  + S6.b m(double)
  + S6.b m0(long) → boolean
  + S6.b n(short)
  + S6.b n0(D0, A, j) → r
  + S6.b o(g, int, double)
  + S6.b o0(Context, String) → String
  + S6.b p(g, int, long)
  + S6.b p0(LinkedHashMap, c) → ArrayLi
...✂

ARSC

ENTRIES:

   old  │ new  │ diff       
  ──────┼──────┼────────────
   6248 │ 6249 │ +1 (+1 -0) 
  + string/stripe_set_as_default_payment_method

samer-stripe

I think this approach is sound but I would be curious what the rest of the team thinks of the tradeoffs. It might make more sense for logic maintainability if these two were combined into one element.

samer-stripe · 2025-02-11T00:16:07Z

...i-core/src/main/java/com/stripe/android/ui/core/elements/SetAsDefaultPaymentMethodElement.kt

+data class SetAsDefaultPaymentMethodElement(
+    val initialValue: Boolean,
+    val shouldShowElementFlow: StateFlow<Boolean>
+): FormElement {


This probably doesn't make sense as a data class since we are passing a StateFlow

I assume that it should be a normal class instead then

Yup! That way we don't generate the additional equals and copy methods.

...ents-ui-core/src/main/java/com/stripe/android/ui/core/elements/SavePaymentMethodElementUI.kt

samer-stripe · 2025-02-11T00:31:34Z

.../src/main/java/com/stripe/android/lpmfoundations/paymentmethod/definitions/CardDefinition.kt

            }

+


I think the approach is sound. It has its upsides and tradeoffs.

Don't need to create a combined element for managing the Save for future and Set as default checkboxes.

Spacing remains controlled by FormUI.

Tradeoffs here are that the element is not a serializable element and is coupled to the state of whatever decides to show the element (in this case SaveForFutureElement). It would make it harder to parcelize form elements if we need to though I imagine we would be serializing the values rather the elements themselves.

I agree that this approach lgtm! I do think we only care about serializing the values -- when do we parcelize the form elements themselves @samer-stripe?

tianzhao-stripe added 2 commits February 10, 2025 12:47

3084 upstream

914c102

test

4217507

tianzhao-stripe requested a review from jaynewstrom-stripe February 11, 2025 00:26

tianzhao-stripe changed the title ~~[MOBILESDK 2683] Set As Default Payment Method Element in AddCard~~ [MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard Feb 11, 2025

samer-stripe reviewed Feb 11, 2025

View reviewed changes

remove unused

025bd80

tianzhao-stripe closed this Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard #10129

[MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard #10129

tianzhao-stripe commented Feb 11, 2025 •

edited

Loading

github-actions bot commented Feb 11, 2025 •

edited

Loading

samer-stripe left a comment

samer-stripe Feb 11, 2025

tianzhao-stripe Feb 11, 2025

samer-stripe Feb 11, 2025

samer-stripe Feb 11, 2025

amk-stripe Feb 11, 2025

[MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard #10129

[MOBILESDK 2683] Request for feedback on approach: Set As Default Payment Method Element in AddCard #10129

Conversation

tianzhao-stripe commented Feb 11, 2025 • edited Loading

Summary

Motivation

Testing

Screenshots

Changelog

github-actions bot commented Feb 11, 2025 • edited Loading

samer-stripe left a comment

Choose a reason for hiding this comment

samer-stripe Feb 11, 2025

Choose a reason for hiding this comment

tianzhao-stripe Feb 11, 2025

Choose a reason for hiding this comment

samer-stripe Feb 11, 2025

Choose a reason for hiding this comment

samer-stripe Feb 11, 2025

Choose a reason for hiding this comment

amk-stripe Feb 11, 2025

Choose a reason for hiding this comment

tianzhao-stripe commented Feb 11, 2025 •

edited

Loading

github-actions bot commented Feb 11, 2025 •

edited

Loading