-
Notifications
You must be signed in to change notification settings - Fork 656
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 PaymentMethodEmbeddedLayoutUI #9808
base: master
Are you sure you want to change the base?
Conversation
Diffuse output:
APK
|
5b0276d
to
040dad3
Compare
Will remove rowType from |
...src/main/java/com/stripe/android/paymentsheet/verticalmode/PaymentMethodVEmbeddedLayoutUI.kt
Outdated
Show resolved
Hide resolved
...src/main/java/com/stripe/android/paymentsheet/verticalmode/PaymentMethodVEmbeddedLayoutUI.kt
Outdated
Show resolved
Hide resolved
...src/main/java/com/stripe/android/paymentsheet/verticalmode/PaymentMethodVEmbeddedLayoutUI.kt
Outdated
Show resolved
Hide resolved
.../com/stripe/android/paymentsheet/verticalmode/PaymentMethodEmbeddedLayoutUIScreenshotTest.kt
Outdated
Show resolved
Hide resolved
.../test/java/com/stripe/android/paymentsheet/verticalmode/PaymentMethodEmbeddedLayoutUITest.kt
Outdated
Show resolved
Hide resolved
.../src/main/java/com/stripe/android/paymentsheet/verticalmode/PaymentMethodEmbeddedLayoutUI.kt
Outdated
Show resolved
Hide resolved
@@ -47,7 +47,7 @@ class PaymentMethodEmbeddedLayoutUIScreenshotTest { | |||
onSelectSavedPaymentMethod = {}, | |||
onManageOneSavedPaymentMethod = {}, |
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 was actually thinking a bit about the reverse. I thought all of the fields outside of row style were the same. And row style was the only "difference".
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.
How about both? Added TestPaymentMethodLayoutUi
helper function
if (rowStyle.hasSeparators()) { | ||
val color = Color(rowStyle.separatorColor()) | ||
val thickness = rowStyle.separatorThickness() | ||
val modifier = if (rowStyle is Embedded.RowStyle.FlatWithRadio) { |
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 feels like another one we should move into rowStyle
.
import org.robolectric.RobolectricTestRunner | ||
import org.robolectric.annotation.Config | ||
|
||
@RunWith(RobolectricTestRunner::class) |
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 shouldn't be necessary, and is deceptive! This isn't actually running any tests, but is a helper!
@RunWith(RobolectricTestRunner::class) | ||
@Config(sdk = [Build.VERSION_CODES.Q]) | ||
@OptIn(ExperimentalEmbeddedPaymentElementApi::class) | ||
class PaymentMethodLayoutUITestHelper( |
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 approach might not be ideal.
It would be easy to add another scenario, but miss adding it for both variants. Maybe we should have a shared test for both, driven by parameterized tests. That would ensure we couldn't miss a test case for the common case.
Summary
Add PaymentMethodEmbeddedLayoutUI
Motivation
MOBILESDK-2851
Testing
Screenshots
Changelog