-
Notifications
You must be signed in to change notification settings - Fork 650
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
base: master
Are you sure you want to change the base?
Conversation
Diffuse output:
APK
|
import com.stripe.android.uicore.stripeColors | ||
|
||
@Composable | ||
internal fun CardBrandDropdown( |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
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 |
There was a problem hiding this comment.
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
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
Screen recording
cbc.dropdown.-.no.functionality.webm