Improve policy editing UI/UX

Signed-off-by: Iván Ávalos <avalos@disroot.org>

5 files changed, 113 insertions, 92 deletions

diff --git a/anastasis/src/main/java/net/taler/anastasis/ui/backup/ReviewPoliciesScreen.kt b/anastasis/src/main/java/net/taler/anastasis/ui/backup/ReviewPoliciesScreen.kt
index 38853c7..04c44d8 100644
--- a/anastasis/src/main/java/net/taler/anastasis/ui/backup/ReviewPoliciesScreen.kt
+++ b/anastasis/src/main/java/net/taler/anastasis/ui/backup/ReviewPoliciesScreen.kt
@@ -17,7 +17,6 @@
 package net.taler.anastasis.ui.backup
 
 import androidx.compose.foundation.layout.Arrangement
-import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
@@ -26,12 +25,12 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.lazy.LazyColumn
-import androidx.compose.material.DropdownMenuItem
 import androidx.compose.material.IconButton
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Add
-import androidx.compose.material.icons.filled.MoreVert
-import androidx.compose.material3.DropdownMenu
+import androidx.compose.material.icons.filled.Delete
+import androidx.compose.material.icons.filled.Edit
+import androidx.compose.material.icons.filled.EditOff
 import androidx.compose.material3.ElevatedCard
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.FloatingActionButton
@@ -57,6 +56,7 @@ import net.taler.anastasis.models.AuthenticationProviderStatus
 import net.taler.anastasis.models.Policy
 import net.taler.anastasis.models.ReducerState
 import net.taler.anastasis.ui.dialogs.EditPolicyDialog
+import net.taler.anastasis.ui.forms.EditPolicyForm
 import net.taler.anastasis.ui.reusable.pages.WizardPage
 import net.taler.anastasis.ui.theme.LocalSpacing
 import net.taler.anastasis.viewmodels.ReducerViewModel
@@ -141,9 +141,7 @@ fun ReviewPoliciesScreen(
                         providers = providers,
                         index = index,
                         onEdit = {
-                            editingPolicy = policy
-                            editingPolicyIndex = index
-                            showEditDialog = true
+                            viewModel.reducerManager.updatePolicy(index, it)
                         },
                     ) {
                         viewModel.reducerManager.deletePolicy(index)
@@ -158,17 +156,18 @@ fun ReviewPoliciesScreen(
 fun PolicyCard(
     modifier: Modifier = Modifier,
     methods: List<AuthMethod>,
-    providers: Map<String, AuthenticationProviderStatus>,
+    providers: Map<String, AuthenticationProviderStatus.Ok>,
     policy: Policy,
     index: Int,
-    onEdit: () -> Unit,
+    onEdit: (policy: Policy) -> Unit,
     onDelete: () -> Unit,
 ) {
     ElevatedCard(
         modifier = modifier,
     ) {
+        var editing by remember{ mutableStateOf(false) }
+
         Column(modifier = Modifier.padding(LocalSpacing.current.medium)) {
-            var expanded by remember { mutableStateOf(false) }
             Row(
                 horizontalArrangement = Arrangement.SpaceAround,
                 verticalAlignment = Alignment.CenterVertically,
@@ -178,42 +177,42 @@ fun PolicyCard(
                     style = MaterialTheme.typography.titleLarge,
                 )
                 Spacer(Modifier.weight(1f))
-                Box {
-                    IconButton(onClick = { expanded = true }) {
-                        Icon(
-                            Icons.Default.MoreVert,
-                            contentDescription = stringResource(R.string.menu),
-                            tint = MaterialTheme.colorScheme.onBackground,
-                        )
-                        DropdownMenu(
-                            expanded = expanded,
-                            onDismissRequest = { expanded = false },
-                        ) {
-                            DropdownMenuItem(onClick = {
-                                onEdit()
-                                expanded = false
-                            }) {
-                                Text(stringResource(R.string.edit))
-                            }
-                            DropdownMenuItem(onClick = onDelete) {
-                                Text(stringResource(R.string.delete))
-                            }
-                        }
-                    }
+                IconButton(onClick = { editing = !editing }) {
+                    Icon(
+                        if (editing) Icons.Default.EditOff else Icons.Default.Edit,
+                        contentDescription = if (editing)
+                            stringResource(R.string.cancel) else stringResource(R.string.edit),
+                    )
+                }
+                IconButton(onClick = onDelete) {
+                    Icon(
+                        Icons.Default.Delete,
+                        contentDescription = stringResource(R.string.delete),
+                    )
                 }
             }
-            Column {
-                policy.methods.forEach { m ->
-                    val method = methods[m.authenticationMethod]
-                    val provider = providers[m.provider] as? AuthenticationProviderStatus.Ok
-                    if (provider != null) {
-                        PolicyMethodCard(
-                            modifier = Modifier
-                                .padding(top = LocalSpacing.current.small)
-                                .fillMaxWidth(),
-                            method = method,
-                            provider = provider,
-                        )
+
+            if (editing) {
+                EditPolicyForm(
+                    policy = policy,
+                    methods = methods,
+                    providers = providers,
+                    onPolicyEdited = { onEdit(it) },
+                )
+            } else {
+                Column {
+                    policy.methods.forEach { m ->
+                        val method = methods[m.authenticationMethod]
+                        val provider = providers[m.provider]
+                        if (provider != null) {
+                            PolicyMethodCard(
+                                modifier = Modifier
+                                    .padding(top = LocalSpacing.current.small)
+                                    .fillMaxWidth(),
+                                method = method,
+                                provider = provider,
+                            )
+                        }
                     }
                 }
             }
@@ -244,14 +243,14 @@ fun PolicyMethodCard(
                 Text(method.instructions, style = MaterialTheme.typography.labelLarge)
                 Spacer(Modifier.height(LocalSpacing.current.small))
                 Text(
-                    stringResource(R.string.provider),
+                    stringResource(R.string.provided_by, provider.businessName),
                     style = MaterialTheme.typography.labelMedium,
                     fontWeight = FontWeight.Bold,
                 )
-                Text(
-                    provider.businessName,
-                    style = MaterialTheme.typography.labelMedium,
-                )
+//                Text(
+//                    provider.businessName,
+//                    style = MaterialTheme.typography.labelMedium,
+//                )
             }
         }
     }
diff --git a/anastasis/src/main/java/net/taler/anastasis/ui/dialogs/EditPolicyDialog.kt b/anastasis/src/main/java/net/taler/anastasis/ui/dialogs/EditPolicyDialog.kt
index e9f0e2b..8ec3662 100644
--- a/anastasis/src/main/java/net/taler/anastasis/ui/dialogs/EditPolicyDialog.kt
+++ b/anastasis/src/main/java/net/taler/anastasis/ui/dialogs/EditPolicyDialog.kt
@@ -45,8 +45,7 @@ fun EditPolicyDialog(
 
     AlertDialog(
         onDismissRequest = onCancel,
-        title = { Text(stringResource(if (policy != null)
-            R.string.edit_policy else R.string.add_policy)) },
+        title = { Text(stringResource(R.string.add_policy)) },
         text = {
             EditPolicyForm(
                 modifier = Modifier.fillMaxWidth(),
@@ -69,8 +68,7 @@ fun EditPolicyDialog(
             TextButton(onClick = {
                 localPolicy?.let { onPolicyEdited(it) }
             }) {
-                Text(stringResource(if (policy != null)
-                    R.string.edit else R.string.add))
+                Text(stringResource(R.string.add))
             }
         }
     )
diff --git a/anastasis/src/main/java/net/taler/anastasis/ui/forms/EditPolicyForm.kt b/anastasis/src/main/java/net/taler/anastasis/ui/forms/EditPolicyForm.kt
index 4cc3661..89309f8 100644
--- a/anastasis/src/main/java/net/taler/anastasis/ui/forms/EditPolicyForm.kt
+++ b/anastasis/src/main/java/net/taler/anastasis/ui/forms/EditPolicyForm.kt
@@ -16,16 +16,18 @@
 
 package net.taler.anastasis.ui.forms
 
+import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxWidth
-import androidx.compose.foundation.layout.padding
-import androidx.compose.foundation.lazy.LazyColumn
+import androidx.compose.foundation.layout.height
 import androidx.compose.material3.Checkbox
 import androidx.compose.material3.Icon
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
+import net.taler.anastasis.R
 import net.taler.anastasis.models.AuthMethod
 import net.taler.anastasis.models.AuthenticationProviderStatus
 import net.taler.anastasis.models.Policy
@@ -42,25 +44,25 @@ fun EditPolicyForm(
 ) {
     val localPolicy = policy ?: Policy(methods = listOf())
     val localMethods = localPolicy.methods.associateBy { it.authenticationMethod }
-    val submitLocalMethods = { it: Map<Int, Policy.PolicyMethod> ->
+    val submitLocalMethods = { it: MutableMap<Int, Policy.PolicyMethod>.() -> Unit ->
         onPolicyEdited(
             localPolicy.copy(
-                methods = it.flatMap { entry ->
+                methods = localMethods.toMutableMap().apply(it).flatMap { entry ->
                     listOf(entry.value)
-                }
+                },
             )
         )
     }
 
-    LazyColumn(
+    Column(
         modifier = modifier,
     ) {
-        items(count = methods.size) { index ->
-            val method = methods[index]
+        methods.forEachIndexed { index, method ->
             // Get only the providers that support this method type
             val methodProviders = providers.filterValues { provider ->
                 method.type in provider.methods.map { it.type }
-            }.keys.toList()
+            }
+            val providerUrls = methodProviders.keys.toList()
             val selectedProvider = localMethods[index]?.provider
             val checked = selectedProvider != null
             Row(
@@ -68,49 +70,64 @@ fun EditPolicyForm(
                 verticalAlignment = Alignment.CenterVertically,
             ) {
                 Checkbox(
-                    enabled = checked,
+                    // enabled = checked,
                     checked = checked,
                     onCheckedChange = {
-                        if (it) selectedProvider?.let { prov ->
-                            submitLocalMethods(
-                                localMethods.toMutableMap().apply {
+                        if (it) {
+                            selectedProvider?.let { prov ->
+                                submitLocalMethods {
                                     this[index] = Policy.PolicyMethod(
                                         authenticationMethod = index,
                                         provider = prov,
                                     )
                                 }
-                            )
-                        } else {
-                            submitLocalMethods(
-                                localMethods.toMutableMap().apply {
-                                    remove(index)
+                            } ?: run {
+                                if (providerUrls.isNotEmpty()) {
+                                    submitLocalMethods {
+                                        this[index] = Policy.PolicyMethod(
+                                            authenticationMethod = index,
+                                            provider = providerUrls.first(),
+                                        )
+                                    }
                                 }
-                            )
+                            }
+                        } else {
+                            submitLocalMethods {
+                                remove(index)
+                            }
                         }
                     },
                 )
-                DropdownTextField(
-                    modifier = Modifier.padding(bottom = LocalSpacing.current.small),
-                    label = method.instructions,
-                    leadingIcon = {
-                        Icon(
-                            method.type.icon,
-                            contentDescription = stringResource(method.type.nameRes),
-                        )
-                    },
-                    selectedIndex = selectedProvider?.let{ methodProviders.indexOf(it) },
-                    options = methodProviders,
-                    onOptionSelected = {
-                        submitLocalMethods(
-                            localMethods.toMutableMap().apply {
+                Column {
+                    Spacer(Modifier.height(LocalSpacing.current.small))
+                    DropdownTextField(
+                        modifier = Modifier
+                            .fillMaxWidth(),
+                        label = method.instructions,
+                        enabled = checked,
+                        supportingText = selectedProvider?.let {
+                            providers[it]?.businessName
+                        }?.let {
+                            stringResource(R.string.provided_by, it)
+                        } ?: stringResource(R.string.disabled),
+                        leadingIcon = {
+                            Icon(
+                                method.type.icon,
+                                contentDescription = stringResource(method.type.nameRes),
+                            )
+                        },
+                        selectedIndex = selectedProvider?.let { providerUrls.indexOf(it) },
+                        options = providerUrls,
+                        onOptionSelected = {
+                            submitLocalMethods {
                                 this[index] = Policy.PolicyMethod(
                                     authenticationMethod = index,
-                                    provider = methodProviders[it],
+                                    provider = providerUrls[it],
                                 )
                             }
-                        )
-                    },
-                )
+                        },
+                    )
+                }
             }
         }
     }
diff --git a/anastasis/src/main/java/net/taler/anastasis/ui/reusable/components/DropdownTextField.kt b/anastasis/src/main/java/net/taler/anastasis/ui/reusable/components/DropdownTextField.kt
index cbed1e9..fc5bb31 100644
--- a/anastasis/src/main/java/net/taler/anastasis/ui/reusable/components/DropdownTextField.kt
+++ b/anastasis/src/main/java/net/taler/anastasis/ui/reusable/components/DropdownTextField.kt
@@ -45,7 +45,9 @@ import androidx.compose.ui.unit.toSize
 fun DropdownTextField(
     modifier: Modifier = Modifier,
     label: String,
+    supportingText: String? = null,
     leadingIcon: (@Composable () -> Unit)? = null,
+    enabled: Boolean = true,
     selectedIndex: Int? = null,
     options: List<String>,
     onOptionSelected: (index: Int) -> Unit,
@@ -64,12 +66,16 @@ fun DropdownTextField(
                 },
             readOnly = true,
             leadingIcon = leadingIcon,
+            enabled = enabled,
             value = if (selectedIndex != null) options[selectedIndex] else "",
+            supportingText = { Text(supportingText ?: "") },
             onValueChange = {},
             singleLine = true,
             label = { Text(label) },
             trailingIcon = {
-                Box(modifier = Modifier.clickable { expanded = !expanded }) {
+                Box(modifier = Modifier.clickable {
+                    if (enabled) expanded = !expanded
+                }) {
                     ExposedDropdownMenuDefaults.TrailingIcon(
                         expanded = expanded,
                     )
diff --git a/anastasis/src/main/res/values/strings.xml b/anastasis/src/main/res/values/strings.xml
index 51bcd53..cf7054d 100644
--- a/anastasis/src/main/res/values/strings.xml
+++ b/anastasis/src/main/res/values/strings.xml
@@ -21,7 +21,8 @@
     <string name="question">Security question</string>
     <string name="answer">Answer</string>
     <string name="policy_n">Policy %1$d</string>
-    <string name="provider">Provider</string>
+    <string name="provided_by">Provided by %1$s</string>
+    <string name="disabled">Disabled</string>
     <string name="unknown">Unknown</string>
 
     <!-- Common -->