From 76c822eaa23574d710bc08f96de92dab3a194fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Tue, 31 Oct 2023 13:26:29 +0100 Subject: [PATCH] Fix button highlight and refactor screen --- .../mullvadvpn/compose/component/List.kt | 32 +- .../mullvadvpn/compose/component/TopBar.kt | 1 + .../compose/screen/DeviceListScreen.kt | 327 ++++++++++-------- 3 files changed, 185 insertions(+), 175 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 index 77929b38f313..e3d5eff7483d 100644 --- 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 @@ -1,33 +1,25 @@ 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.width import androidx.compose.foundation.layout.wrapContentHeight -import androidx.compose.material3.CircularProgressIndicator 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 @@ -110,11 +102,6 @@ fun ListItem( subText?.let { Text( text = subText, - style = MaterialTheme.typography.listItemSubText, - color = - MaterialTheme.colorScheme.onPrimary - .copy(alpha = AlphaDescription) - .compositeOver(background) ) } } @@ -123,23 +110,6 @@ fun ListItem( modifier = Modifier.align(Alignment.CenterEnd) .padding(horizontal = Dimens.loadingSpinnerPadding) - ) { - if (isLoading) { - CircularProgressIndicator( - strokeWidth = Dimens.loadingSpinnerStrokeWidth, - color = MaterialTheme.colorScheme.onPrimary, - modifier = - Modifier.height(Dimens.loadingSpinnerSize).width(Dimens.loadingSpinnerSize) - ) - } 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 8130e71963f6..cba7fa1eebc2 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 19a06e453d89..e35649981a2c 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,10 +2,9 @@ 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.height import androidx.compose.foundation.layout.padding @@ -14,27 +13,35 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.CircularProgressIndicator +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.ScaffoldWithTopBar +import net.mullvad.mullvadvpn.compose.component.drawVerticalScrollbar import net.mullvad.mullvadvpn.compose.dialog.ShowDeviceRemovalDialog 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 @@ -51,9 +58,19 @@ private fun PreviewDeviceListScreen() { device = Device( id = "ID", - name = "Name", + name = "Powerful Hen", pubkey = ByteArray(10), - created = "2002-12-12" + created = "2022-12-17 17:12:00 PST" + ), + isLoading = false + ), + DeviceListItemUiState( + device = + Device( + id = "ID", + name = "Weak Ostrich", + pubkey = ByteArray(10), + created = "2021-02-06 19:21:41 PST" ), isLoading = false ) @@ -91,156 +108,178 @@ 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() - - 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) - ) + DeviceListHeader(state = state) - 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.mediumPadding) +@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) { + CircularProgressIndicator( + strokeWidth = Dimens.loadingSpinnerStrokeWidth, + color = MaterialTheme.colorScheme.onPrimary, + modifier = + Modifier.height(Dimens.loadingSpinnerSize).width(Dimens.loadingSpinnerSize) + ) + } else { + IconButton(onClick = onDeviceRemovalClicked) { + Icon( + painter = painterResource(id = R.drawable.icon_close), + contentDescription = "Remove", + ) + } + } + }, + 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.mediumPadding) + ) } }