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 CBC dropdown to UpdatePaymentMethodUI without full functionality #9686

Merged
merged 15 commits into from
Nov 22, 2024

Conversation

amk-stripe
Copy link
Collaborator

@amk-stripe amk-stripe commented Nov 21, 2024

Summary

Add CBC dropdown to UpdatePaymentMethodUI without full functionality

This just adds the UI, but doesn't actually respond to CBC updates. I'll follow up with actually responding to changes.

Motivation

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

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screen recording

cbc.dropdown.-.no.functionality.webm

Copy link
Contributor

github-actions bot commented Nov 21, 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.1 MiB │   4.1 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 KiB │     8 KiB │  0 B 
      res │ 301.8 KiB │ 301.8 KiB │  0 B │ 455.5 KiB │ 455.5 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.1 KiB │   7.1 KiB │  0 B │   6.9 KiB │   6.9 KiB │  0 B 
    other │  90.2 KiB │  90.2 KiB │ +6 B │ 170.3 KiB │ 170.3 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.6 MiB │   9.6 MiB │ +6 B │  21.5 MiB │  21.5 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 19966 │ 19966 │ 0 (+0 -0) 
   types │  6188 │  6188 │ 0 (+0 -0) 
 classes │  4979 │  4979 │ 0 (+0 -0) 
 methods │ 29759 │ 29759 │ 0 (+0 -0) 
  fields │ 17526 │ 17526 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3622 │ 3622 │  0
APK
    compressed    │   uncompressed   │                                           
──────────┬───────┼───────────┬──────┤                                           
 size     │ diff  │ size      │ diff │ path                                      
──────────┼───────┼───────────┼──────┼───────────────────────────────────────────
 28.5 KiB │ +11 B │  62.9 KiB │  0 B │ ∆ META-INF/CERT.SF                        
 25.3 KiB │  -4 B │  62.8 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
    270 B │  -2 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
  1.2 KiB │  +1 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
──────────┼───────┼───────────┼──────┼───────────────────────────────────────────
 55.2 KiB │  +6 B │ 127.1 KiB │  0 B │ (total)

import com.stripe.android.uicore.stripeColors

@Composable
internal fun CardBrandDropdown(
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 was moved from EditPaymentMethod. It previously took in an EditPaymentMethodViewState and an EditPaymentMethodViewActionHandler as params. I refactored it so that it didn't rely on those specific types, so that we could re-use it in UpdatePaymentMethodUI

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 was moved from EditPaymentMethodViewState. I didn't make any changes to the existing type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved these functions from EditPaymentMethod, so I could re-use them. They were previously extension functions on PaymentMethod which would throw an IllegalStateException if the PM was not a card. In UpdatePaymentMethodUI, we can guarantee that we have a card, so I made them extension functions on the card instead. That allows us to not bring that exception into UpdatePaymentMethodUI.

@@ -20,6 +21,7 @@ internal interface UpdatePaymentMethodInteractor {
val canRemove: Boolean
val displayableSavedPaymentMethod: DisplayableSavedPaymentMethod
val screenTitle: ResolvableString?
val cardBrandFilter: CardBrandFilter
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ideally, all of the card brand filtering logic would live in DisplayableSavedPaymentMethod, and then we wouldn't need to add it here. It's hard to refactor the existing screen for that change though, so I'm going to do this once we remove the feature flag and remove the old screen

@amk-stripe
Copy link
Collaborator Author

I had to make several refactors on this PR, reviewing commit by commit may make it easier to see what all of the individual changes were

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The "Card details cannot be changed" text is going to be updated btw. Just trying to keep this PR from getting too big

@amk-stripe amk-stripe marked this pull request as ready for review November 21, 2024 21:21
@amk-stripe amk-stripe requested review from a team as code owners November 21, 2024 21:21
@amk-stripe amk-stripe merged commit f6720ce into master Nov 22, 2024
13 checks passed
@amk-stripe amk-stripe deleted the make-cbc-dropdown-more-independent branch November 22, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants