fix: missing remove device button [WPB-8819] (#3110)

Co-authored-by: Yamil Medina <[email protected]>
wireapp · Jun 24, 2024 · fc4e272 · fc4e272
1 parent 6ffc698
commit fc4e272
Show file tree

Hide file tree

Showing 13 changed files with 310 additions and 109 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt
@@ -18,29 +18,25 @@
 
 package com.wire.android.ui.authentication.devices
 
-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.ColumnScope
 import androidx.compose.foundation.layout.PaddingValues
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
 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.wrapContentWidth
 import androidx.compose.foundation.shape.RoundedCornerShape
-import androidx.compose.material.icons.Icons
-import androidx.compose.material.icons.filled.ChevronRight
 import androidx.compose.material3.Icon
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.vector.ImageVector
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.res.painterResource
@@ -52,7 +48,7 @@ import com.wire.android.BuildConfig
 import com.wire.android.R
 import com.wire.android.ui.authentication.devices.model.Device
 import com.wire.android.ui.authentication.devices.model.lastActiveDescription
-import com.wire.android.ui.common.Icon
+import com.wire.android.ui.common.ArrowRightIcon
 import com.wire.android.ui.common.MLSVerificationIcon
 import com.wire.android.ui.common.ProteusVerifiedIcon
 import com.wire.android.ui.common.button.WireSecondaryButton
@@ -62,9 +58,9 @@ import com.wire.android.ui.theme.WireTheme
 import com.wire.android.ui.theme.wireColorScheme
 import com.wire.android.ui.theme.wireDimensions
 import com.wire.android.ui.theme.wireTypography
+import com.wire.android.util.deviceDateTimeFormat
 import com.wire.android.util.extension.formatAsFingerPrint
 import com.wire.android.util.extension.formatAsString
-import com.wire.android.util.deviceDateTimeFormat
 import com.wire.android.util.ui.PreviewMultipleThemes
 import com.wire.android.util.ui.UIText
 
@@ -73,41 +69,41 @@ fun DeviceItem(
     device: Device,
     placeholder: Boolean,
     shouldShowVerifyLabel: Boolean,
+    icon: @Composable (() -> Unit),
+    modifier: Modifier = Modifier,
     isCurrentClient: Boolean = false,
     shouldShowE2EIInfo: Boolean = false,
-    background: Color? = null,
-    icon: @Composable (() -> Unit),
     isWholeItemClickable: Boolean = false,
     onClickAction: ((Device) -> Unit)? = null
 ) {
     DeviceItemContent(
         device = device,
         placeholder = placeholder,
-        background = background,
         icon = icon,
         onClickAction = onClickAction,
         isWholeItemClickable = isWholeItemClickable,
         shouldShowVerifyLabel = shouldShowVerifyLabel,
         isCurrentClient = isCurrentClient,
-        shouldShowE2EIInfo = shouldShowE2EIInfo
+        shouldShowE2EIInfo = shouldShowE2EIInfo,
+        modifier = modifier,
     )
 }
 
 @Composable
 private fun DeviceItemContent(
     device: Device,
     placeholder: Boolean,
-    background: Color? = null,
     icon: @Composable (() -> Unit),
     onClickAction: ((Device) -> Unit)?,
     isWholeItemClickable: Boolean,
     shouldShowVerifyLabel: Boolean,
     isCurrentClient: Boolean,
-    shouldShowE2EIInfo: Boolean
+    shouldShowE2EIInfo: Boolean,
+    modifier: Modifier = Modifier,
 ) {
     Row(
         verticalAlignment = Alignment.Top,
-        modifier = (if (background != null) Modifier.background(color = background) else Modifier)
+        modifier = modifier
             .clickable(enabled = isWholeItemClickable) {
                 if (isWholeItemClickable) {
                     onClickAction?.invoke(device)
@@ -129,12 +125,16 @@ private fun DeviceItemContent(
                 modifier = Modifier
                     .padding(start = MaterialTheme.wireDimensions.removeDeviceItemPadding)
                     .weight(1f)
-            ) { DeviceItemTexts(device, placeholder, shouldShowVerifyLabel, isCurrentClient, shouldShowE2EIInfo) }
+            ) {
+                DeviceItemTexts(device, placeholder, shouldShowVerifyLabel, isCurrentClient, shouldShowE2EIInfo)
+            }
         }
         if (!placeholder) {
             if (onClickAction != null && !isWholeItemClickable) {
                 WireSecondaryButton(
-                    modifier = Modifier.testTag("remove device button"),
+                    modifier = Modifier
+                        .padding(end = MaterialTheme.wireDimensions.removeDeviceItemPadding)
+                        .testTag("remove device button"),
                     onClick = { onClickAction(device) },
                     leadingIcon = icon,
                     fillMaxWidth = false,
@@ -143,11 +143,17 @@ private fun DeviceItemContent(
                     shape = RoundedCornerShape(size = MaterialTheme.wireDimensions.buttonSmallCornerSize),
                     contentPadding = PaddingValues(0.dp),
                     colors = wireSecondaryButtonColors().copy(
-                        enabled = background ?: MaterialTheme.wireColorScheme.secondaryButtonEnabled
+                        enabled = MaterialTheme.wireColorScheme.secondaryButtonEnabled
                     )
                 )
             } else {
-                Box(modifier = Modifier.padding(MaterialTheme.wireDimensions.removeDeviceItemPadding)) {
+                Box(
+                    modifier = Modifier
+                        .padding(
+                            top = MaterialTheme.wireDimensions.removeDeviceItemPadding,
+                            end = MaterialTheme.wireDimensions.removeDeviceItemPadding,
+                        )
+                ) {
                     icon()
                 }
             }
@@ -156,13 +162,13 @@ private fun DeviceItemContent(
 }
 
 @Composable
-private fun DeviceItemTexts(
+private fun ColumnScope.DeviceItemTexts(
     device: Device,
     placeholder: Boolean,
     shouldShowVerifyLabel: Boolean,
     isCurrentClient: Boolean = false,
     shouldShowE2EIInfo: Boolean = false,
-    isDebug: Boolean = BuildConfig.DEBUG
+    isDebug: Boolean = BuildConfig.DEBUG,
 ) {
     val displayZombieIndicator = remember {
         if (isDebug) {
@@ -172,7 +178,11 @@ private fun DeviceItemTexts(
         }
     }
 
-    Row(Modifier.fillMaxWidth()) {
+    Row(
+        modifier = Modifier
+            .fillMaxWidth()
+            .shimmerPlaceholder(visible = placeholder)
+    ) {
         Text(
             style = MaterialTheme.wireTypography.body02,
             color = MaterialTheme.wireColorScheme.onBackground,
@@ -185,7 +195,6 @@ private fun DeviceItemTexts(
             if (shouldShowE2EIInfo) {
                 MLSVerificationIcon(device.e2eiCertificate?.status)
             }
-            Spacer(modifier = Modifier.width(MaterialTheme.wireDimensions.spacing8x))
             if (device.isVerifiedProteus && !isCurrentClient) {
                 ProteusVerifiedIcon(
                     Modifier
@@ -202,7 +211,6 @@ private fun DeviceItemTexts(
             text = "this client is invalid",
             modifier = Modifier
                 .fillMaxWidth()
-                .shimmerPlaceholder(visible = placeholder)
         )
     }
 
@@ -249,6 +257,7 @@ private fun DeviceItemTexts(
         style = MaterialTheme.wireTypography.subline01,
         color = MaterialTheme.wireColorScheme.labelText,
         text = proteusDetails,
+        minLines = 2,
         modifier = Modifier
             .fillMaxWidth()
             .shimmerPlaceholder(visible = placeholder)
@@ -260,13 +269,12 @@ private fun DeviceItemTexts(
 fun PreviewDeviceItemWithActionIcon() {
     WireTheme {
         DeviceItem(
-            device = Device(name = UIText.DynamicString("name"), isVerifiedProteus = true),
+            device = Device(name = UIText.DynamicString("Name"), isVerifiedProteus = true, registrationTime = "2024-01-01T12:00:00.000Z"),
             placeholder = false,
             shouldShowVerifyLabel = true,
             isCurrentClient = true,
             shouldShowE2EIInfo = true,
-            background = null,
-            { Icon(painter = painterResource(id = R.drawable.ic_remove), contentDescription = "") }
+            icon = { Icon(painter = painterResource(id = R.drawable.ic_remove), contentDescription = "") }
         ) {}
     }
 }
@@ -276,12 +284,11 @@ fun PreviewDeviceItemWithActionIcon() {
 fun PreviewDeviceItem() {
     WireTheme {
         DeviceItem(
-            device = Device(name = UIText.DynamicString("name"), isVerifiedProteus = true),
+            device = Device(name = UIText.DynamicString("Name"), isVerifiedProteus = true, registrationTime = "2024-01-01T12:00:00.000Z"),
             placeholder = false,
             shouldShowVerifyLabel = true,
-            background = null,
             isWholeItemClickable = true,
-            icon = Icons.Filled.ChevronRight.Icon()
+            icon = { ArrowRightIcon() }
         ) {}
     }
 }
diff --git a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt
@@ -33,6 +33,7 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
+import androidx.compose.ui.window.DialogProperties
 import androidx.hilt.navigation.compose.hiltViewModel
 import com.ramcosta.composedestinations.annotation.Destination
 import com.ramcosta.composedestinations.annotation.RootNavGraph
@@ -63,6 +64,7 @@ import com.wire.android.ui.destinations.InitialSyncScreenDestination
 import com.wire.android.ui.theme.WireTheme
 import com.wire.android.util.dialogErrorStrings
 import com.wire.android.util.ui.PreviewMultipleThemes
+import com.wire.kalium.logic.data.conversation.ClientId
 
 @RootNavGraph
 @Destination(
@@ -96,6 +98,25 @@ fun RemoveDeviceScreen(
         onCancelLoginClicked = { clearSessionViewModel.onCancelLoginClicked(NavigationSwitchAccountActions(navigator::navigate)) },
         onProceedLoginClicked = clearSessionViewModel::onProceedLoginClicked
     )
+
+    if (viewModel.state.error is RemoveDeviceError.InitError) {
+        WireDialog(
+            properties = DialogProperties(dismissOnBackPress = false, dismissOnClickOutside = false, usePlatformDefaultWidth = false),
+            title = stringResource(id = R.string.label_general_error),
+            text = stringResource(id = R.string.devices_loading_error),
+            onDismiss = viewModel::clearDeleteClientError,
+            dismissButtonProperties = WireDialogButtonProperties(
+                onClick = clearSessionViewModel::onBackButtonClicked,
+                text = stringResource(id = R.string.label_cancel),
+                type = WireDialogButtonType.Secondary,
+            ),
+            optionButton1Properties = WireDialogButtonProperties(
+                onClick = viewModel::retryFetch,
+                text = stringResource(id = R.string.label_retry),
+                type = WireDialogButtonType.Primary,
+            )
+        )
+    }
 }
 
 @Composable
@@ -118,12 +139,8 @@ private fun RemoveDeviceContent(
     val cancelLoginDialogState = rememberVisibilityState<CancelLoginDialogState>()
     CancelLoginDialogContent(
         dialogState = cancelLoginDialogState,
-        onActionButtonClicked = {
-            onCancelLoginClicked()
-        },
-        onProceedButtonClicked = {
-            onProceedLoginClicked()
-        }
+        onActionButtonClicked = onCancelLoginClicked,
+        onProceedButtonClicked = onProceedLoginClicked,
     )
     if (clearSessionState.showCancelLoginDialog) {
         cancelLoginDialogState.show(
@@ -144,10 +161,15 @@ private fun RemoveDeviceContent(
         }
     ) { internalPadding ->
         Box(modifier = Modifier.padding(internalPadding)) {
-            when (state.isLoadingClientsList) {
-                true -> RemoveDeviceItemsList(lazyListState, List(10) { Device() }, true, onItemClicked)
-                false -> RemoveDeviceItemsList(lazyListState, state.deviceList, false, onItemClicked)
-            }
+            RemoveDeviceItemsList(
+                lazyListState = lazyListState,
+                items = when (state.isLoadingClientsList) {
+                    true -> List(4) { Device(clientId = ClientId("placeholder_$it")) }
+                    false -> state.deviceList
+                },
+                placeholders = state.isLoadingClientsList,
+                onItemClicked = onItemClicked
+            )
         }
         // TODO handle list loading errors
         if (!state.isLoadingClientsList && state.removeDeviceDialogState is RemoveDeviceDialogState.Visible) {

diff --git a/...rc/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt b/...rc/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt
@@ -84,12 +84,14 @@ class RemoveDeviceViewModel @Inject constructor(
         viewModelScope.launch {
             state = state.copy(isLoadingClientsList = true)
             val selfClientsResult = fetchSelfClientsFromRemote()
-            if (selfClientsResult is SelfClientsResult.Success) {
-                state = state.copy(
+            state = if (selfClientsResult is SelfClientsResult.Success) {
+                state.copy(
                     isLoadingClientsList = false,
                     deviceList = selfClientsResult.clients.filter { it.type == ClientType.Permanent }.map { Device(it) },
                     removeDeviceDialogState = RemoveDeviceDialogState.Hidden
                 )
+            } else {
+                state.copy(isLoadingClientsList = false, error = RemoveDeviceError.InitError)
             }
         }
     }
@@ -103,6 +105,11 @@ class RemoveDeviceViewModel @Inject constructor(
         updateStateIfDialogVisible { state.copy(error = RemoveDeviceError.None) }
     }
 
+    fun retryFetch() {
+        state = state.copy(isLoadingClientsList = true, error = RemoveDeviceError.None)
+        loadClientsList()
+    }
+
     fun onItemClicked(device: Device, onCompleted: (initialSyncCompleted: Boolean, isE2EIRequired: Boolean) -> Unit) {
         viewModelScope.launch {
             val isPasswordRequired: Boolean = when (val passwordRequiredResult = isPasswordRequired()) {

diff --git a/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt
@@ -197,7 +197,7 @@ class DeviceDetailsViewModel @Inject constructor(
                             device = state.device.updateFromClient(result.client),
                             isCurrentDevice = result.isCurrentClient,
                             removeDeviceDialogState = RemoveDeviceDialogState.Hidden,
-                            canBeRemoved = !result.isCurrentClient && isSelfClient && result.client.type == ClientType.Permanent,
+                            canBeRemoved = !result.isCurrentClient && isSelfClient && result.client.type != ClientType.LegalHold,
                         )
                     }
                 }