diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Theme.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Theme.kt index 804c46b0..7614bab4 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Theme.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Theme.kt @@ -10,7 +10,11 @@ import androidx.compose.runtime.Immutable import androidx.compose.ui.graphics.Color import com.example.ui.theme.AuxTypography -private val lightScheme = lightColorScheme( +// The six schemes are `internal` rather than `private` so ColorSchemeContrastTest can +// walk the real objects. Testing a copy rebuilt in the test from Color.kt would assert +// the palette and miss the wiring, which is the half that has gone wrong before -- a +// role pointed at the neighbouring value reads fine in isolation. +internal val lightScheme = lightColorScheme( primary = primaryLight, onPrimary = onPrimaryLight, primaryContainer = primaryContainerLight, @@ -48,7 +52,7 @@ private val lightScheme = lightColorScheme( surfaceContainerHighest = surfaceContainerHighestLight, ) -private val darkScheme = darkColorScheme( +internal val darkScheme = darkColorScheme( primary = primaryDark, onPrimary = onPrimaryDark, primaryContainer = primaryContainerDark, @@ -86,7 +90,7 @@ private val darkScheme = darkColorScheme( surfaceContainerHighest = surfaceContainerHighestDark, ) -private val mediumContrastLightColorScheme = lightColorScheme( +internal val mediumContrastLightColorScheme = lightColorScheme( primary = primaryLightMediumContrast, onPrimary = onPrimaryLightMediumContrast, primaryContainer = primaryContainerLightMediumContrast, @@ -124,7 +128,7 @@ private val mediumContrastLightColorScheme = lightColorScheme( surfaceContainerHighest = surfaceContainerHighestLightMediumContrast, ) -private val highContrastLightColorScheme = lightColorScheme( +internal val highContrastLightColorScheme = lightColorScheme( primary = primaryLightHighContrast, onPrimary = onPrimaryLightHighContrast, primaryContainer = primaryContainerLightHighContrast, @@ -162,7 +166,7 @@ private val highContrastLightColorScheme = lightColorScheme( surfaceContainerHighest = surfaceContainerHighestLightHighContrast, ) -private val mediumContrastDarkColorScheme = darkColorScheme( +internal val mediumContrastDarkColorScheme = darkColorScheme( primary = primaryDarkMediumContrast, onPrimary = onPrimaryDarkMediumContrast, primaryContainer = primaryContainerDarkMediumContrast, @@ -200,7 +204,7 @@ private val mediumContrastDarkColorScheme = darkColorScheme( surfaceContainerHighest = surfaceContainerHighestDarkMediumContrast, ) -private val highContrastDarkColorScheme = darkColorScheme( +internal val highContrastDarkColorScheme = darkColorScheme( primary = primaryDarkHighContrast, onPrimary = onPrimaryDarkHighContrast, primaryContainer = primaryContainerDarkHighContrast, diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/ColorSchemeContrastTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/ColorSchemeContrastTest.kt new file mode 100644 index 00000000..d97a445b --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/ColorSchemeContrastTest.kt @@ -0,0 +1,202 @@ +package press.mantra.compose.ui.theme + +import androidx.compose.material3.ColorScheme +import androidx.compose.ui.graphics.Color +import kotlin.math.pow +import kotlin.test.Test +import kotlin.test.assertTrue + +/** + * Every colour pair the six declared schemes promise, measured. + * + * M3's accessibility foundation gives two thresholds: small text needs 4.5:1 against + * its background, and large text or a meaningful non-text boundary needs 3:1. Disabled + * states are exempt. See docs/material-design-conformance.md, "The numbers". + * + * This passes as written -- the generated palette is sound, and the tightest pair in + * the tree is `onPrimaryContainer` on `primaryContainer` at 4.61:1 light. It exists so + * that a later edit to Color.kt or to the wiring in Theme.kt cannot quietly break one, + * because a broken pair is invisible in review: the two hex values look unrelated, and + * the failure only appears on a device, in one theme, to someone who cannot read it. + * + * It walks the real `ColorScheme` objects rather than rebuilding them from Color.kt, so + * it also covers the wiring. A role pointed at its neighbour's value -- `surfaceContainerHigh + * = surfaceContainerHighestLight` -- is a plausible slip that reads fine in isolation. + * + * What is deliberately *not* asserted here: + * + * - **Monotonicity across the contrast ladder.** The obvious invariant, that + * high-contrast beats medium beats default for every pair, is false and correctly + * so. In the light high-contrast scheme `surfaceContainerHighest` goes darker to + * separate it from `surface`, which lowers its ratio against `onSurface` (13.30 -> + * 12.29) while raising the one that matters. Ten pairs move that way. The floor is + * the invariant; the ladder is not. + * - **`outlineVariant`.** It reads 1.61:1 against surface, which looks alarming and is + * not a defect: M3's own baseline sits in the same range, and outlineVariant is a + * decorative divider. `outline`, the meaningful-boundary role, is asserted at 3:1. + * - **Call-site pairings.** Seven of those are below threshold today, the worst at + * 1.00:1. They belong to phase 3 of the conformance plan and get their own table + * there; asserting them now would mean checking in a red test. + */ +class ColorSchemeContrastTest { + + private val schemes: List> = listOf( + "light" to lightScheme, + "dark" to darkScheme, + "light medium-contrast" to mediumContrastLightColorScheme, + "dark medium-contrast" to mediumContrastDarkColorScheme, + "light high-contrast" to highContrastLightColorScheme, + "dark high-contrast" to highContrastDarkColorScheme, + ) + + /** + * Container role paired with the content role M3 assigns to it. Text drawn on the + * first is drawn in the second, so each of these is a small-text pairing. + */ + private val textPairs: List Color, (ColorScheme) -> Color>> = + listOf( + Triple("onPrimary on primary", { s: ColorScheme -> s.primary }, { s: ColorScheme -> s.onPrimary }), + Triple("onPrimaryContainer on primaryContainer", { s: ColorScheme -> s.primaryContainer }, { s: ColorScheme -> s.onPrimaryContainer }), + Triple("onSecondary on secondary", { s: ColorScheme -> s.secondary }, { s: ColorScheme -> s.onSecondary }), + Triple("onSecondaryContainer on secondaryContainer", { s: ColorScheme -> s.secondaryContainer }, { s: ColorScheme -> s.onSecondaryContainer }), + Triple("onTertiary on tertiary", { s: ColorScheme -> s.tertiary }, { s: ColorScheme -> s.onTertiary }), + Triple("onTertiaryContainer on tertiaryContainer", { s: ColorScheme -> s.tertiaryContainer }, { s: ColorScheme -> s.onTertiaryContainer }), + Triple("onError on error", { s: ColorScheme -> s.error }, { s: ColorScheme -> s.onError }), + Triple("onErrorContainer on errorContainer", { s: ColorScheme -> s.errorContainer }, { s: ColorScheme -> s.onErrorContainer }), + Triple("onBackground on background", { s: ColorScheme -> s.background }, { s: ColorScheme -> s.onBackground }), + Triple("onSurface on surface", { s: ColorScheme -> s.surface }, { s: ColorScheme -> s.onSurface }), + Triple("onSurfaceVariant on surfaceVariant", { s: ColorScheme -> s.surfaceVariant }, { s: ColorScheme -> s.onSurfaceVariant }), + Triple("inverseOnSurface on inverseSurface", { s: ColorScheme -> s.inverseSurface }, { s: ColorScheme -> s.inverseOnSurface }), + ) + + /** + * The tonal surfaces. All eight carry `onSurface` content -- there is no + * `onSurfaceContainer` role -- so every one of them is a text background, and a + * scheme that darkens one of them without checking is how this breaks. + */ + private val tonalSurfaces: List Color>> = listOf( + "surfaceDim" to { s: ColorScheme -> s.surfaceDim }, + "surfaceBright" to { s: ColorScheme -> s.surfaceBright }, + "surfaceContainerLowest" to { s: ColorScheme -> s.surfaceContainerLowest }, + "surfaceContainerLow" to { s: ColorScheme -> s.surfaceContainerLow }, + "surfaceContainer" to { s: ColorScheme -> s.surfaceContainer }, + "surfaceContainerHigh" to { s: ColorScheme -> s.surfaceContainerHigh }, + "surfaceContainerHighest" to { s: ColorScheme -> s.surfaceContainerHighest }, + ) + + @Test + fun `every content role reads at 4_5 to 1 on its container`() { + val failures = mutableListOf() + + schemes.forEach { (schemeName, scheme) -> + textPairs.forEach { (pairName, container, content) -> + val ratio = contrastRatio(container(scheme), content(scheme)) + if (ratio < SMALL_TEXT_MINIMUM) { + failures += "$schemeName: $pairName is ${ratio.format()}:1" + } + } + } + + assertTrue(failures.isEmpty(), "below 4.5:1 —\n" + failures.joinToString("\n")) + } + + @Test + fun `onSurface reads at 4_5 to 1 on every tonal surface`() { + val failures = mutableListOf() + + schemes.forEach { (schemeName, scheme) -> + tonalSurfaces.forEach { (surfaceName, surface) -> + val ratio = contrastRatio(surface(scheme), scheme.onSurface) + if (ratio < SMALL_TEXT_MINIMUM) { + failures += "$schemeName: onSurface on $surfaceName is ${ratio.format()}:1" + } + } + } + + assertTrue(failures.isEmpty(), "below 4.5:1 —\n" + failures.joinToString("\n")) + } + + @Test + fun `outline separates from every surface it is drawn on at 3 to 1`() { + val failures = mutableListOf() + + schemes.forEach { (schemeName, scheme) -> + (listOf("surface" to { s: ColorScheme -> s.surface }) + tonalSurfaces) + .forEach { (surfaceName, surface) -> + val ratio = contrastRatio(surface(scheme), scheme.outline) + if (ratio < NON_TEXT_MINIMUM) { + failures += "$schemeName: outline on $surfaceName is ${ratio.format()}:1" + } + } + } + + assertTrue(failures.isEmpty(), "below 3:1 —\n" + failures.joinToString("\n")) + } + + @Test + fun `a filled container stands off the surface behind it at 3 to 1`() { + // M3 asks for 3:1 between a clustered interactive container and its background. + // A standalone element such as a FAB is exempt by prominence, but `primary` and + // `error` are both used for buttons that sit beside other buttons. + val failures = mutableListOf() + + schemes.forEach { (schemeName, scheme) -> + listOf( + "primary" to scheme.primary, + "error" to scheme.error, + ).forEach { (roleName, role) -> + val ratio = contrastRatio(scheme.surface, role) + if (ratio < NON_TEXT_MINIMUM) { + failures += "$schemeName: $roleName on surface is ${ratio.format()}:1" + } + } + } + + assertTrue(failures.isEmpty(), "below 3:1 —\n" + failures.joinToString("\n")) + } + + private companion object { + /** WCAG 2.x, small text. */ + const val SMALL_TEXT_MINIMUM = 4.5 + + /** WCAG 2.x, large text and meaningful non-text elements. */ + const val NON_TEXT_MINIMUM = 3.0 + } +} + +/** + * WCAG relative luminance of one sRGB channel. + * + * The 0.03928 knee and the 2.4 exponent are the specification's, not an approximation + * of gamma 2.2 -- swapping in the simpler curve moves borderline pairs by enough to + * change a verdict, which is the whole point of this file. + */ +private fun channelLuminance(component: Float): Double { + val c = component.toDouble() + return if (c <= 0.03928) c / 12.92 else ((c + 0.055) / 1.055).pow(2.4) +} + +private fun Color.relativeLuminance(): Double = + 0.2126 * channelLuminance(red) + + 0.7152 * channelLuminance(green) + + 0.0722 * channelLuminance(blue) + +/** + * Contrast ratio between two opaque colours, 1.0 to 21.0. + * + * Both arguments must be opaque. A translucent colour has no ratio of its own -- it has + * one only once composited over something -- so composite it first and pass the result. + * Phase 3 needs that for the `.copy(alpha = 0.5f)` call sites; the schemes here are all + * fully opaque. + */ +internal fun contrastRatio(a: Color, b: Color): Double { + val la = a.relativeLuminance() + val lb = b.relativeLuminance() + return (maxOf(la, lb) + 0.05) / (minOf(la, lb) + 0.05) +} + +/** Two decimal places, without pulling in a platform formatter. */ +private fun Double.format(): String { + val scaled = (this * 100).toInt() + return "${scaled / 100}.${(scaled % 100).toString().padStart(2, '0')}" +} diff --git a/docs/material-design-conformance.md b/docs/material-design-conformance.md index 8de1dc3a..9d53c1e1 100644 --- a/docs/material-design-conformance.md +++ b/docs/material-design-conformance.md @@ -195,21 +195,20 @@ is the specified disabled opacity and is exempt. ### Spacing is a habit, not a system -520 `.dp` literals. Roughly 419 land on a defined spacing stop and about 101 do -not: +527 `.dp` literals. 419 land on a defined spacing stop, 19 are dimensions rather +than spacing (a hairline border, an avatar, an image height), and 89 are off-scale: ``` - 1dp × 12 5dp × 10 15dp × 14 18dp × 1 22dp × 1 - 30dp × 2 35dp × 3 50dp × 53 55dp × 2 70dp × 2 75dp × 1 + 5dp × 10 15dp × 14 18dp × 1 22dp × 1 30dp × 2 + 35dp × 3 50dp × 53 55dp × 2 70dp × 2 75dp × 1 ``` -Two things are true at once here, and the second matters more. The 1dp values -are hairline borders and dividers, which is fine. But `10.dp` (132 uses) and +The off-scale values are the smaller half of the problem. `10.dp` (132 uses) and `20.dp` (115) — the two dominant values — *are* on the scale, as `space125` and -`space250`. So this is not mostly an off-grid problem. It is that **nothing -records which of padding, gap or margin any of these is**, so there is no way to -adapt them per breakpoint or density later, and no way to tell a deliberate 15dp -from a typo. +`space250`, so a sweep for off-grid numbers would change almost nothing. What is +wrong is that **nothing records which of padding, gap or margin any of these +is**, so there is no way to adapt them per breakpoint or density later, and no +way to tell a deliberate 15dp from a typo. `Modifier.height(50.dp)` appears 49 times, almost always as a `Spacer` pushing an empty or error message down the screen. It is the same three lines copied @@ -226,7 +225,7 @@ labelLarge 13 titleSmall 12 titleMedium 6 titleLarge 4 headlineSmall 4 headlineMedium 2 headlineLarge 2 displayMedium 1 ``` -`label*` roles are 90 of 240 uses. Labels are for component text — buttons, +`label*` roles are 92 of 240 uses. Labels are for component text — buttons, tabs, chips — not for body copy or list content, and they are the smallest and tightest roles in the scale. Reading a screen where `labelMedium` carries the prose is the visual equivalent of everything being at the same pitch. Meanwhile @@ -338,7 +337,7 @@ state change in the app is a hard cut. |---|---|---| | Theme completeness | 12 roles unset, 4 schemes unreachable, no shapes/motion | 1 | | Colour at call sites | 7 pairings under threshold, one at 1.00:1 | 1, 3 | -| Spacing | 520 literals, no role recorded | 2 | +| Spacing | 527 literals, no role recorded | 2 | | Typography | 90/240 uses on `label*`, 2/30 roles emphasized | 1 | | Targets & labels | 33 unguarded `.clickable`, 18 untriaged nulls | 3 | | Content | 334 literals, title case throughout | 4 | @@ -361,38 +360,47 @@ so they lock in real state rather than aspiration. **Why first.** Every count in this document was produced by hand. If they cannot be regenerated, the phases below have no acceptance criteria — only opinions. -**Work.** +**Built.** -1. `docs/scripts/m3-audit.sh`, checked in, emitting the tables above: dp - histogram split by on/off the spacing scale, typography role distribution, - hardcoded colour sites, `.clickable` sites, `contentDescription = null` - count, string literal count, snackbar count, adaptive API count. -2. `composeApp/src/commonTest/.../ui/theme/ColorSchemeContrastTest.kt` — a pure - computation over the six declared schemes, no Compose runtime needed: +1. **`docs/scripts/m3-audit.sh`** regenerates every count in "Where this app + stands": the dp histogram split three ways, the typography role + distribution, hardcoded colour sites, `.clickable` sites, + `contentDescription = null`, string literals, snackbars, adaptive APIs. Each + number that a phase is meant to move carries a **budget** at the top of the + file, and `--check` exits 1 when one is exceeded. Budgets ratchet down in the + same commit that earns the reduction; Phase 8 wires `--check` into CI, at + which point raising one is the thing a reviewer looks for. - ```kotlin - private fun ratio(a: Color, b: Color): Double { … } // WCAG relative luminance + Two counts it reports separately, because conflating them would overstate the + problem: the twelve `*Fixed*` roles that fall through to Material baseline + lavender, and `surfaceTint`, which is also unassigned but defaults to + `primary` and is therefore correct. The `.dp` histogram likewise splits + dimensions (a hairline border, an avatar) out of the off-scale count. - @Test - fun everyOnRolePairsAtFourPointFive() { - schemes.forEach { (name, scheme) -> - pairs.forEach { (bg, fg) -> - val r = ratio(bg(scheme), fg(scheme)) - assertTrue(r >= 4.5, "$name: ${fg.name} on ${bg.name} is $r") - } - } - } - ``` +2. **`ColorSchemeContrastTest`** in `commonTest` — WCAG relative luminance from + first principles, no Compose runtime, four assertions over all six schemes: + every content role on its container at 4.5:1, `onSurface` on each of the + seven tonal surfaces at 4.5:1, `outline` against every surface it is drawn on + at 3:1, and `primary`/`error` containers against `surface` at 3:1. 4 tests, + green. - It passes today. It exists so Phase 1 cannot regress it, and so Phase 3 has - somewhere to add the call-site pairings. -3. Baseline the numbers into `docs/` alongside this file, dated. + It walks the real `ColorScheme` objects, which is why `Theme.kt`'s six + schemes moved from `private` to `internal`: rebuilding them in the test from + `Color.kt` would assert the palette and miss the wiring, and a role pointed at + its neighbour's value is exactly the slip that reads fine in review. -**Done when** the script runs from a clean checkout and its output matches the -tables in "Where this app stands", and `:composeApp:jvmTest` runs the contrast -test green. + Verified to bite. Nudging `onSurfaceVariantLight` from `#4C4546` to `#9C9496` + — a plausible "soften the secondary text" edit — fails it with + `light: onSurfaceVariant on surfaceVariant is 2.29:1`. -**Risk:** none. Nothing in the app changes. +**Deliberately not asserted:** monotonicity across the contrast ladder. The +obvious invariant — high-contrast beats medium beats default, for every pair — +is false, and correctly so: ten pairs move the other way because a high-contrast +scheme darkens `surfaceContainerHighest` to separate it from `surface`, trading +ratio against `onSurface` for the separation that matters. The floor is the +invariant; the ladder is not. + +**Risk:** none to the app. The only production change is a visibility keyword. --- @@ -463,7 +471,7 @@ alone. ### Phase 2 — spacing becomes a token -**Why here.** Phase 6 has to adapt spacing per breakpoint. It cannot adapt 520 +**Why here.** Phase 6 has to adapt spacing per breakpoint. It cannot adapt 527 literals. **Work.** @@ -495,7 +503,7 @@ literals. the audit distinguish padding from gap from margin, which the raw scale cannot. -3. **Migrate, in the order the audit reports.** The 101 off-scale values are the +3. **Migrate, in the order the audit reports.** The 89 off-scale values are the interesting ones and go first: each is either a typo (round to the nearest stop) or deliberate (say why, in a comment, and pick the nearest stop anyway). Then `10.dp` and `20.dp` en masse. diff --git a/docs/scripts/m3-audit.sh b/docs/scripts/m3-audit.sh new file mode 100755 index 00000000..446753d4 --- /dev/null +++ b/docs/scripts/m3-audit.sh @@ -0,0 +1,239 @@ +#!/usr/bin/env bash +# +# Material Design 3 conformance audit. +# +# Regenerates every count quoted in docs/material-design-conformance.md. The plan +# in that document has acceptance criteria per phase; this is what checks them. +# +# Usage: +# docs/scripts/m3-audit.sh report, always exit 0 +# docs/scripts/m3-audit.sh --check report, exit 1 if any budget is exceeded +# +# The budgets at the top are the state of the tree at the phase named beside each +# one. They ratchet down as phases land: lower the number in the same commit that +# earns it, never raise one. Phase 8 wires --check into CI, at which point raising +# a budget is what a reviewer looks for. + +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." || exit 1 + +UI=composeApp/src/commonMain/kotlin/press/mantra/compose/ui +THEME="$UI/theme" + +# --------------------------------------------------------------------------- +# Budgets. "-1" means not yet budgeted -- reported, but never fails --check. +# --------------------------------------------------------------------------- +BUDGET_HARDCODED_COLOR=11 # phase 3 drives to 0 outside theme/ +BUDGET_DP_LITERALS=-1 # phase 2 drives to ~0 outside theme/ +BUDGET_OFF_SCALE_DP=-1 # phase 2 drives to 0 +BUDGET_BARE_CLICKABLE=33 # phase 3 drives to 0 +BUDGET_NULL_DESCRIPTION=18 # phase 3 triages each one +BUDGET_STRING_LITERALS=-1 # phase 4 drives to <10 +BUDGET_TITLE_CASE=-1 # phase 4 drives to 0 +BUDGET_UNSET_COLOR_ROLES=12 # phase 1 drives to 0 + +# The M3 spacing scale: docs/material-design-conformance.md, "The numbers". +# space0..space900. Anything outside this set is off-scale. +ON_SCALE=(0 2 4 6 8 10 12 14 16 20 24 32 36 40 48 56 64 72) + +# Dimensions rather than spacing -- an avatar, an image height, a hairline +# border. These are exempt from the off-scale count; keep the list short and +# justify additions in the commit that makes them. +DIMENSION_EXEMPT=(1 80 128 180 200 500) + +fail_count=0 + +hdr() { printf '\n\033[1m== %s\033[0m\n' "$1"; } +note() { printf ' %s\n' "$1"; } + +# report