feat: draw the expressive loading indicator, and stop shouting the sign-out button
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user