From 641911e95ded902f11d0a5b6b3bec5cbe7648d73 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fernando=20Garc=C3=ADa=20Lid=C3=B3n?=
 <fernando.garcialidon@telefonica.com>
Date: Tue, 26 Sep 2023 14:06:16 +0200
Subject: [PATCH] Android-13783 Improve Callout buttons margin (#304)

* ANDROID-13783 Update. Improve margins for Callout component

* ANDROID-13783 Update. Make private the implementation of ListRowItem

* ANDROID-13783 Update. Align XML Catalog option names with the Compose Catalog ones.

* ANDROID-13783 Update. Retro-compatibility for Button padding
---
 .../components/CalloutsCatalogFragment.kt     | 16 ++---
 .../mistica/compose/button/Button.kt          |  5 +-
 .../mistica/compose/button/ButtonSizes.kt     |  8 +--
 .../mistica/compose/callout/Callout.kt        | 59 ++++++++++++-------
 .../mistica/compose/list/ListRowItem.kt       |  2 +-
 5 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/catalog/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/CalloutsCatalogFragment.kt b/catalog/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/CalloutsCatalogFragment.kt
index 9f49c5395..a02ee88e9 100644
--- a/catalog/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/CalloutsCatalogFragment.kt
+++ b/catalog/src/main/java/com/telefonica/mistica/catalog/ui/classic/components/CalloutsCatalogFragment.kt
@@ -38,7 +38,7 @@ class CalloutsCatalogFragment : Fragment() {
                     CalloutButtonsConfig.values().map { it.name },
                 )
             )
-            setText(CalloutButtonsConfig.BUTTONS_CONFIG_PRIMARY.name)
+            setText(CalloutButtonsConfig.PRIMARY.name)
         }
         view.findViewById<Button>(R.id.update_button).setOnClickListener {
             updateCalloutView(view)
@@ -87,12 +87,12 @@ class CalloutsCatalogFragment : Fragment() {
     }
 
     private enum class CalloutButtonsConfig(@CalloutView.ButtonsConfig val buttonsConfig: Int) {
-        BUTTONS_CONFIG_NONE(CalloutView.BUTTONS_CONFIG_NONE),
-        BUTTONS_CONFIG_PRIMARY(CalloutView.BUTTONS_CONFIG_PRIMARY),
-        BUTTONS_CONFIG_PRIMARY_LINK(CalloutView.BUTTONS_CONFIG_PRIMARY_LINK),
-        BUTTONS_CONFIG_PRIMARY_SECONDARY(CalloutView.BUTTONS_CONFIG_PRIMARY_SECONDARY),
-        BUTTONS_CONFIG_SECONDARY(CalloutView.BUTTONS_CONFIG_SECONDARY),
-        BUTTONS_CONFIG_SECONDARY_LINK(CalloutView.BUTTONS_CONFIG_SECONDARY_LINK),
-        BUTTONS_CONFIG_LINK(CalloutView.BUTTONS_CONFIG_LINK),
+        NONE(CalloutView.BUTTONS_CONFIG_NONE),
+        PRIMARY(CalloutView.BUTTONS_CONFIG_PRIMARY),
+        PRIMARY_AND_LINK(CalloutView.BUTTONS_CONFIG_PRIMARY_LINK),
+        PRIMARY_AND_SECONDARY(CalloutView.BUTTONS_CONFIG_PRIMARY_SECONDARY),
+        SECONDARY(CalloutView.BUTTONS_CONFIG_SECONDARY),
+        SECONDARY_AND_LINK(CalloutView.BUTTONS_CONFIG_SECONDARY_LINK),
+        LINK(CalloutView.BUTTONS_CONFIG_LINK),
     }
 }
\ No newline at end of file
diff --git a/library/src/main/java/com/telefonica/mistica/compose/button/Button.kt b/library/src/main/java/com/telefonica/mistica/compose/button/Button.kt
index fe6491f7c..1224aed44 100644
--- a/library/src/main/java/com/telefonica/mistica/compose/button/Button.kt
+++ b/library/src/main/java/com/telefonica/mistica/compose/button/Button.kt
@@ -50,7 +50,7 @@ import com.telefonica.mistica.compose.theme.brand.MovistarBrand
 
 private val iconSpacing = 10.dp
 private val easing = CubicBezierEasing(0.77f, 0f, 0.175f, 1f)
