diff --git a/CLAUDE.md b/CLAUDE.md index 28cea88f..3c68fa57 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,6 +73,13 @@ Compose Resources is not aapt: it does *not* unescape `\'` and does *not* collap though it does process `\n`. `StringCatalogueJvmTest` asserts each escape the app depends on; add to it rather than assuming a family rule. +A file whose strings are sample text rather than UI text — a gallery of colour pairings, +say — marks itself once at the top: + +```kotlin +// m3-string-exempt: these words are sample text for looking at colour pairings +``` + **Budget: 0 title-case strings.** Literals in composables are reported without a budget — 39 remain, all of them terms of a `+` concatenation. diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/ThemeGallery.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/ThemeGallery.kt index 6117c407..e5e393a0 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/ThemeGallery.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/ThemeGallery.kt @@ -1,3 +1,7 @@ +// m3-string-exempt: the words in this file are sample text for looking at colour +// pairings, not UI text. Nobody navigates here and nothing here is translated; +// putting these eight strings in the catalogue would add eight entries that no +// screen shows and that a translator would have to be told to ignore. package press.mantra.compose.ui.theme import androidx.compose.foundation.layout.Arrangement diff --git a/docs/material-design-conformance.md b/docs/material-design-conformance.md index 847c5326..b8709b3a 100644 --- a/docs/material-design-conformance.md +++ b/docs/material-design-conformance.md @@ -973,25 +973,113 @@ now would mean writing it twice. **Why last.** These lock in what the previous phases achieved. Written earlier, they would only fail. -**Work.** +**Built.** Three commits over the four items: the second needed nothing, and +finding that out is most of what it was worth. -1. **CI runs the Phase 0 audit** and fails on regression: no new `.dp` literals - outside the theme, no new hardcoded `Color`, no new string literal in a - composable, no new bare `.clickable`. -2. **The contrast test covers every scheme and every call-site pairing**, and is - part of the normal test run. It is the only one of these that catches a bug a - human reviewer reliably misses. -3. **Preview coverage per screen**: light, dark, high-contrast, 200% font scale, - compact and expanded. There are already 51 previews to build on. -4. **A short conventions note in `CLAUDE.md`** — spacing comes from the scale, - colour from the role, text from resources, targets are 48dp — so the rules - are visible at the point of writing new code rather than at review. +1. **`:composeApp:m3Audit`, wired into `check`.** The audit has existed since + phase 0 and has been run by hand at the end of every phase since, which is the + arrangement it was written to end: a budget nobody checks at the moment the + number moves is a number that drifts. It shells out to + `docs/scripts/m3-audit.sh --check` and fails the build on a budget exceeded or + a floor undercut, declares the script and the ui source tree as inputs so it is + up-to-date-able, and skips loudly rather than failing where there is no bash. -**Done when** the audit and the contrast test both run in CI, and a change -violating any of the four rules fails. + Verified to bite: one `Color(0xFFAABBCC)` added to `LoadingScreen.kt` reports + `hardcoded Color outside theme/ 1 over budget 0` and takes the build with it. + + **A Gitea Actions workflow** beside it, since the remote is a Gitea 1.25 + instance. Two jobs on purpose: `budgets` is grep over the source tree with no + gradle, no SDK, no submodules and no network — which is why the audit is a + shell script rather than a gradle plugin — and `tests` needs the whole + composite chain and a cold cross-compile of secp256k1, so it is split out for a + runner that has the capacity. **The workflow is unverified**: this repository + has had no CI of any kind, so there is no runner to try it against. The gradle + task is the half that is proven, and it is the half that runs on every + developer machine regardless. + +2. **The contrast test was already there.** Phases 0 and 3 built it out to ten + assertions over all six schemes — every content role on its container, every + tonal surface, the twelve fixed roles, the extended brand families, the + composited translucent containers, `outline` at 3:1 — and it runs in + `:composeApp:jvmTest`, which is now a CI job. Nothing was added, and the reason + is recorded in the test itself: a call site that pairs two roles the scheme + already covers produces a pairing the first assertion already walks, so + restating it per site would double the maintenance and catch nothing. + +3. **`@ConformancePreviews` on all 53 previews.** Every one of them rendered a + light theme at 100% text at whatever width the pane happened to be — the only + condition under which this app has never had a defect. They now render under + five: light, dark, 200% text, compact 400dp, expanded 1000dp. Each of the four + new ones is where a defect has actually been. It also gives phase 6 the check + it could not make: two of its five acceptance widths are now one click away on + every screen. + + High contrast is deliberately not in the annotation. Contrast is a property of + the scheme rather than of a screen, `ColorSchemeContrastTest` measures every + pair in all six, and there is no `@Preview` parameter for it — it needs + `TorchTheme(contrast = …)` in the body. `ThemeGallery` covers the six once, + over components rather than screens, with `dynamicColor = false` because an + android 12+ preview would otherwise paint all six columns from the wallpaper. + It is the only place the medium and high contrast schemes can be seen at all. + +4. **`CLAUDE.md`**, which this repository did not have. Seven rules, each with the + shape to copy, the shape not to, and the budget the audit holds it to. Where a + rule has a trap that has already caught somebody, the trap is named rather than + the rule restated — `.copy(alpha = …)` on a content role is how nine contrast + failures got in, Compose Resources unescapes `\n` but not `\'`, + `AnimatedContent` takes a `when`'s branches out of `ColumnScope`, + `MotionSchemeKeyTokens` is internal. **Risk:** none to the app; some friction for contributors, which is the point. +--- + +## Where this leaves the app + +All nine phases are built. The counts the audit was written to move, from the +state recorded in "Where this app stands" to the state `docs/scripts/m3-audit.sh` +reports today: + +| area | was | is | budget | +|---|---|---|---| +| roles falling to the baseline palette | 12 | 0 | 0 | +| hardcoded `Color` outside `theme/` | 11 | 0 | 0 | +| dp literals in spacing positions | 527 | 0 | 0 | +| `.clickable` with no minimum target | 33 | 0 | 0 | +| untriaged `contentDescription = null` | 18 | 0 | 0 | +| title case in UI strings | ~45 | 0 | 0 | +| string literals in composables | 334 | 39 | reported | +| `stringResource` call sites | 2 | 422 | — | +| snackbar hosts | 0 | 137 | — | +| adaptive API uses | 2 | 12 | floor 12 | +| navigation components | 0 | 2 | floor 2 | +| motion API uses | 1 | 13 | — | +| navigation transitions | 0 | 3 | — | + +The 39 remaining literals are all terms of a `+` concatenation, several of them +pluralisations that want a real plural resource rather than a format argument; +`m3-extract-formatted.py --remaining` lists them. + +What a person still has to look at, gathered from the phases that said so: + +- **which of a competing pair of filled buttons is primary**, on eight screens. + That is a product decision about what each screen is for, and getting it wrong + quietly weights a choice the user is supposed to make freely (phase 5). +- **whether a 480dp column of a particular screen reads well.** The measure is + applied at every root and asserted at five widths, but "renders correctly" is a + judgement no assertion makes (phase 6). +- **proposals → signing and artifacts → chapters**, the two list-detail families + the pane work did not reach. Same shape as chat, different content (phase 6). +- **the container transform** between a list item and its detail screen, which + wants `SharedTransitionLayout` and only applies below the expanded breakpoint, + where the detail is not already beside the list (phase 7). +- **full keyboard traversal on desktop** — tab order across the modal sheets and + the dialog, and focus returning to what opened them. Compose restores focus by + default, so the gap is evidence rather than known breakage (phase 3). +- **the avatar picker's selected state**, `secondaryContainer` at 1.65:1 against + the surface, which M3 accepts only where a second cue carries the selection. + This grid has neither an outline nor a checkmark (phase 3). + ## Order and dependencies | phase | depends on | touches | reversible alone | diff --git a/docs/scripts/m3-audit.sh b/docs/scripts/m3-audit.sh index 74369123..a5e0e35e 100755 --- a/docs/scripts/m3-audit.sh +++ b/docs/scripts/m3-audit.sh @@ -209,8 +209,32 @@ note "TextAlign.Center: $centred" # --------------------------------------------------------------------------- hdr 'Content (phase 4)' -literals=$(( $(count 'text = "') + $(count 'Text\("') )) +# A file whose strings are sample text rather than UI text -- ThemeGallery, whose whole +# job is to render colour pairings and whose words are chosen to be words -- marks itself +# once at the top with `// m3-string-exempt: `. Per file rather than per line, +# because the exemption is a property of what the file is for, and eight markers down one +# gallery would say less than one at the top of it. +# +# The same shape as `m3-color-exempt` and `m3-spacing-exempt`: the reason travels with the +# code rather than living in a list of file names here. +sample_text_files() { + grep -rlE 'm3-string-exempt' "$UI" --include=*.kt 2>/dev/null +} + +string_literals() { + local exempt + exempt=$(sample_text_files | sed 's|^|^|' ) + if [[ -z $exempt ]]; then + grep -rE 'text = "|Text\("' "$UI" --include=*.kt 2>/dev/null | grep -vE "$NOT_A_COMMENT" + else + grep -rE 'text = "|Text\("' "$UI" --include=*.kt 2>/dev/null | grep -vE "$NOT_A_COMMENT" \ + | grep -vE "$(sample_text_files | paste -sd'|' -)" + fi +} + +literals=$(string_literals | wc -l | tr -d ' ') report 'string literals in composables' "$literals" "$BUDGET_STRING_LITERALS" +note "sample-text files exempt: $(sample_text_files | wc -l | tr -d ' ')" res=$(count 'stringResource|Res\.string') note "stringResource / Res.string: $res" # Delegated, because the grep version was wrong twice: it required every word after the