diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/PackageActionMenu.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/PackageActionMenu.kt index 47ae6a11b..7fa03357b 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/PackageActionMenu.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/PackageActionMenu.kt @@ -16,11 +16,12 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width +import androidx.compose.foundation.selection.toggleable import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.Launch import androidx.compose.material.icons.rounded.Bolt -import androidx.compose.material.icons.rounded.DeleteOutline +import androidx.compose.material.icons.rounded.Delete import androidx.compose.material.icons.rounded.Info import androidx.compose.material.icons.rounded.Stop import androidx.compose.material3.ExperimentalMaterial3Api @@ -28,20 +29,22 @@ import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import org.matrix.vector.manager.Constants import org.matrix.vector.manager.ui.theme.LocalizedOverlay @@ -134,7 +137,14 @@ fun PackageActionSheet( } var confirmSoftReboot by remember { mutableStateOf(false) } - val scope = rememberCoroutineScope() + // Deliberately not `rememberCoroutineScope()`. Every action on this sheet dismisses it before + // it starts working, and the dismissal takes this composable out of the composition — which + // cancels the scope a composition remembered for it. The work launched into that scope then + // dies at the first `withContext` hop inside the daemon call, before the transaction is ever + // made, and dies quietly: no daemon call, no error branch, no snackbar, a button that did + // nothing. Worse, it is a race against the next frame rather than a reliable failure, so it + // reads as a flaky button. `appScope` belongs to the process and outlives the sheet. + val scope = ServiceLocator.appScope val daemon = ServiceLocator.daemon if (confirmSoftReboot) { @@ -148,7 +158,7 @@ fun PackageActionSheet( onClick = { confirmSoftReboot = false onDismiss() - scope.launch { + scope.launch(Dispatchers.Main) { daemon.softReboot().onFailure { Log.e(Constants.TAG, "actions: soft reboot request failed", it) } @@ -177,9 +187,11 @@ fun PackageActionSheet( // still open at their own height and nothing gains a useless drag. val sheetState = rememberModalBottomSheetState() + // `Dispatchers.Main` because [onResult] reaches a snackbar on the screen underneath, and + // because that is the thread the composition scope this replaces used to resume on. fun finish(block: suspend () -> PackageActionResult) { onDismiss() - scope.launch { onResult(block()) } + scope.launch(Dispatchers.Main) { onResult(block()) } } ModalBottomSheet(onDismissRequest = onDismiss, sheetState = sheetState) { @@ -361,7 +373,7 @@ LocalizedOverlay { if (isModule) { HorizontalDivider(Modifier.padding(horizontal = 24.dp, vertical = 4.dp)) ActionRow( - icon = Icons.Rounded.DeleteOutline, + icon = Icons.Rounded.Delete, title = stringResource(R.string.action_uninstall), tint = colors.error, ) { @@ -392,18 +404,31 @@ LocalizedOverlay { } /** - * One action, with its icon in a tinted disc. + * The one shape every row on this sheet takes: a glyph in a tinted disc, the verb, and — when it + * needs one — the sentence under it saying what the verb costs. * * The disc is what lets a destructive action look destructive: an error-red glyph on a bare row is - * easy to miss, the same glyph on a red disc is not. + * easy to miss, the same glyph on a red disc is not. Once one row carries it they all have to, or + * the bare one reads as a different kind of thing sitting in the same list — which is what the mute + * switch did while it was borrowing the generic [ToggleRow], a Material list item whose leading + * icon has no disc and whose text starts ten pixels to the left of every other row here. + * + * The measurements are chosen so that one column runs down the whole sheet: 24dp of margin, a 40dp + * disc and 20dp of gap put every title at 84dp, which is where the header puts the app's name over + * its 44dp icon and 16dp gap. + * + * [trailing] is for a row that carries state as well as an action, and the click behaviour comes in + * through [modifier] rather than as a callback: a switch row has to announce itself to a screen + * reader as a switch, not as a button, and only the caller knows which it is. */ @Composable -private fun ActionRow( +private fun ActionRowLayout( + modifier: Modifier, icon: ImageVector, title: String, - subtitle: String? = null, - tint: Color? = null, - onClick: () -> Unit, + subtitle: String?, + tint: Color?, + trailing: (@Composable () -> Unit)? = null, ) { val colors = MaterialTheme.colorScheme val accent = tint ?: colors.onSurfaceVariant @@ -411,7 +436,7 @@ private fun ActionRow( Row( modifier = Modifier.fillMaxWidth() - .clickable(onClick = onClick) + .then(modifier) .padding(horizontal = 24.dp, vertical = 12.dp), verticalAlignment = Alignment.CenterVertically, ) { @@ -422,7 +447,7 @@ private fun ActionRow( ) { Icon(icon, contentDescription = null, tint = accent, modifier = Modifier.size(22.dp)) } - Spacer(Modifier.width(18.dp)) + Spacer(Modifier.width(20.dp)) Column(Modifier.weight(1f)) { Text( text = title, @@ -437,9 +462,63 @@ private fun ActionRow( ) } } + if (trailing != null) { + Spacer(Modifier.width(12.dp)) + trailing() + } } } +/** + * One action. + * + * [onClick] is nullable because one row on this sheet is a statement rather than an action — "not + * in the store" — and a row that ripples under a thumb and then does nothing is a worse answer than + * one that visibly cannot be pressed. + */ +@Composable +private fun ActionRow( + icon: ImageVector, + title: String, + subtitle: String? = null, + tint: Color? = null, + onClick: (() -> Unit)?, +) { + ActionRowLayout( + modifier = if (onClick != null) Modifier.clickable(onClick = onClick) else Modifier, + icon = icon, + title = title, + subtitle = subtitle, + tint = tint, + ) +} + +/** One setting, in the same shape as the actions it sits among. */ +@Composable +private fun ActionToggleRow( + icon: ImageVector, + title: String, + checked: Boolean, + onCheckedChange: (Boolean) -> Unit, + subtitle: String? = null, +) { + ActionRowLayout( + modifier = + Modifier.toggleable( + value = checked, + role = Role.Switch, + onValueChange = onCheckedChange, + ), + icon = icon, + title = title, + subtitle = subtitle, + tint = null, + // The whole row is the target, and the switch itself takes no callback, so a tap on it + // cannot be counted twice. + trailing = { Switch(checked = checked, onCheckedChange = null) }, + ) +} + /** * What this module's update situation is, and the two things to do about it. * @@ -484,7 +563,7 @@ private fun ModuleUpdateSection( icon = Icons.Rounded.CloudOff, title = stringResource(R.string.action_not_in_store), subtitle = stringResource(R.string.action_not_in_store_summary), - onClick = {}, + onClick = null, ) HorizontalDivider(Modifier.padding(horizontal = 24.dp)) Spacer(Modifier.height(4.dp)) @@ -538,7 +617,7 @@ private fun ModuleUpdateSection( ) } - ToggleRow( + ActionToggleRow( title = stringResource(R.string.store_mute_updates), icon = Icons.Rounded.NotificationsOff, checked = packageName in muted, diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/SheetParts.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/SheetParts.kt index 3331ad94b..4888dc0de 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/SheetParts.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/components/SheetParts.kt @@ -14,6 +14,8 @@ import androidx.compose.foundation.selection.toggleable import androidx.compose.material3.Icon import androidx.compose.material3.LocalContentColor import androidx.compose.material3.ListItem +import androidx.compose.material3.ListItemColors +import androidx.compose.material3.ListItemDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text @@ -35,6 +37,19 @@ import androidx.compose.ui.unit.dp * wraps at another width. A new sheet inherits the pattern, and changing the pattern changes every * sheet at once. */ + +/** + * What a [ListItem] needs to be given to sit on a sheet. + * + * A list item's container defaults to `surface`; a `ModalBottomSheet` is drawn on + * `surfaceContainerLow`, which is a shade darker. On a screen those two agree, so the default looks + * right in the place a row is usually written and wrong the moment it is put in a sheet — a pale + * full-width band across the sheet, ending wherever the row ends. Transparent takes whatever it is + * placed on, so it is right in both, and it is what every list item in a sheet should be given. + */ +val sheetRowColors: ListItemColors + @Composable get() = ListItemDefaults.colors(containerColor = Color.Transparent) + @Composable fun SheetHeading(text: String, icon: ImageVector) { Row( @@ -105,6 +120,7 @@ fun ToggleRow( supportingContent = subtitle?.let { { Text(it) } }, leadingContent = { Icon(icon, contentDescription = null) }, trailingContent = { Switch(checked = checked, onCheckedChange = null) }, + colors = sheetRowColors, ) } @@ -167,5 +183,6 @@ fun SheetAction( leadingContent = { Icon(icon, contentDescription = null, tint = tint ?: LocalContentColor.current) }, + colors = sheetRowColors, ) } diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/logs/LogsScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/logs/LogsScreen.kt index a7a413b37..322ff1ac7 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/logs/LogsScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/logs/LogsScreen.kt @@ -110,6 +110,7 @@ import org.matrix.vector.manager.R import org.matrix.vector.manager.ui.theme.VectorLogLine import org.matrix.vector.manager.data.log.LogLevel import org.matrix.vector.manager.ui.components.PanelHeader +import org.matrix.vector.manager.ui.components.sheetRowColors import org.matrix.vector.manager.ui.components.SearchField import org.matrix.vector.manager.ui.theme.VectorMono @@ -721,6 +722,7 @@ LocalizedOverlay { trailingContent = { Switch(checked = enabled, onCheckedChange = { viewModel.setVerbose(it) }) }, + colors = sheetRowColors, ) HorizontalDivider(Modifier.padding(vertical = 4.dp)) @@ -730,6 +732,7 @@ LocalizedOverlay { headlineContent = { Text(stringResource(R.string.logs_save)) }, supportingContent = { Text(stringResource(R.string.logs_save_summary)) }, leadingContent = { Icon(Icons.Rounded.Save, contentDescription = null) }, + colors = sheetRowColors, ) ListItem( modifier = Modifier.clickable(onClick = onRotate), @@ -742,6 +745,7 @@ LocalizedOverlay { tint = MaterialTheme.colorScheme.error, ) }, + colors = sheetRowColors, ) } } diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ModulesScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ModulesScreen.kt index 62c97dc6f..bf4383307 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ModulesScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ModulesScreen.kt @@ -29,7 +29,7 @@ import androidx.compose.material.icons.rounded.SettingsBackupRestore import androidx.compose.material.icons.rounded.Block import androidx.compose.material.icons.rounded.Check import androidx.compose.material.icons.rounded.CheckCircle -import androidx.compose.material.icons.rounded.DeleteOutline +import androidx.compose.material.icons.rounded.Delete import androidx.compose.material.icons.rounded.Close import androidx.compose.material.icons.rounded.FilterList import androidx.compose.material.icons.rounded.Android @@ -101,6 +101,7 @@ import org.matrix.vector.manager.data.model.ReleaseAsset import org.matrix.vector.manager.data.model.StoreEntry import org.matrix.vector.manager.data.repository.ModuleUpdateQueue import org.matrix.vector.manager.ui.components.SheetHeading +import org.matrix.vector.manager.ui.components.sheetRowColors import org.matrix.vector.manager.ui.screens.repo.StoreChannel import org.matrix.vector.manager.ui.screens.repo.releasesOn import org.lsposed.lspd.ILSPManagerService @@ -428,7 +429,7 @@ fun ModulesScreen( if (confirmUninstall) { VectorAlertDialog( onDismissRequest = { confirmUninstall = false }, - icon = { Icon(Icons.Rounded.DeleteOutline, contentDescription = null) }, + icon = { Icon(Icons.Rounded.Delete, contentDescription = null) }, title = { Text(stringResource(R.string.modules_uninstall_title)) }, // Names the consequence rather than asking "are you sure". The backup on this screen // holds the enabled flag and the scope; the module's own stored settings go with it @@ -526,7 +527,7 @@ private fun SelectionBar( SelectionAction(Icons.Rounded.Block, R.string.modules_batch_disable, onDisable) SelectionAction(Icons.Rounded.SaveAlt, R.string.modules_backup, onBackup) SelectionAction( - Icons.Rounded.DeleteOutline, + Icons.Rounded.Delete, R.string.action_uninstall, onUninstall, tint = MaterialTheme.colorScheme.error, @@ -1402,6 +1403,7 @@ private fun ModuleUpdatesSheet( ) } }, + colors = sheetRowColors, ) } } diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt index f7f0d2d21..4e7ab0572 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import org.matrix.vector.manager.ui.components.ConfirmInstall import org.matrix.vector.manager.ui.components.ToggleRow import org.matrix.vector.manager.ui.components.SheetHeading +import org.matrix.vector.manager.ui.components.sheetRowColors import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.material.icons.rounded.Tune import androidx.compose.material.icons.rounded.NotificationsOff @@ -881,6 +882,7 @@ LocalizedOverlay { } Text(listOfNotNull(size, downloads).joinToString(" · ")) }, + colors = sheetRowColors, ) } Spacer(Modifier.navigationBarsPadding().height(16.dp)) diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/update/FrameworkUpdateScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/update/FrameworkUpdateScreen.kt index 98eee67d5..01aef5fa6 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/update/FrameworkUpdateScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/update/FrameworkUpdateScreen.kt @@ -6,6 +6,7 @@ import androidx.compose.foundation.clickable import org.matrix.vector.manager.ui.theme.currentLocale import org.matrix.vector.manager.ui.theme.LocalizedOverlay import org.matrix.vector.manager.ui.components.SheetHeading +import org.matrix.vector.manager.ui.components.sheetRowColors import org.matrix.vector.manager.data.repository.ReleaseDirection import org.matrix.vector.manager.data.github.FrameworkRelease import java.util.Date @@ -645,6 +646,7 @@ private fun VersionsSheet( ) } }, + colors = sheetRowColors, ) } }