Improve changelog dialog

- Replace hardcoded font sizes and colors - Place it within AppTheme - Make it scrollable - Change some text styles to be more in line with design
mullvad · Sep 21, 2023 · 1f1bc76 · 1f1bc76
1 parent b0ea3c0
commit 1f1bc76
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 112 deletions.
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/List.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/List.kt
@@ -6,7 +6,6 @@ import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.absolutePadding
 import androidx.compose.foundation.layout.defaultMinSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.height
@@ -23,7 +22,6 @@ import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.Dp
-import androidx.constraintlayout.compose.ConstraintLayout
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.lib.theme.Dimens
@@ -79,12 +77,6 @@ fun PreviewListItem() {
     }
 }
 
-@Preview
-@Composable
-fun PreviewChangeListItem() {
-    ChangeListItem(text = "ChangeListItem")
-}
-
 @Composable
 fun ListItem(
     text: String,
@@ -146,40 +138,3 @@ fun ListItem(
         }
     }
 }
-
-@Composable
-fun ChangeListItem(text: String) {
-
-    ConstraintLayout {
-        val (bullet, changeLog) = createRefs()
-        Box(
-            modifier =
-                Modifier.constrainAs(bullet) {
-                    top.linkTo(parent.top)
-                    start.linkTo(parent.absoluteLeft)
-                }
-        ) {
-            Text(
-                text = "•",
-                style = MaterialTheme.typography.bodyMedium,
-                color = MaterialTheme.colorScheme.onPrimary
-            )
-        }
-        Box(
-            modifier =
-                Modifier.absolutePadding(left = Dimens.mediumPadding).constrainAs(changeLog) {
-                    top.linkTo(parent.top)
-                    bottom.linkTo(parent.bottom)
-                    start.linkTo(parent.start)
-                    end.linkTo(parent.end)
-                }
-        ) {
-            Text(
-                text = text,
-                style = MaterialTheme.typography.bodyMedium,
-                color = MaterialTheme.colorScheme.onPrimary,
-                modifier = Modifier
-            )
-        }
-    }
-}
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/ChangelogDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/ChangelogDialog.kt
@@ -1,28 +1,26 @@
 package net.mullvad.mullvadvpn.compose.dialog
 
+import androidx.compose.foundation.ScrollState
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.defaultMinSize
+import androidx.compose.foundation.layout.absolutePadding
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.wrapContentHeight
+import androidx.compose.foundation.rememberScrollState
+import androidx.compose.foundation.verticalScroll
 import androidx.compose.material3.AlertDialog
-import androidx.compose.material3.Button
 import androidx.compose.material3.ButtonDefaults
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.graphics.Color
-import androidx.compose.ui.res.colorResource
-import androidx.compose.ui.res.dimensionResource
 import androidx.compose.ui.res.stringResource
-import androidx.compose.ui.text.font.FontStyle
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.tooling.preview.Preview
-import androidx.compose.ui.unit.sp
-import androidx.compose.ui.window.DialogProperties
+import androidx.constraintlayout.compose.ConstraintLayout
 import net.mullvad.mullvadvpn.R
-import net.mullvad.mullvadvpn.compose.component.ChangeListItem
+import net.mullvad.mullvadvpn.compose.button.ActionButton
+import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.lib.theme.Dimens
 
 @Composable
@@ -32,61 +30,84 @@ fun ChangelogDialog(changesList: List<String>, version: String, onDismiss: () ->
         title = {
             Text(
                 text = version,
-                color = colorResource(id = R.color.white),
-                fontSize = 30.sp,
-                fontStyle = FontStyle.Normal,
+                style = MaterialTheme.typography.headlineLarge,
                 textAlign = TextAlign.Center,
                 modifier = Modifier.fillMaxWidth()
             )
         },
         text = {
+            val scrollState: ScrollState = rememberScrollState()
             Column(
-                modifier = Modifier.fillMaxWidth(),
-                verticalArrangement = Arrangement.spacedBy(Dimens.smallPadding)
+                modifier = Modifier.fillMaxWidth().verticalScroll(scrollState),
+                verticalArrangement = Arrangement.spacedBy(Dimens.smallPadding),
             ) {
                 Text(
                     text = stringResource(R.string.changes_dialog_subtitle),
-                    fontSize = 18.sp,
-                    color = Color.White,
+                    style = MaterialTheme.typography.titleSmall,
+                    color = MaterialTheme.colorScheme.onBackground,
                     modifier = Modifier.fillMaxWidth()
                 )
 
                 changesList.forEach { changeItem -> ChangeListItem(text = changeItem) }
             }
         },
         confirmButton = {
-            Button(
-                modifier =
-                    Modifier.wrapContentHeight()
-                        .defaultMinSize(minHeight = dimensionResource(id = R.dimen.button_height))
-                        .fillMaxWidth(),
+            ActionButton(
+                text = stringResource(R.string.changes_dialog_dismiss_button),
+                onClick = onDismiss,
                 colors =
                     ButtonDefaults.buttonColors(
-                        containerColor = colorResource(id = R.color.blue),
-                        contentColor = colorResource(id = R.color.white)
-                    ),
-                onClick = { onDismiss() },
-                shape = MaterialTheme.shapes.small
-            ) {
-                Text(
-                    text = stringResource(R.string.changes_dialog_dismiss_button),
-                    fontSize = 18.sp
-                )
-            }
+                        containerColor = MaterialTheme.colorScheme.primary,
+                        contentColor = MaterialTheme.colorScheme.onPrimary,
+                    )
+            )
         },
