From 64e2a2cc9a33f163ff1c892399f22f74f52633ae 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 | 138 -------- .../mullvadvpn/compose/component/TopBar.kt | 1 + .../compose/screen/DeviceListScreen.kt | 310 ++++++++++-------- 3 files changed, 169 insertions(+), 280 deletions(-) delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/List.kt 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 4b774bfe8fcb..000000000000 --- 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 bd50d45c8621..6db5d6473599 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 7064d82b9ee4..1617c1fb7a5a 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) + ) } }