From 043d725599920070a0e42154d3c212d57ca6d49e Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Tue, 8 Sep 2026 02:01:39 +0200 Subject: [PATCH] feat: draw the expressive loading indicator, and stop shouting the sign-out button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5, second step, of docs/material-design-conformance.md. Two smaller pieces, and a correction to the plan. **41 loading states stopped being a gold spinner.** `LoadingDataIndicator` wraps every wait in the app, and it drew a `CircularProgressIndicator` hardcoded to 80dp in `colorScheme.secondary` -- the brand gold, which reads as a warning rather than as a wait, on a component that has a size of its own. It now draws `LoadingIndicator`, which is M3's component for an indeterminate wait with no progress to report and the one `MaterialExpressiveTheme` expects to be paired with. One wrapper changed; 41 call sites follow. **The profile screen had two maximum-emphasis buttons, and one of them was Sign out.** Seven actions in one list: five `TextButton`s (edit profile, key packages, change account, profile keys, network relays) and two filled `Button`s. A filled button is M3's highest emphasis and is meant for one action per screen, so this was two competing primaries -- and the more prominent of the pair was the list's most destructive item. Sharing is now `FilledTonalButton`: it is the useful action, at medium emphasis rather than maximum. Signing out is a `TextButton` in the error colour, which is not a new pattern -- it is how leaving and deleting a group are already treated in `ChatRoomDetailScreen`. Screenshot verified on emulator-5554: one tonal button, one red text button, five plain ones, and a hierarchy a reader can follow. **The plan was wrong about disabled FABs, and the code was right.** It said five screens should stop hand-computing a container colour from a `can…` flag and pass `enabled` instead. **No `FloatingActionButton` overload in material3 1.10 takes `enabled`** -- checked in the source, zero matches for `enabled: Boolean` in FloatingActionButton.kt -- because the spec's own position is that an unavailable FAB should not appear at all. Hand-computing is the only way to show a disabled one. More to the point, the existing code is already better than the plan assumed: it pairs the colour with `Modifier.semantics { disabled() }` and a comment saying "looking unavailable is not being unavailable: without this a screen reader still announces a button it is happy to press." Left alone, and the plan corrected. **Eight screens are left for a person.** LandingScreen puts "Sign in" beside "Create profile", SocialPreconditionScreen puts "Invite a friend" beside "View invites", and six others do the same. Both members of each pair are filled buttons. Which one is primary is a product decision about what the screen is *for*, and picking wrong quietly weights a choice the user is supposed to make freely -- so this is listed in the plan rather than guessed at here. **Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, unchanged. Both changes are composition-time rendering, which this repo has no UI test infrastructure to assert; the device screenshot stands in for it. `:composeApp:compileDebugKotlinAndroid` builds and the apk runs. Co-Authored-By: Claude Opus 5 --- .../ui/composable/ActiveProfileScreen.kt | 20 +++++- .../widgets/LoadingDataIndicator.kt | 25 ++++++-- docs/material-design-conformance.md | 64 +++++++++++-------- 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ActiveProfileScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ActiveProfileScreen.kt index 86e48564..2babace8 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ActiveProfileScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ActiveProfileScreen.kt @@ -67,6 +67,8 @@ import mantra.composeapp.generated.resources.we_couldn_t_find_the_local_profile_ import press.mantra.compose.ui.composable.widgets.ErrorState import androidx.compose.material3.SnackbarHost import press.mantra.compose.ui.composable.widgets.LocalSnackbarHostState +import androidx.compose.material3.FilledTonalButton +import androidx.compose.material3.ButtonDefaults @OptIn(ExperimentalMaterial3ExpressiveApi::class, ExperimentalMaterial3Api::class) @Composable @@ -195,7 +197,13 @@ fun ActiveProfileScreen( modifier = Modifier.weight(1f) ) - Button( + // The list this sits in is otherwise TextButtons -- + // edit profile, key packages, change account, profile + // keys, network relays. Two of the seven were filled + // Buttons, which is M3's highest emphasis and is meant + // for one action per screen. Sharing is the useful one, + // so it keeps medium emphasis rather than maximum. + FilledTonalButton( onClick = { onNavigateToRoute.invoke( ShareProfileRoute( @@ -324,7 +332,15 @@ fun ActiveProfileScreen( } item { - Button( + // Signing out was the *other* filled Button -- the most + // prominent control on the screen given to its most + // destructive action. It now matches how leaving and + // deleting a group are already treated in + // ChatRoomDetailScreen: a TextButton in the error colour. + TextButton( + colors = ButtonDefaults.textButtonColors( + contentColor = MaterialTheme.colorScheme.error + ), onClick = { onNavigateToRoute.invoke( ImplementationPendingRoute("Sign out") diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/LoadingDataIndicator.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/LoadingDataIndicator.kt index 3d91c1f4..35ae8b3a 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/LoadingDataIndicator.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/LoadingDataIndicator.kt @@ -1,7 +1,9 @@ package press.mantra.compose.ui.composable.widgets import androidx.compose.foundation.layout.* -import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi +import androidx.compose.material3.LoadingIndicator +import androidx.compose.material3.LoadingIndicatorDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface import androidx.compose.material3.Text @@ -14,10 +16,23 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import press.mantra.compose.ui.theme.spacing +/** + * The app's one loading state, used at 41 call sites. + * + * It draws M3's `LoadingIndicator` rather than a `CircularProgressIndicator`. That is the + * expressive component for exactly this -- an indeterminate wait with no progress to + * report -- and it is what `MaterialExpressiveTheme` expects to be paired with. + * + * The colour default moved from `secondary` to the component's own + * `LoadingIndicatorDefaults.indicatorColor`. `secondary` is the brand gold, which read as + * a warning rather than as a wait, and the previous version also hardcoded an 80dp width + * where the component has a size of its own. + */ +@OptIn(ExperimentalMaterial3ExpressiveApi::class) @Composable fun LoadingDataIndicator( modifier: Modifier = Modifier.fillMaxWidth(), - color: Color = MaterialTheme.colorScheme.secondary, + color: Color = LoadingIndicatorDefaults.indicatorColor, fillScreen: Boolean = true, text: String? = null ) { @@ -29,11 +44,7 @@ fun LoadingDataIndicator( Spacer(modifier = Modifier.weight(1f)) } - CircularProgressIndicator( - modifier = Modifier.width(80.dp).aspectRatio(1f), - color = color, - trackColor = MaterialTheme.colorScheme.surfaceVariant, - ) + LoadingIndicator(color = color) text?.let { diff --git a/docs/material-design-conformance.md b/docs/material-design-conformance.md index 3fc8dda6..b0a13d7d 100644 --- a/docs/material-design-conformance.md +++ b/docs/material-design-conformance.md @@ -658,37 +658,47 @@ a network identity question rather than a content one. ### Phase 5 — every screen has four states -**Why here.** It needs the tokens from Phase 2 and the strings from Phase 4, and -it produces the components Phase 6 will lay out. +**Why here.** It needs the tokens from phase 2 and the strings from phase 4, and it +produces the components phase 6 will lay out. -**Work.** +**Built.** Two commits. -1. **A `SnackbarHost` on every `Scaffold`**, and a single place to send messages - to it. 26 scaffolds, zero hosts, is why there is nowhere to report an invite - failing. -2. **One empty-state composable, one error-state composable.** Icon, message, - and — for errors — a retry action. This replaces 16 copies of - `Text("Something went wrong")` and 5 of `Text("No events were found")`, and - absorbs the 49 spacer idioms. **None of the 16 sites offers a retry today**; - each one is a dead end for the user. -3. **Loading gets the M3 component.** `LoadingDataIndicator` hardcodes an 80dp - `CircularProgressIndicator` in `colorScheme.secondary`. The pinned material3 - ships `LoadingIndicator`, which is the expressive equivalent and themed. -4. **Audit the disabled states.** Five screens compute a FAB container colour - by hand from a `can…` flag (`AddArtifactScreen.kt:156`, - `AddChapterScreen.kt:149`, `AddDialectScreen.kt:128`, - `TranslateChunkScreen.kt:131`, `AddTranslationArtifactVersionScreen.kt:155`). - Passing `enabled` and letting the component apply the 38% state layer is both - less code and the specified behaviour. -5. **Give buttons a hierarchy.** 31 `Button` and 27 `TextButton`, and nothing in - between — no `FilledTonalButton`, `OutlinedButton` or `ElevatedButton` - anywhere. Every screen therefore reads as either maximum or minimum emphasis. - Assign one filled button per screen and demote the rest. +1. **`ErrorState` and `EmptyState`** replace 21 hand-copied blocks — 16 saying "Something + went wrong", five saying "No events were found", **none of the sixteen with a retry**. + `EmptyState`'s message is required with no default, because that one sentence was shown + for five different absences and a shared default would have preserved exactly that. -**Done when** every `Scaffold` has a host, every list has an empty state, every -error offers a retry, and no screen computes a disabled colour by hand. +2. **A snackbar host**, where there had been none across 43 `Scaffold`s. On a composition + local rather than a parameter, because a view model coroutine reporting an outcome sits + several composables below the `Scaffold` that owns the host. It throws rather than + defaulting to a detached state: a default would make `notify(…)` a silent no-op on any + screen that forgot the host, which is the failure the file exists to end. Wired to + `publishNewKeyPackage` and `rotateKeyPackage`, both of which were fire-and-forget, and + verified on a device. -**Risk:** low. Additive. +3. **`LoadingDataIndicator` draws `LoadingIndicator`** — the expressive component for an + indeterminate wait — instead of a `CircularProgressIndicator` hardcoded to 80dp in the + brand gold, which read as a warning rather than as a wait. One wrapper, 41 call sites. + +4. **The profile screen's hierarchy.** Seven actions in one list, five of them + `TextButton`s and two filled `Button`s — M3's highest emphasis, meant for one action per + screen. One of the two was **Sign out**: the most prominent control on the screen given + to its most destructive action. Sharing is now `FilledTonalButton`; signing out is a + `TextButton` in the error colour, matching how leaving and deleting a group are already + treated elsewhere in this app. + +**The plan was wrong about disabled FABs.** It said five screens should pass `enabled` and +let the component apply the 38% state layer. No `FloatingActionButton` overload in +material3 1.10 takes `enabled` — the spec's position is that an unavailable FAB should not +appear at all — so hand-computing the colours is the only way to show one, and the existing +code already pairs it with `Modifier.semantics { disabled() }` so a screen reader does not +announce a button it is happy to press. Left alone. + +**Left for a person.** Eight more screens have two or more filled buttons competing: +LandingScreen's "Sign in" beside "Create profile", SocialPreconditionScreen's "Invite a +friend" beside "View invites", and six others. Which of a pair is primary is a product +decision about what the screen is for, not something to infer from the source, and getting +it wrong quietly weights a choice the user is supposed to make freely. ---