-private const val CHEVRON_ASPECT_RATIO = 8f/20f
+private const val CHEVRON_ASPECT_RATIO = 8f / 20f
 
 @Composable
 fun Button(
@@ -62,6 +62,7 @@ fun Button(
     enabled: Boolean = true,
     @DrawableRes icon: Int? = null,
     withChevron: Boolean = false,
+    invalidatePaddings: Boolean = false,
     onClickListener: () -> Unit,
 ) {
 
@@ -84,7 +85,7 @@ fun Button(
                 }
                 .height(size.height)
                 .applyWidth(originalWidth),
-            contentPadding = PaddingValues(horizontal = size.contentPadding, vertical = 0.dp),
+            contentPadding = PaddingValues(horizontal = if (invalidatePaddings) 0.dp else size.horizontalPadding, vertical = 0.dp),
             onClick = onClickListener,
             enabled = enabled,
             colors = style.buttonColors,
diff --git a/library/src/main/java/com/telefonica/mistica/compose/button/ButtonSizes.kt b/library/src/main/java/com/telefonica/mistica/compose/button/ButtonSizes.kt
index 70f582827..0c8f78a1b 100644
--- a/library/src/main/java/com/telefonica/mistica/compose/button/ButtonSizes.kt
+++ b/library/src/main/java/com/telefonica/mistica/compose/button/ButtonSizes.kt
@@ -6,14 +6,14 @@ import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
 import com.telefonica.mistica.compose.theme.MisticaTheme
 
-internal class ButtonSizeConfig(
+internal data class ButtonSizeConfig(
     val textStyle: TextStyle,
     val height: Dp,
     val minWidth: Dp,
     val iconSize: Dp,
     val progressBarSize: Dp,
     val progressBarStroke: Dp,
-    val contentPadding: Dp,
+    val horizontalPadding: Dp,
 )
 
 @Composable
@@ -38,7 +38,7 @@ private fun getDefaultButtonSizeConfig() = ButtonSizeConfig(
     iconSize = 24.dp,
     progressBarSize = 20.dp,
     progressBarStroke = 2.dp,
-    contentPadding = 16.dp
+    horizontalPadding = 16.dp
 )
 
 @Composable
@@ -49,5 +49,5 @@ private fun getSmallButtonSizeConfig() = ButtonSizeConfig(
     iconSize = 16.dp,
     progressBarSize = 16.dp,
     progressBarStroke = 1.dp,
-    contentPadding = 12.dp
+    horizontalPadding = 12.dp
 )
\ No newline at end of file
diff --git a/library/src/main/java/com/telefonica/mistica/compose/callout/Callout.kt b/library/src/main/java/com/telefonica/mistica/compose/callout/Callout.kt
index fe4009278..161369a61 100644
--- a/library/src/main/java/com/telefonica/mistica/compose/callout/Callout.kt
+++ b/library/src/main/java/com/telefonica/mistica/compose/callout/Callout.kt
@@ -17,6 +17,7 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.graphics.ColorFilter
 import androidx.compose.ui.res.painterResource
+import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
 import com.telefonica.mistica.R
 import com.telefonica.mistica.compose.button.Button
@@ -88,27 +89,40 @@ fun Callout(
                         .padding(top = 16.dp),
                 ) {
                     when (buttonConfig) {
-                        CalloutButtonConfig.NONE -> { /* No buttons are shown */ }
+                        CalloutButtonConfig.NONE -> { /* No buttons are shown */
+                        }
                         CalloutButtonConfig.PRIMARY -> {
-                            PrimaryButton(primaryButtonText, onPrimaryButtonClick)
+                            PrimaryButton(text = primaryButtonText, onClick = onPrimaryButtonClick)
                         }
                         CalloutButtonConfig.PRIMARY_AND_LINK -> {
-                            PrimaryButton(primaryButtonText, onPrimaryButtonClick)
-                            LinkButton(linkText, onLinkClicked)
+                            PrimaryButton(
+                                text = primaryButtonText,
+                                modifier = Modifier.padding(end = 16.dp),
+                                onClick = onPrimaryButtonClick,
+                            )
+                            LinkButton(text = linkText, onClick = onLinkClicked)
                         }
                         CalloutButtonConfig.SECONDARY -> {
-                            SecondaryButton(secondaryButtonText, onSecondaryButtonClick)
+                            SecondaryButton(text = secondaryButtonText, onClick = onSecondaryButtonClick)
                         }
                         CalloutButtonConfig.PRIMARY_AND_SECONDARY -> {
-                            PrimaryButton(primaryButtonText, onPrimaryButtonClick)
-                            SecondaryButton(secondaryButtonText, onSecondaryButtonClick)
+                            PrimaryButton(
+                                text = primaryButtonText,
+                                modifier = Modifier.padding(end = 16.dp),
+                                onClick = onPrimaryButtonClick,
+                            )
+                            SecondaryButton(text = secondaryButtonText, onClick = onSecondaryButtonClick)
                         }
                         CalloutButtonConfig.SECONDARY_AND_LINK -> {
-                            SecondaryButton(secondaryButtonText, onSecondaryButtonClick)
-                            LinkButton(linkText, onLinkClicked)
+                            SecondaryButton(
+                                text = secondaryButtonText,
+                                modifier = Modifier.padding(end = 16.dp),
+                                onClick = onSecondaryButtonClick,
+                            )
+                            LinkButton(text = linkText, onClick = onLinkClicked)
                         }
                         CalloutButtonConfig.LINK -> {
-                            LinkButton(linkText, onLinkClicked)
+                            LinkButton(text = linkText, onClick = onLinkClicked)
                         }
                     }
                 }
@@ -117,10 +131,9 @@ fun Callout(
             if (dismissable) {
                 Image(
                     modifier = Modifier
-                        .padding(start = 16.dp)
                         .clickable { onDismiss?.invoke() },
                     painter = painterResource(id = R.drawable.icn_cross),
-                    contentDescription = null
+                    contentDescription = stringResource(id = R.string.close_button_content_description)
                 )
             }
         }
@@ -130,25 +143,28 @@ fun Callout(
 @Composable
 private fun PrimaryButton(
     text: String?,
-    onClick: (() -> Unit)? = null
+    modifier: Modifier = Modifier,
+    onClick: (() -> Unit)? = null,
 ) {
-    CalloutButton(text, onClick, ButtonStyle.PRIMARY_SMALL)
+    CalloutButton(text = text, onClick = onClick, style = ButtonStyle.PRIMARY_SMALL, modifier = modifier)
 }
 
 @Composable
 private fun SecondaryButton(
     text: String?,
-    onClick: (() -> Unit)? = null
+    modifier: Modifier = Modifier,
+    onClick: (() -> Unit)? = null,
 ) {
-    CalloutButton(text, onClick, ButtonStyle.SECONDARY_SMALL)
+    CalloutButton(text, onClick, ButtonStyle.SECONDARY_SMALL, modifier)
 }
 
 @Composable
 private fun LinkButton(
     text: String?,
-    onClick: (() -> Unit)? = null
+    modifier: Modifier = Modifier,
+    onClick: (() -> Unit)? = null,
 ) {
-    CalloutButton(text, onClick, ButtonStyle.LINK)
+    CalloutButton(text, onClick, ButtonStyle.LINK, modifier, true)
 }
 
 @Composable
@@ -156,13 +172,16 @@ private fun CalloutButton(
     text: String?,
     onClick: (() -> Unit)?,
     style: ButtonStyle,
+    modifier: Modifier = Modifier,
+    invalidatePaddings: Boolean = false,
 ) {
     text?.let {
         Button(
-            modifier = Modifier.padding(end = 16.dp),
+            modifier = modifier,
             text = text,
             buttonStyle = style,
-            onClickListener = { onClick?.invoke() }
+            onClickListener = { onClick?.invoke() },
+            invalidatePaddings = invalidatePaddings,
         )
     }
 }
diff --git a/library/src/main/java/com/telefonica/mistica/compose/list/ListRowItem.kt b/library/src/main/java/com/telefonica/mistica/compose/list/ListRowItem.kt
index 303e0d56f..a61b11d9e 100644
--- a/library/src/main/java/com/telefonica/mistica/compose/list/ListRowItem.kt
+++ b/library/src/main/java/com/telefonica/mistica/compose/list/ListRowItem.kt
@@ -109,7 +109,7 @@ fun ListRowItem(
 
 @ExperimentalMaterialApi
 @Composable
-fun ListRowItemImp(
+private fun ListRowItemImp(
     modifier: Modifier = Modifier,
     icon: @Composable (() -> Unit)? = null,
     title: String? = null,