Update RowElementUI and screenshot tests

stripe · May 27, 2022 · a31aea7 · a31aea7
1 parent 7d844aa
commit a31aea7
Show file tree

Hide file tree

Showing 14 changed files with 45 additions and 61 deletions.
diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/CardNumberController.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/CardNumberController.kt
@@ -65,14 +65,14 @@ internal class CardNumberController constructor(
     override val trailingIcon: Flow<TextFieldIcon?> = _fieldValue.map {
         val cardBrands = CardBrand.getCardBrands(it)
         if (accountRangeService.accountRange != null) {
-            TextFieldIcon.Trailing(accountRangeService.accountRange!!.brand.icon, isIcon = false)
+            TextFieldIcon.Trailing(accountRangeService.accountRange!!.brand.icon, isTintable = false)
         } else {
             val staticIcons = cardBrands.map { cardBrand ->
-                TextFieldIcon.Trailing(cardBrand.icon, isIcon = false)
+                TextFieldIcon.Trailing(cardBrand.icon, isTintable = false)
             }.filterIndexed { index, _ -> index < 3 }
 
             val animatedIcons = cardBrands.map { cardBrand ->
-                TextFieldIcon.Trailing(cardBrand.icon, isIcon = false)
+                TextFieldIcon.Trailing(cardBrand.icon, isTintable = false)
             }.filterIndexed { index, _ -> index > 2 }
 
             TextFieldIcon.MultiTrailing(

diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/CvcController.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/CvcController.kt
@@ -68,7 +68,7 @@ internal class CvcController constructor(
         }
 
     override val trailingIcon: Flow<TextFieldIcon?> = cardBrandFlow.map {
-        TextFieldIcon.Trailing(it.cvcIcon, isIcon = false)
+        TextFieldIcon.Trailing(it.cvcIcon, isTintable = false)
     }
 
     override val loading: Flow<Boolean> = MutableStateFlow(false)

diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/IbanConfig.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/IbanConfig.kt
@@ -32,7 +32,7 @@ class IbanConfig : TextFieldConfig {
     override val trailingIcon: MutableStateFlow<TextFieldIcon?> = MutableStateFlow(
         TextFieldIcon.Trailing(
             R.drawable.stripe_ic_bank_generic,
-            isIcon = true
+            isTintable = true
         )
     )
     override val loading: StateFlow<Boolean> = MutableStateFlow(false)

diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/RowElementUI.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/RowElementUI.kt
@@ -1,15 +1,18 @@
 package com.stripe.android.ui.core.elements
 
+import android.content.res.Resources
+import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.width
 import androidx.compose.material.Divider
 import androidx.compose.material.MaterialTheme
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.layout.onSizeChanged
 import androidx.compose.ui.unit.dp
-import androidx.constraintlayout.compose.ConstraintLayout
-import androidx.constraintlayout.compose.Dimension
 import com.stripe.android.ui.core.paymentsColors
 import com.stripe.android.ui.core.paymentsShapes
 
@@ -21,54 +24,30 @@ internal fun RowElementUI(
     lastTextFieldIdentifier: IdentifierSpec?
 ) {
     val fields = controller.fields
-
     val numVisibleFields = fields.filter { !hiddenIdentifiers.contains(it.identifier) }.size
-
+    val dividerHeight = remember { mutableStateOf(0.dp) }
     // Only draw the row if the items in the row are not hidden, otherwise the entire
     // section will fail to draw
     if (fields.map { it.identifier }.any { !hiddenIdentifiers.contains(it) }) {
-        // An attempt was made to do this with a row, and a vertical divider created with a box.
-        // The row had a height of IntrinsicSize.Min, and the box/vertical divider filled the height
-        // when adding in the trailing icon this broke and caused the overall height of the row to
-        // increase.  By using the constraint layout the vertical divider does not negatively effect
-        // the size of the row.
-        ConstraintLayout {
-            // Create references for the composables to constrain
-            val fieldRefs = fields.map { createRef() }
-            val dividerRefs = fields.map { createRef() }
-
+        Row(modifier = Modifier.fillMaxWidth()) {
             fields.forEachIndexed { index, field ->
                 SectionFieldElementUI(
                     enabled,
                     field,
                     hiddenIdentifiers = hiddenIdentifiers,
                     lastTextFieldIdentifier = lastTextFieldIdentifier,
                     modifier = Modifier
-                        .constrainAs(fieldRefs[index]) {
-                            if (index == 0) {
-                                start.linkTo(parent.start)
-                            } else {
-                                start.linkTo(dividerRefs[index - 1].end)
-                            }
-                            top.linkTo(parent.top)
+                        .weight(1.0f / numVisibleFields.toFloat())
+                        .onSizeChanged {
+                            dividerHeight.value =
+                                (it.height / Resources.getSystem().displayMetrics.density).dp
                         }
-                        .fillMaxWidth(
-                            (1f / numVisibleFields.toFloat())
-                        )
                 )
 
-                if (!hiddenIdentifiers.contains(field.identifier) && index != fields.lastIndex) {
+                if (index != fields.lastIndex) {
                     Divider(
                         modifier = Modifier
-                            .constrainAs(dividerRefs[index]) {
-                                start.linkTo(fieldRefs[index].end)
-                                top.linkTo(parent.top)
-                                bottom.linkTo(parent.bottom)
-                                height = (Dimension.fillToConstraints)
-                            }
-                            .padding(
-                                horizontal = MaterialTheme.paymentsShapes.borderStrokeWidth.dp
-                            )
+                            .height(dividerHeight.value)
                             .width(MaterialTheme.paymentsShapes.borderStrokeWidth.dp),
                         color = MaterialTheme.paymentsColors.componentDivider
                     )

diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/TextFieldController.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/TextFieldController.kt
@@ -44,7 +44,7 @@ sealed class TextFieldIcon {
         val contentDescription: Int? = null,
 
         /** If it is an icon that should be tinted to match the text the value should be true */
-        val isIcon: Boolean
+        val isTintable: Boolean
     ) : TextFieldIcon()
 
     @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP_PREFIX)

diff --git a/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/TextFieldUI.kt b/payments-ui-core/src/main/java/com/stripe/android/ui/core/elements/TextFieldUI.kt
@@ -168,16 +168,18 @@ fun TextField(
         },
         trailingIcon = trailingIcon?.let {
             {
-                when (it) {
-                    is TextFieldIcon.Trailing -> {
-                        TrailingIcon(it, loading)
-                    }
-                    is TextFieldIcon.MultiTrailing -> {
-                        Row(modifier = Modifier.padding(end = 16.dp)) {
-                            it.staticIcons.forEach {
-                                TrailingIcon(it, loading)
+                Row {
+                    when (it) {
+                        is TextFieldIcon.Trailing -> {
+                            TrailingIcon(it, loading)
+                        }
+                        is TextFieldIcon.MultiTrailing -> {
+                            Row(modifier = Modifier.padding(10.dp)) {
+                                it.staticIcons.forEach {
+                                    TrailingIcon(it, loading)
+                                }
+                                AnimatedIcons(icons = it.animatedIcons, loading = loading)
                             }
-                            AnimatedIcons(icons = it.animatedIcons, loading = loading)
                         }
                     }
                 }
@@ -251,7 +253,7 @@ internal fun TrailingIcon(
 ) {
     if (loading) {
         CircularProgressIndicator()
-    } else if (trailingIcon.isIcon) {
+    } else if (trailingIcon.isTintable) {
         Icon(
             painter = painterResource(id = trailingIcon.idRes),
             contentDescription = trailingIcon.contentDescription?.let {

diff --git a/...nts-ui-core/src/test/java/com/stripe/android/ui/core/elements/CardNumberControllerTest.kt b/...nts-ui-core/src/test/java/com/stripe/android/ui/core/elements/CardNumberControllerTest.kt
@@ -143,15 +143,15 @@ internal class CardNumberControllerTest {
             .isEqualTo(
                 TextFieldIcon.MultiTrailing(
                     staticIcons = listOf(
-                        TextFieldIcon.Trailing(CardBrand.Visa.icon, isIcon = false),
-                        TextFieldIcon.Trailing(CardBrand.MasterCard.icon, isIcon = false),
-                        TextFieldIcon.Trailing(CardBrand.AmericanExpress.icon, isIcon = false),
+                        TextFieldIcon.Trailing(CardBrand.Visa.icon, isTintable = false),
+                        TextFieldIcon.Trailing(CardBrand.MasterCard.icon, isTintable = false),
+                        TextFieldIcon.Trailing(CardBrand.AmericanExpress.icon, isTintable = false),
                     ),
                     animatedIcons = listOf(
-                        TextFieldIcon.Trailing(CardBrand.Discover.icon, isIcon = false),
-                        TextFieldIcon.Trailing(CardBrand.JCB.icon, isIcon = false),
-                        TextFieldIcon.Trailing(CardBrand.DinersClub.icon, isIcon = false),
-                        TextFieldIcon.Trailing(CardBrand.UnionPay.icon, isIcon = false),
+                        TextFieldIcon.Trailing(CardBrand.Discover.icon, isTintable = false),
+                        TextFieldIcon.Trailing(CardBrand.JCB.icon, isTintable = false),
+                        TextFieldIcon.Trailing(CardBrand.DinersClub.icon, isTintable = false),
+                        TextFieldIcon.Trailing(CardBrand.UnionPay.icon, isTintable = false),
                     )
                 )
             )
@@ -169,9 +169,12 @@ internal class CardNumberControllerTest {
         idleLooper()
         assertThat(trailingIcons[0])
             .isInstanceOf(TextFieldIcon.MultiTrailing::class.java)
-        assertThat(trailingIcons[1] as TextFieldIcon.Trailing)
+        assertThat(trailingIcons[1] as TextFieldIcon.MultiTrailing)
             .isEqualTo(
-                TextFieldIcon.Trailing(CardBrand.Visa.icon, isIcon = false)
+                TextFieldIcon.MultiTrailing(
+                    staticIcons = listOf(TextFieldIcon.Trailing(CardBrand.Visa.icon, isTintable = false)),
+                    animatedIcons = listOf()
+                )
             )
         assertThat(trailingIcons[2])
             .isInstanceOf(TextFieldIcon.MultiTrailing::class.java)

diff --git a/....android.screenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomer.png b/....android.screenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomer.png
diff --git a/...creenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerAppearance.png b/...creenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerAppearance.png
diff --git a/...roid.screenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerDark.png b/...roid.screenshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerDark.png
diff --git a/...nshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerDarkAppearance.png b/...nshot.TestPaymentSheetScreenshots_testPaymentSheetNewCustomerDarkAppearance.png
diff --git a/...hot.TestPaymentSheetScreenshots_testPaymentSheetPrimaryButtonAppearanceDark.png b/...hot.TestPaymentSheetScreenshots_testPaymentSheetPrimaryButtonAppearanceDark.png
diff --git a/...ot.TestPaymentSheetScreenshots_testPaymentSheetPrimaryButtonAppearanceLight.png b/...ot.TestPaymentSheetScreenshots_testPaymentSheetPrimaryButtonAppearanceLight.png
diff --git a/paymentsheet-example/src/androidTestDebug/java/com/stripe/android/README.md b/paymentsheet-example/src/androidTestDebug/java/com/stripe/android/README.md
@@ -5,7 +5,7 @@ To Run Screenshot tests, takes 30 seconds a test and is long running:
 ./gradlew executeScreenshotTests
 
 To Run Screenshot tests for a specific file (and test method):
-./gradlew executeScreenshotTests -Pandroid.testInstrumentationRunnerArguments.class=com.stripe.android.TestPaymentSheetScreenshots#testPaymentSheetReturningCustomerLight
+./gradlew executeScreenshotTests -Pandroid.testInstrumentationRunnerArguments.class=com.stripe.android.screenshot.TestPaymentSheetScreenshots#testPaymentSheetReturningCustomerLight
 
 After running you can check the report here: ./stripe-android/paymentsheet-example/build/reports/shot/debug/verification/index.html