review: Use Spacers instead of modifying padding

Infomaniak · Oct 18, 2024 · 8d4c942 · 8d4c942
1 parent a894228
commit 8d4c942
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 37 deletions.
diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/EmailAddressChip.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/EmailAddressChip.kt
@@ -32,9 +32,9 @@ import com.infomaniak.swisstransfer.ui.theme.Margin
 import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme
 
 @Composable
-fun EmailAddressChip(text: String, modifier: Modifier = Modifier) {
+fun EmailAddressChip(text: String) {
     Box(
-        modifier = modifier
+        modifier = Modifier
             .clip(CustomShapes.Rounded)
             .background(SwissTransferTheme.colors.emailAddressChipColor)
             .padding(horizontal = Margin.Small),

diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/IllustratedMessageBlock.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/IllustratedMessageBlock.kt
@@ -51,20 +51,21 @@ fun IllustratedMessageBlock(
     ) {
         Image(imageVector = icon, contentDescription = null)
 
+        Spacer(Modifier.height(Margin.XLarge))
+
         Text(
             text = stringResource(title),
             style = SwissTransferTheme.typography.h1,
-            modifier = Modifier.padding(PaddingValues(top = Margin.XLarge)),
         )
 
+        Spacer(Modifier.height(Margin.Medium))
+
         Text(
             text = stringResource(description),
             textAlign = TextAlign.Center,
             style = SwissTransferTheme.typography.bodyRegular,
             color = SwissTransferTheme.colors.secondaryTextColor,
-            modifier = Modifier
-                .widthIn(max = Dimens.DescriptionWidth)
-                .padding(PaddingValues(top = Margin.Medium)),
+            modifier = Modifier.widthIn(max = Dimens.DescriptionWidth),
         )
     }
 }

diff --git a/.../java/com/infomaniak/swisstransfer/ui/screen/main/received/components/ReceivedEmptyFab.kt b/.../java/com/infomaniak/swisstransfer/ui/screen/main/received/components/ReceivedEmptyFab.kt
@@ -18,7 +18,10 @@
 package com.infomaniak.swisstransfer.ui.screen.main.received.components
 
 import android.content.res.Configuration
-import androidx.compose.foundation.layout.*
+import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.fillMaxSize
+import androidx.compose.foundation.layout.padding
 import androidx.compose.material3.Icon
 import androidx.compose.material3.Surface
 import androidx.compose.material3.Text
@@ -47,7 +50,7 @@ fun ReceivedEmptyFab(isMessageVisible: () -> Boolean) {
                 style = SwissTransferTheme.typography.bodyRegular,
                 color = SwissTransferTheme.colors.secondaryTextColor,
                 modifier = Modifier
-                    .padding(PaddingValues(top = Margin.XLarge))
+                    .padding(top = Margin.XLarge)
                     .constrainAs(text) {
                         bottom.linkTo(icon.top, Margin.Small)
                         end.linkTo(icon.end, Margin.Small)

diff --git a/...ava/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadSuccessEmailScreen.kt b/...ava/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadSuccessEmailScreen.kt
@@ -17,10 +17,7 @@
  */
 package com.infomaniak.swisstransfer.ui.screen.newtransfer.upload
 
-import androidx.compose.foundation.layout.Arrangement
-import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.*
 import androidx.compose.material3.Surface
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
@@ -57,10 +54,9 @@ fun UploadSuccessEmailScreen() {
                     description = R.string.uploadSuccessEmailDescription,
                 )
 
-                EmailAddressChip(
-                    text = "[email protected]", // TODO: Use correct email instead of hard-coded value.
-                    modifier = Modifier.padding(top = Margin.Medium),
-                )
+                Spacer(Modifier.height(Margin.Medium))
+
+                EmailAddressChip("[email protected]") // TODO: Use correct email instead of hard-coded value.
             }
         },
     )

diff --git a/...n/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadSuccessQrScreen.kt b/...n/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadSuccessQrScreen.kt
@@ -70,37 +70,45 @@ fun UploadSuccessQrScreen(transferType: TransferType) {
                 horizontalAlignment = Alignment.CenterHorizontally,
                 verticalArrangement = Arrangement.Center,
             ) {
+
                 Image(
                     imageVector = AppIllus.UploadSuccessQr.image(),
                     contentDescription = null,
                 )
+
+                Spacer(Modifier.height(Margin.XLarge))
+
                 Text(
                     text = stringResource(uploadSuccessTitle),
                     style = SwissTransferTheme.typography.h1,
                     color = SwissTransferTheme.colors.primaryTextColor,
-                    modifier = Modifier.padding(Margin.XLarge)
                 )
+
+                Spacer(Modifier.height(Margin.XLarge))
+
                 QrCode()
-                Text(
-                    text = stringResource(R.string.uploadSuccessLinkDescription),
-                    style = SwissTransferTheme.typography.bodyRegular,
-                    textAlign = TextAlign.Center,
-                    color = SwissTransferTheme.colors.secondaryTextColor,
-                    modifier = Modifier
-                        .alpha(if (transferType == TransferType.QR_CODE) 0.0f else 1.0f)
-                        .padding(top = Margin.XXLarge, start = Margin.Medium, end = Margin.Medium)
-                        .widthIn(max = Dimens.DescriptionWidth),
-                )
-                // TODO: What do we want to do with this button placement?
-                LargeButton(
-                    modifier = Modifier
-                        .fillMaxSize()
-                        .padding(Margin.Medium),
-                    style = ButtonType.SECONDARY,
-                    titleRes = R.string.buttonCopyLink,
-                    onClick = { /* TODO */ },
-                )
+
+                if (transferType != TransferType.QR_CODE) {
+                    Spacer(Modifier.height(Margin.XXLarge))
+                    Text(
+                        text = stringResource(R.string.uploadSuccessLinkDescription),
+                        style = SwissTransferTheme.typography.bodyRegular,
+                        textAlign = TextAlign.Center,
+                        color = SwissTransferTheme.colors.secondaryTextColor,
+                        modifier = Modifier
+                            .alpha(1.0f)
+                            .widthIn(max = Dimens.DescriptionWidth),
+                    )
+                }
             }
+
+            // TODO: What do we want to do with this button placement?
+            LargeButton(
+                modifier = Modifier.padding(Margin.Medium),
+                style = ButtonType.SECONDARY,
+                titleRes = R.string.buttonCopyLink,
+                onClick = { /* TODO */ },
+            )
         },
     )
 }
@@ -110,7 +118,7 @@ fun UploadSuccessQrScreen(transferType: TransferType) {
 private fun UploadSuccessQrScreenPreview() {
     SwissTransferTheme {
         Surface {
-            UploadSuccessQrScreen(transferType = TransferType.QR_CODE)
+            UploadSuccessQrScreen(transferType = TransferType.LINK)
         }
     }
 }