Fix button highlight and refactor screen

3 files changed, 169 insertions, 280 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
deleted file mode 100644
index 4b774bfe8f..0000000000
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/List.kt
+++ /dev/null
@@ -1,138 +0,0 @@
-package net.mullvad.mullvadvpn.compose.component
-
-import androidx.annotation.DrawableRes
-import androidx.compose.foundation.Image
-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.defaultMinSize
-import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.height
-import androidx.compose.foundation.layout.padding
-import androidx.compose.foundation.layout.wrapContentHeight
-import androidx.compose.material3.MaterialTheme
-import androidx.compose.material3.Text
-import androidx.compose.runtime.Composable
-import androidx.compose.ui.Alignment
-import androidx.compose.ui.Modifier
-import androidx.compose.ui.graphics.Color
-import androidx.compose.ui.graphics.compositeOver
-import androidx.compose.ui.res.painterResource
-import androidx.compose.ui.tooling.preview.Preview
-import androidx.compose.ui.unit.Dp
-import net.mullvad.mullvadvpn.R
-import net.mullvad.mullvadvpn.lib.theme.AppTheme
-import net.mullvad.mullvadvpn.lib.theme.Dimens
-import net.mullvad.mullvadvpn.lib.theme.color.AlphaDescription
-import net.mullvad.mullvadvpn.lib.theme.typeface.listItemSubText
-import net.mullvad.mullvadvpn.lib.theme.typeface.listItemText
-
-@Preview
-@Composable
-private fun PreviewListItem() {
-    AppTheme {
-        Column {
-            ListItem(text = "No subtext No icon not loading", isLoading = false, onClick = {})
-            ListItem(text = "No subtext No icon is loading", isLoading = true, onClick = {})
-            ListItem(
-                text = "No subtext With icon is loading",
-                isLoading = true,
-                iconResourceId = R.drawable.icon_close,
-                onClick = {}
-            )
-            ListItem(
-                text = "No subtext With icon not loading",
-                isLoading = false,
-                iconResourceId = R.drawable.icon_close,
-                onClick = {}
-            )
-            ListItem(
-                text = "With subtext with icon is loading",
-                subText = "Subtext",
-                isLoading = true,
-                iconResourceId = R.drawable.icon_close,
-                onClick = {}
-            )
-            ListItem(
-                text = "With subtext no icon is loading",
-                subText = "Subtext",
-                isLoading = true,
-                onClick = {}
-            )
-            ListItem(
-                text = "With subtext with icon not loading",
-                subText = "Subtext",
-                isLoading = false,
-                iconResourceId = R.drawable.icon_close,
-                onClick = {}
-            )
-            ListItem(
-                text = "With subtext no icon not loading",
-                subText = "Subtext",
-                isLoading = false,
-                onClick = {}
-            )
-        }
-    }
-}
-
-@Composable
-fun ListItem(
-    text: String,
-    subText: String? = null,
-    height: Dp = Dimens.listItemHeight,
-    isLoading: Boolean,
-    @DrawableRes iconResourceId: Int? = null,
-    background: Color = MaterialTheme.colorScheme.primary,
-    onClick: (() -> Unit)?
-) {
-    Box(
-        modifier =
-            Modifier.fillMaxWidth()
-                .padding(vertical = Dimens.listItemDivider)
-                .wrapContentHeight()
-                .defaultMinSize(minHeight = height)
-                .background(background)
-    ) {
-        Column(
-            modifier =
-                Modifier.padding(horizontal = Dimens.mediumPadding, vertical = Dimens.smallPadding)
-                    .align(Alignment.CenterStart)
-        ) {
-            Text(
-                text = text,
-                style = MaterialTheme.typography.listItemText,
-                color = MaterialTheme.colorScheme.onPrimary
-            )
-            subText?.let {
-                Text(
-                    text = subText,
-                    style = MaterialTheme.typography.listItemSubText,
-                    color =
-                        MaterialTheme.colorScheme.onPrimary
-                            .copy(alpha = AlphaDescription)
-                            .compositeOver(background)
-                )
-            }
-        }
-
-        Box(
-            modifier =
-                Modifier.align(Alignment.CenterEnd)
-                    .padding(horizontal = Dimens.loadingSpinnerPadding)
-        ) {
-            if (isLoading) {
-                MullvadCircularProgressIndicatorMedium()
-            } else if (iconResourceId != null) {
-                Image(
-                    painter = painterResource(id = iconResourceId),
-                    contentDescription = "Remove",
-                    modifier =
-                        onClick?.let { Modifier.align(Alignment.CenterEnd).clickable { onClick() } }
-                            ?: Modifier.align(Alignment.CenterEnd)
-                )
-            }
-        }
-    }
-}
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
index bd50d45c86..6db5d64735 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
@@ -171,6 +171,7 @@ fun MullvadTopBar(
         colors =
             TopAppBarDefaults.topAppBarColors(
                 containerColor = containerColor,
+                actionIconContentColor = iconTintColor,
             ),
     )
 }
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceListScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceListScreen.kt
index 7064d82b9e..1617c1fb7a 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceListScreen.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceListScreen.kt
@@ -2,37 +2,44 @@ package net.mullvad.mullvadvpn.compose.screen
 
 import androidx.compose.animation.animateContentSize
 import androidx.compose.foundation.Image
