From d7b22047c63a1df3144a2e6206a9bb3051574c5c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 22 Aug 2023 10:17:55 +0200 Subject: [PATCH 1/4] Code clarity (no change effect): always use when with `ButtonStyle.Filled`, `ButtonStyle.Outlined` and `ButtonStyle.Text` cases. --- .../designsystem/theme/components/Button.kt | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt index 8c5d96c400..99d2a68036 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt @@ -136,29 +136,33 @@ internal fun ButtonInternal( val contentPadding = when (size) { ButtonSize.Medium -> { when (style) { + ButtonStyle.Filled, + ButtonStyle.Outlined -> PaddingValues(horizontal = 16.dp, vertical = 10.dp) ButtonStyle.Text -> PaddingValues(horizontal = 12.dp, vertical = 10.dp) - else -> PaddingValues(horizontal = 16.dp, vertical = 10.dp) } } ButtonSize.Large -> { when (style) { + ButtonStyle.Filled, + ButtonStyle.Outlined -> PaddingValues(horizontal = 24.dp, vertical = 13.dp) ButtonStyle.Text -> PaddingValues(horizontal = 16.dp, vertical = 13.dp) - else -> PaddingValues(horizontal = 24.dp, vertical = 13.dp) } } } val shape = when (style) { - ButtonStyle.Filled, ButtonStyle.Outlined -> RoundedCornerShape(percent = 50) + ButtonStyle.Filled, + ButtonStyle.Outlined -> RoundedCornerShape(percent = 50) ButtonStyle.Text -> RectangleShape } val border = when (style) { - ButtonStyle.Filled, ButtonStyle.Text -> null + ButtonStyle.Filled -> null ButtonStyle.Outlined -> BorderStroke( width = 1.dp, color = ElementTheme.colors.borderInteractiveSecondary ) + ButtonStyle.Text -> null } val textStyle = when (size) { @@ -166,9 +170,10 @@ internal fun ButtonInternal( ButtonSize.Large -> ElementTheme.typography.fontBodyLgMedium } - val internalPadding = when { - style == ButtonStyle.Text -> if (leadingIcon != null) PaddingValues(start = 8.dp) else PaddingValues(0.dp) - else -> PaddingValues(horizontal = 8.dp) + val internalPadding = when (style) { + ButtonStyle.Filled, + ButtonStyle.Outlined -> PaddingValues(horizontal = 8.dp) + ButtonStyle.Text -> if (leadingIcon != null) PaddingValues(start = 8.dp) else PaddingValues(0.dp) } androidx.compose.material3.Button( From 549218eeb22160685f0c2399d405d47f926e1a1c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 22 Aug 2023 10:38:24 +0200 Subject: [PATCH 2/4] Button theme: fix small integration mistake on button padding from Figma. --- .../designsystem/theme/components/Button.kt | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt index 99d2a68036..ccc014b2c8 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt @@ -133,23 +133,39 @@ internal fun ButtonInternal( ButtonSize.Large -> 48.dp } - val contentPadding = when (size) { - ButtonSize.Medium -> { - when (style) { - ButtonStyle.Filled, - ButtonStyle.Outlined -> PaddingValues(horizontal = 16.dp, vertical = 10.dp) - ButtonStyle.Text -> PaddingValues(horizontal = 12.dp, vertical = 10.dp) - } + val hasNoStartDrawable = !showProgress && leadingIcon == null + + val (paddingStart, paddingCenter, paddingEnd) = when (size) { + ButtonSize.Medium -> when (style) { + ButtonStyle.Filled, + ButtonStyle.Outlined -> if (hasNoStartDrawable) + Triple(24.dp, 0.dp, 24.dp) + else + Triple(16.dp, 8.dp, 24.dp) + ButtonStyle.Text -> if (hasNoStartDrawable) + Triple(12.dp, 0.dp, 12.dp) + else + Triple(12.dp, 8.dp, 16.dp) } - ButtonSize.Large -> { - when (style) { - ButtonStyle.Filled, - ButtonStyle.Outlined -> PaddingValues(horizontal = 24.dp, vertical = 13.dp) - ButtonStyle.Text -> PaddingValues(horizontal = 16.dp, vertical = 13.dp) - } + ButtonSize.Large -> when (style) { + ButtonStyle.Filled, + ButtonStyle.Outlined -> if (hasNoStartDrawable) + Triple(32.dp, 0.dp, 32.dp) + else + Triple(24.dp, 8.dp, 32.dp) + ButtonStyle.Text -> if (hasNoStartDrawable) + Triple(16.dp, 0.dp, 16.dp) + else + Triple(12.dp, 8.dp, 16.dp) } } + val contentPadding = when (size) { + ButtonSize.Medium -> PaddingValues(start = paddingStart, end = paddingEnd, top = 10.dp, bottom = 10.dp) + ButtonSize.Large -> PaddingValues(start = paddingStart, end = paddingEnd, top = 13.dp, bottom = 13.dp) + } + val internalPadding = PaddingValues(start = paddingCenter) + val shape = when (style) { ButtonStyle.Filled, ButtonStyle.Outlined -> RoundedCornerShape(percent = 50) @@ -170,12 +186,6 @@ internal fun ButtonInternal( ButtonSize.Large -> ElementTheme.typography.fontBodyLgMedium } - val internalPadding = when (style) { - ButtonStyle.Filled, - ButtonStyle.Outlined -> PaddingValues(horizontal = 8.dp) - ButtonStyle.Text -> if (leadingIcon != null) PaddingValues(start = 8.dp) else PaddingValues(0.dp) - } - androidx.compose.material3.Button( onClick = { if (!showProgress) { From 16c4801dd3c51f1db94324a4408f5b89ea4068b2 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Tue, 22 Aug 2023 08:49:40 +0000 Subject: [PATCH 3/4] Update screenshots --- ...ts_null_Buttons_TextButtonLarge_0_null,NEXUS_5,1.0,en].png | 4 ++-- ...s_null_Buttons_TextButtonMedium_0_null,NEXUS_5,1.0,en].png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonLarge_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonLarge_0_null,NEXUS_5,1.0,en].png index 775891e893..1f610f0fa1 100644 --- a/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonLarge_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonLarge_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:83ae98e86b9e229c82e66e495ce9130f8aac3fd3364479409dd8aec2f60f5c44 -size 32430 +oid sha256:d8233ab7e58175a8dbd14aa8c30dda882176099232cd56417102fa3675efe92b +size 31114 diff --git a/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonMedium_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonMedium_0_null,NEXUS_5,1.0,en].png index 5eb71bc999..9c23dea6cf 100644 --- a/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonMedium_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/ui_S_t[l.designsystem.theme.components_null_Buttons_TextButtonMedium_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d8e9ce0aafcaa2c873d706c6e477f2103c8f930f0c15e186a981eaa831e51bfc -size 30609 +oid sha256:974dd693b78cc08e278ff81a65e234629cdbf1075d3fbf255895cf1ee2eb906c +size 29264 From 7ea9512642b23a1a724b2475313c6d4f42bc796e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 22 Aug 2023 18:10:49 +0200 Subject: [PATCH 4/4] Simplify code (code review). --- .../designsystem/theme/components/Button.kt | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt index ccc014b2c8..ff613226e2 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding @@ -133,39 +134,33 @@ internal fun ButtonInternal( ButtonSize.Large -> 48.dp } - val hasNoStartDrawable = !showProgress && leadingIcon == null + val hasStartDrawable = showProgress || leadingIcon != null - val (paddingStart, paddingCenter, paddingEnd) = when (size) { + val contentPadding = when (size) { ButtonSize.Medium -> when (style) { ButtonStyle.Filled, - ButtonStyle.Outlined -> if (hasNoStartDrawable) - Triple(24.dp, 0.dp, 24.dp) + ButtonStyle.Outlined -> if (hasStartDrawable) + PaddingValues(start = 16.dp, top = 10.dp, end = 24.dp, bottom = 10.dp) else - Triple(16.dp, 8.dp, 24.dp) - ButtonStyle.Text -> if (hasNoStartDrawable) - Triple(12.dp, 0.dp, 12.dp) + PaddingValues(start = 24.dp, top = 10.dp, end = 24.dp, bottom = 10.dp) + ButtonStyle.Text -> if (hasStartDrawable) + PaddingValues(start = 12.dp, top = 10.dp, end = 16.dp, bottom = 10.dp) else - Triple(12.dp, 8.dp, 16.dp) + PaddingValues(start = 12.dp, top = 10.dp, end = 12.dp, bottom = 10.dp) } ButtonSize.Large -> when (style) { ButtonStyle.Filled, - ButtonStyle.Outlined -> if (hasNoStartDrawable) - Triple(32.dp, 0.dp, 32.dp) + ButtonStyle.Outlined -> if (hasStartDrawable) + PaddingValues(start = 24.dp, top = 13.dp, end = 32.dp, bottom = 13.dp) else - Triple(24.dp, 8.dp, 32.dp) - ButtonStyle.Text -> if (hasNoStartDrawable) - Triple(16.dp, 0.dp, 16.dp) + PaddingValues(start = 32.dp, top = 13.dp, end = 32.dp, bottom = 13.dp) + ButtonStyle.Text -> if (hasStartDrawable) + PaddingValues(start = 12.dp, top = 13.dp, end = 16.dp, bottom = 13.dp) else - Triple(12.dp, 8.dp, 16.dp) + PaddingValues(start = 16.dp, top = 13.dp, end = 16.dp, bottom = 13.dp) } } - val contentPadding = when (size) { - ButtonSize.Medium -> PaddingValues(start = paddingStart, end = paddingEnd, top = 10.dp, bottom = 10.dp) - ButtonSize.Large -> PaddingValues(start = paddingStart, end = paddingEnd, top = 13.dp, bottom = 13.dp) - } - val internalPadding = PaddingValues(start = paddingCenter) - val shape = when (style) { ButtonStyle.Filled, ButtonStyle.Outlined -> RoundedCornerShape(percent = 50) @@ -210,6 +205,7 @@ internal fun ButtonInternal( color = LocalContentColor.current, strokeWidth = 2.dp, ) + Spacer(modifier = Modifier.width(8.dp)) } leadingIcon != null -> { androidx.compose.material.Icon( @@ -218,15 +214,14 @@ internal fun ButtonInternal( tint = LocalContentColor.current, modifier = Modifier.size(20.dp), ) + Spacer(modifier = Modifier.width(8.dp)) } - else -> Unit } Text( text = text, style = textStyle, maxLines = 1, overflow = TextOverflow.Ellipsis, - modifier = Modifier.padding(internalPadding), ) } }