From e81737151026538a0a99d3a9a6e501a96212f2de Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Thu, 30 Jul 2026 13:47:28 +0200 Subject: [PATCH 1/2] Stop the long-press sheet cancelling its own actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every row on PackageActionSheet goes through finish(), which dismisses the sheet and then launches the work on rememberCoroutineScope(). The dismissal sets menuOpen = false, the sheet leaves the composition on the next frame, and a scope the composition remembered is cancelled when it does — so the coroutine dies at the first withContext hop inside DaemonClient, usually before the binder transaction is made. Nothing is logged, onResult never runs, and the button did nothing. Whether the work beats the frame is a race, which is why it worked often enough to look flaky rather than broken. The daemon was never involved. In the reporter's logs every start that reached the ActivityManager returned 0, and the manager logged nothing at all across two minutes of pressing — neither the "refused by the activity manager" line nor the "row had resolved a target" one, and a real failure prints one of them. The sheet's actions now run on ServiceLocator.appScope, which belongs to the process, with Dispatchers.Main to keep onResult on the thread the composition scope resumed on. This was never specific to "Open companion app": app info, force stop, re-optimize, uninstall and the framework's soft reboot were all launched the same way. The Scope screen's own companion button was unaffected — it goes through ScopeViewModel on viewModelScope. Fixes #810 --- .../manager/ui/components/PackageActionMenu.kt | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) 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..0edeb6ccd 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 @@ -33,7 +33,6 @@ 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 @@ -42,6 +41,7 @@ import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource 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 +134,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 +155,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 +184,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) { From 0cc3a31c410d42c2edda56e59c3208b4bccdbc39 Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Thu, 30 Jul 2026 13:48:04 +0200 Subject: [PATCH 2/2] Make every row on the long-press sheet belong to one list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things were off on that sheet, and each came from a row borrowing its shape from somewhere else. A Material list item paints its container `surface`, while a ModalBottomSheet is drawn on `surfaceContainerLow`, 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. Every list item on a sheet is now given the transparent `sheetRowColors`, which takes whatever it is placed on: the mute switch here, the log settings sheet, the batch update sheet, the store's asset picker and the framework versions sheet. ScopeScreen's AppRow had already worked this out for itself and nothing had carried it across. The mute switch was also the only row here built from the generic ToggleRow, so its icon had no disc and its title started seventy pixels to the left of every other row. It now takes the sheet's own shape, factored out of ActionRow as ActionRowLayout with a trailing slot, with the click behaviour arriving through the modifier so a switch row can still announce itself to a screen reader as a switch rather than as a button. The gap after the disc goes from 18dp to 20dp, which puts every title on 84dp — where the header already puts the app's name over its 44dp icon. Uninstall was drawn with DeleteOutline, a stroked glyph among filled ones, here and again in the module list's selection bar beside CheckCircle and SaveAlt. One verb, one glyph: Delete in all three places. Last, "not in the store" is a statement rather than an action, and it rippled under a thumb and then did nothing. ActionRow's onClick is nullable now and that row passes null. --- .../ui/components/PackageActionMenu.kt | 94 ++++++++++++++++--- .../manager/ui/components/SheetParts.kt | 17 ++++ .../manager/ui/screens/logs/LogsScreen.kt | 4 + .../ui/screens/modules/ModulesScreen.kt | 8 +- .../ui/screens/repo/RepoDetailsScreen.kt | 2 + .../screens/update/FrameworkUpdateScreen.kt | 2 + 6 files changed, 112 insertions(+), 15 deletions(-) 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 0edeb6ccd..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,6 +29,7 @@ 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 @@ -39,6 +41,7 @@ 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 @@ -370,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, ) { @@ -401,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 @@ -420,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, ) { @@ -431,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, @@ -446,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. * @@ -493,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)) @@ -547,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, ) } }