-import androidx.compose.foundation.background
-import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.fillMaxHeight
+import androidx.compose.foundation.layout.ColumnScope
+import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.wrapContentHeight
 import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.verticalScroll
+import androidx.compose.material3.Divider
+import androidx.compose.material3.Icon
+import androidx.compose.material3.IconButton
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.compositeOver
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
-import androidx.constraintlayout.compose.ConstraintLayout
-import androidx.constraintlayout.compose.Dimension
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.compose.button.PrimaryButton
 import net.mullvad.mullvadvpn.compose.button.VariantButton
-import net.mullvad.mullvadvpn.compose.component.ListItem
+import net.mullvad.mullvadvpn.compose.cell.BaseCell
+import net.mullvad.mullvadvpn.compose.component.MullvadCircularProgressIndicatorMedium
 import net.mullvad.mullvadvpn.compose.component.ScaffoldWithTopBar
+import net.mullvad.mullvadvpn.compose.component.drawVerticalScrollbar
 import net.mullvad.mullvadvpn.compose.dialog.DeviceRemovalDialog
 import net.mullvad.mullvadvpn.compose.state.DeviceListItemUiState
 import net.mullvad.mullvadvpn.compose.state.DeviceListUiState
 import net.mullvad.mullvadvpn.lib.common.util.parseAsDateTime
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.lib.theme.Dimens
+import net.mullvad.mullvadvpn.lib.theme.color.AlphaDescription
 import net.mullvad.mullvadvpn.lib.theme.color.AlphaTopBar
+import net.mullvad.mullvadvpn.lib.theme.typeface.listItemSubText
+import net.mullvad.mullvadvpn.lib.theme.typeface.listItemText
 import net.mullvad.mullvadvpn.model.Device
 import net.mullvad.mullvadvpn.util.formatDate
 