-        properties =
-            DialogProperties(
-                dismissOnClickOutside = true,
-                dismissOnBackPress = true,
-            ),
-        containerColor = colorResource(id = R.color.darkBlue)
+        containerColor = MaterialTheme.colorScheme.background,
+        titleContentColor = MaterialTheme.colorScheme.onBackground
     )
 }
 
+@Composable
+private fun ChangeListItem(text: String) {
+
+    ConstraintLayout {
+        val (bullet, changeLog) = createRefs()
+        Box(
+            modifier =
+                Modifier.constrainAs(bullet) {
+                    top.linkTo(parent.top)
+                    start.linkTo(parent.absoluteLeft)
+                }
+        ) {
+            Text(
+                text = "•",
+                style = MaterialTheme.typography.labelMedium,
+                color = MaterialTheme.colorScheme.onBackground
+            )
+        }
+        Box(
+            modifier =
+                Modifier.absolutePadding(left = Dimens.mediumPadding).constrainAs(changeLog) {
+                    top.linkTo(parent.top)
+                    bottom.linkTo(parent.bottom)
+                    start.linkTo(parent.start)
+                    end.linkTo(parent.end)
+                }
+        ) {
+            Text(
+                text = text,
+                style = MaterialTheme.typography.labelMedium,
+                color = MaterialTheme.colorScheme.onBackground,
+                modifier = Modifier
+            )
+        }
+    }
+}
+
 @Preview
 @Composable
 private fun PreviewChangelogDialogWithSingleShortItem() {
-    ChangelogDialog(changesList = listOf("Item 1"), version = "1111.1", onDismiss = {})
+    AppTheme { ChangelogDialog(changesList = listOf("Item 1"), version = "1111.1", onDismiss = {}) }
 }
 
 @Preview
@@ -97,31 +118,35 @@ private fun PreviewChangelogDialogWithTwoLongItems() {
             "The purpose of this specific sample text is to visualize a long text that will result " +
             "in multiple lines in the changelog dialog."
 
-    ChangelogDialog(
-        changesList = listOf(longPreviewText, longPreviewText),
-        version = "1111.1",
-        onDismiss = {}
-    )
+    AppTheme {
+        ChangelogDialog(
+            changesList = listOf(longPreviewText, longPreviewText),
+            version = "1111.1",
+            onDismiss = {}
+        )
+    }
 }
 
 @Preview
 @Composable
 private fun PreviewChangelogDialogWithTenShortItems() {
-    ChangelogDialog(
-        changesList =
-            listOf(
-                "Item 1",
-                "Item 2",
-                "Item 3",
-                "Item 4",
-                "Item 5",
-                "Item 6",
-                "Item 7",
-                "Item 8",
-                "Item 9",
-                "Item 10"
-            ),
-        version = "1111.1",
-        onDismiss = {}
-    )
+    AppTheme {
+        ChangelogDialog(
+            changesList =
+                listOf(
+                    "Item 1",
+                    "Item 2",
+                    "Item 3",
+                    "Item 4",
+                    "Item 5",
+                    "Item 6",
+                    "Item 7",
+                    "Item 8",
+                    "Item 9",
+                    "Item 10"
+                ),
+            version = "1111.1",
+            onDismiss = {}
+        )
+    }
 }
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
@@ -35,6 +35,7 @@ import net.mullvad.mullvadvpn.di.uiModule
 import net.mullvad.mullvadvpn.lib.common.util.SdkUtils.isNotificationPermissionGranted
 import net.mullvad.mullvadvpn.lib.endpoint.ApiEndpointConfiguration
 import net.mullvad.mullvadvpn.lib.endpoint.getApiEndpointConfigurationExtras
+import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.model.AccountExpiry
 import net.mullvad.mullvadvpn.model.DeviceState
 import net.mullvad.mullvadvpn.repository.AccountRepository
@@ -254,11 +255,13 @@ open class MainActivity : FragmentActivity() {
             setContent {
                 val state = changelogViewModel.changelogDialogUiState.collectAsState().value
                 if (state is ChangelogDialogUiState.Show) {
-                    ChangelogDialog(
-                        changesList = state.changes,
-                        version = BuildConfig.VERSION_NAME,
-                        onDismiss = { changelogViewModel.dismissChangelogDialog() }
-                    )
+                    AppTheme {
+                        ChangelogDialog(
+                            changesList = state.changes,
+                            version = BuildConfig.VERSION_NAME,
+                            onDismiss = { changelogViewModel.dismissChangelogDialog() }
+                        )
+                    }
                 }
             }
             changelogViewModel.refreshChangelogDialogUiState()