@@ -89,156 +96,175 @@ fun DeviceListScreen(
         onSettingsClicked = onSettingsClicked,
         onAccountClicked = null,
     ) {
-        ConstraintLayout(
-            modifier =
-                Modifier.fillMaxHeight()
-                    .fillMaxWidth()
-                    .padding(it)
-                    .background(MaterialTheme.colorScheme.background)
+        Column(
+            modifier = Modifier.fillMaxSize().padding(it),
         ) {
-            val (content, buttons) = createRefs()
-
+            val scrollState = rememberScrollState()
             Column(
                 modifier =
-                    Modifier.constrainAs(content) {
-                            top.linkTo(parent.top)
-                            bottom.linkTo(buttons.top)
-                            height = Dimension.fillToConstraints
-                            width = Dimension.matchParent
-                        }
-                        .verticalScroll(rememberScrollState())
+                    Modifier.drawVerticalScrollbar(
+                            scrollState,
+                            MaterialTheme.colorScheme.onBackground
+                        )
+                        .verticalScroll(scrollState)
+                        .weight(1f)
             ) {
-                ConstraintLayout(modifier = Modifier.fillMaxWidth().wrapContentHeight()) {
-                    val (icon, message, list) = createRefs()
+                DeviceListHeader(state = state)
 
-                    Image(
-                        painter =
-                            painterResource(
-                                id =
-                                    if (state.hasTooManyDevices) {
-                                        R.drawable.icon_fail
-                                    } else {
-                                        R.drawable.icon_success
-                                    }
-                            ),
-                        contentDescription = null, // No meaningful user info or action.
-                        modifier =
-                            Modifier.constrainAs(icon) {
-                                    top.linkTo(parent.top)
-                                    start.linkTo(parent.start)
-                                    end.linkTo(parent.end)
-                                }
-                                .padding(top = Dimens.iconFailSuccessTopMargin)
-                                .size(Dimens.bigIconSize)
-                    )
-
-                    Column(
-                        modifier =
-                            Modifier.constrainAs(message) {
-                                    top.linkTo(icon.bottom)
-                                    start.linkTo(parent.start)
-                                    end.linkTo(parent.end)
-                                    width = Dimension.fillToConstraints
-                                }
-                                .padding(
-                                    start = Dimens.sideMargin,
-                                    end = Dimens.sideMargin,
-                                    top = Dimens.screenVerticalMargin
-                                ),
+                state.deviceUiItems.forEachIndexed { index, deviceUiState ->
+                    DeviceListItem(
+                        deviceUiState = deviceUiState,
                     ) {
-                        Text(
-                            text =
-                                stringResource(
-                                    id =
-                                        if (state.hasTooManyDevices) {
-                                            R.string.max_devices_warning_title
-                                        } else {
-                                            R.string.max_devices_resolved_title
-                                        }
-                                ),
-                            style = MaterialTheme.typography.headlineSmall,
-                            color = MaterialTheme.colorScheme.onBackground
-                        )
-
-                        Text(
-                            text =
-                                stringResource(
-                                    id =
-                                        if (state.hasTooManyDevices) {
-                                            R.string.max_devices_warning_description
-                                        } else {
-                                            R.string.max_devices_resolved_description
-                                        }
-                                ),
-                            style = MaterialTheme.typography.bodySmall,
-                            color = MaterialTheme.colorScheme.onBackground,
-                            modifier =
-                                Modifier.wrapContentHeight()
-                                    .animateContentSize()
-                                    .padding(top = Dimens.smallPadding)
-                        )
+                        onDeviceRemovalClicked(deviceUiState.device.id)
                     }
-
-                    Box(
-                        modifier =
-                            Modifier.constrainAs(list) {
-                                    top.linkTo(message.bottom)
-                                    height = Dimension.wrapContent
-                                    width = Dimension.matchParent
-                                }
-                                .padding(top = Dimens.spacingAboveButton)
-                    ) {
-                        Column {
-                            state.deviceUiItems.forEach { deviceUiState ->
-                                ListItem(
-                                    text = deviceUiState.device.displayName(),
-                                    subText =
-                                        deviceUiState.device.created.parseAsDateTime()?.let {
-                                            creationDate ->
-                                            stringResource(
-                                                id = R.string.created_x,
-                                                creationDate.formatDate()
-                                            )
-                                        },
-                                    height = Dimens.listItemHeightExtra,
-                                    isLoading = deviceUiState.isLoading,
-                                    iconResourceId = R.drawable.icon_close
-                                ) {
-                                    onDeviceRemovalClicked(deviceUiState.device.id)
-                                }
-                            }
-                        }
+                    if (state.deviceUiItems.lastIndex != index) {
+                        Divider()
                     }
                 }
             }
+            DeviceListButtonPanel(state, onContinueWithLogin, onBackClick)
+        }
+    }
+}
 
-            Column(
-                modifier =
-                    Modifier.constrainAs(buttons) {
-                            bottom.linkTo(parent.bottom)
-                            start.linkTo(parent.start)
-                            end.linkTo(parent.end)
-                            width = Dimension.fillToConstraints
-                        }
-                        .padding(
-                            start = Dimens.sideMargin,
-                            end = Dimens.sideMargin,
-                            bottom = Dimens.screenVerticalMargin
-                        )
-            ) {
-                VariantButton(
-                    text = stringResource(id = R.string.continue_login),
-                    onClick = onContinueWithLogin,
-                    isEnabled = state.hasTooManyDevices.not() && state.isLoading.not(),
-                    background = MaterialTheme.colorScheme.secondary
+@Composable
+private fun ColumnScope.DeviceListHeader(state: DeviceListUiState) {
+    Image(
+        painter =
+            painterResource(
+                id =
+                    if (state.hasTooManyDevices) {
+                        R.drawable.icon_fail
+                    } else {
+                        R.drawable.icon_success
+                    }
+            ),
+        contentDescription = null, // No meaningful user info or action.
+        modifier =
+            Modifier.align(Alignment.CenterHorizontally)
+                .padding(top = Dimens.iconFailSuccessTopMargin)
+                .size(Dimens.bigIconSize)
+    )
+
+    Text(
+        text =
+            stringResource(
+                id =
+                    if (state.hasTooManyDevices) {
+                        R.string.max_devices_warning_title
+                    } else {
+                        R.string.max_devices_resolved_title
+                    }
+            ),
+        style = MaterialTheme.typography.headlineSmall,
+        color = MaterialTheme.colorScheme.onBackground,
+        modifier =
+            Modifier.padding(
+                start = Dimens.sideMargin,
+                end = Dimens.sideMargin,
+                top = Dimens.screenVerticalMargin
+            ),
+    )
+
+    Text(
+        text =
+            stringResource(
+                id =
+                    if (state.hasTooManyDevices) {
+                        R.string.max_devices_warning_description
+                    } else {
+                        R.string.max_devices_resolved_description
+                    }
+            ),
+        style = MaterialTheme.typography.bodySmall,
+        color = MaterialTheme.colorScheme.onBackground,
+        modifier =
+            Modifier.wrapContentHeight()
+                .animateContentSize()
+                .padding(
+                    top = Dimens.smallPadding,
+                    start = Dimens.sideMargin,
+                    end = Dimens.sideMargin,
+                    bottom = Dimens.spacingAboveButton
                 )
+    )
+}
 
-                PrimaryButton(
-                    text = stringResource(id = R.string.back),
-                    onClick = onBackClick,
-                    modifier = Modifier.padding(top = Dimens.buttonSpacing)
+@Composable
+private fun DeviceListItem(
+    deviceUiState: DeviceListItemUiState,
+    onDeviceRemovalClicked: () -> Unit
+) {
+    BaseCell(
+        isRowEnabled = false,
+        title = {
+            Column(modifier = Modifier.weight(1f)) {
+                Text(
+                    modifier = Modifier.fillMaxWidth(),
+                    text = deviceUiState.device.displayName(),
+                    style = MaterialTheme.typography.listItemText,
+                    color = MaterialTheme.colorScheme.onPrimary
+                )
+                Text(
+                    modifier = Modifier.fillMaxWidth(),
+                    text =
+                        deviceUiState.device.created.parseAsDateTime()?.let { creationDate ->
+                            stringResource(id = R.string.created_x, creationDate.formatDate())
+                        }
+                            ?: "",
+                    style = MaterialTheme.typography.listItemSubText,
+                    color =
+                        MaterialTheme.colorScheme.onPrimary
+                            .copy(alpha = AlphaDescription)
+                            .compositeOver(MaterialTheme.colorScheme.primary)
                 )
             }
-        }
+        },
+        bodyView = {
+            if (deviceUiState.isLoading) {
+                MullvadCircularProgressIndicatorMedium(
+                    modifier = Modifier.padding(Dimens.smallPadding)
+                )
+            } else {
+                IconButton(onClick = onDeviceRemovalClicked) {
+                    Icon(
+                        painter = painterResource(id = R.drawable.icon_close),
+                        contentDescription = stringResource(id = R.string.remove_button),
+                    )
+                }
+            }
+        },
+        endPadding = Dimens.smallPadding,
+    )
+}
+
+@Composable
+private fun DeviceListButtonPanel(
+    state: DeviceListUiState,
+    onContinueWithLogin: () -> Unit,
+    onBackClick: () -> Unit
+) {
+
+    Column(
+        modifier =
+            Modifier.padding(
+                start = Dimens.sideMargin,
+                end = Dimens.sideMargin,
+                top = Dimens.spacingAboveButton,
+                bottom = Dimens.screenVerticalMargin
+            )
+    ) {
+        VariantButton(
+            text = stringResource(id = R.string.continue_login),
+            onClick = onContinueWithLogin,
+            isEnabled = state.hasTooManyDevices.not() && state.isLoading.not(),
+            background = MaterialTheme.colorScheme.secondary
+        )
+
+        PrimaryButton(
+            text = stringResource(id = R.string.back),
+            onClick = onBackClick,
+            modifier = Modifier.padding(top = Dimens.buttonSpacing)
+        )
     }
 }