fix: promote the two pill colours to extended roles, fixing both contrast failures
Phase 1, step 5 of docs/material-design-conformance.md. `BluePill` and `RedPill` were raw
`Color` values in `Color.kt`, paired at the call site with `Color.White` and
`Color.DarkGray` by eye. Both pairings were below the 4.5:1 floor, and one of them was not
the colour it looked like.
**This step could not leave the pixels alone, and it is the only one so far that changes
them.** `Color.DarkGray` on `BluePill` measures **2.90:1**. `RedPill` was
`Color(230, 32, 32, 191)` -- the four-Int constructor, whose last argument is alpha, so it
is `#E62020` at 0.749. Opaque, white on it is 4.57:1 and passes; composited over the
surface as it actually renders, it is **3.50:1** and does not. Any correct version of these
two buttons is a visible change, so "adds, does not restyle" does not apply here and the
plan already said the call sites would move in this step.
**What M3 asks for here is an extended colour**, not a literal: a brand colour promoted to
a full role family -- `color` / `onColor` / `colorContainer` / `onColorContainer` -- so
that contrast is a property of the family rather than a decision repeated at each use.
`ColorFamily` was already declared in `Theme.kt`, unused, alongside an
`unspecified_scheme`; Material Theme Builder emits both, and this is what they are for.
**Derived by the same rule as the gold palette**, which the fixed-roles commit established
and verified: maximum in-gamut chroma at the source colour's Lab hue, sampled at M3's role
tones. BluePill's hue is 277.0 and RedPill's is 36.3.
role light dark blue light red light
color tone 40 tone 80 #0060AB #C00012
onColor tone 100 tone 20 #FFFFFF #FFFFFF
colorContainer tone 90 tone 30 #D7E2FF #FFDAD3
onColorContainer tone 10 tone 90 #001C39 #390C00
The buttons take `color`/`onColor`: 6.46:1 for the red pill and 6.44:1 for the blue, from
2.90 and 3.50.
**A side effect worth having.** At tone 40 the two pills are the same lightness, so they
now read as a matched pair. Before, `#E62020` sat beside `#5D8DD6` -- a saturated red next
to a soft periwinkle -- and the blue looked like the lesser option. On a screen whose whole
content is "commit, or wipe and leave", weighting one choice by accident is a defect of its
own.
**They travel on a composition local, not on `isSystemInDarkTheme()`.** `ColorScheme` has
no slot for extended colours, so `LocalExtendedColors` is provided by `TorchTheme` from the
same `darkTheme` it chooses the scheme with. Reading `isSystemInDarkTheme()` at the call
site would have been one line shorter and subtly wrong: it ignores a caller who passed
`darkTheme` explicitly, so a preview forcing dark would show light pills. The local
defaults to the light families rather than to `unspecified_scheme` -- nothing composes
outside `TorchTheme` today, and an invisible button is a worse way to discover that than a
light-themed one.
**No medium- or high-contrast variants, deliberately.** The entire surface is two buttons
on one screen, and the light family's weakest pair is 6.44:1 -- clear of the floor by more
than the contrast schemes would add. 32 more values for that would be out of proportion,
and the comment in `Color.kt` says so rather than leaving the omission to be read as an
oversight.
**`QRCodeView` lost its constructor default.** `QRCodeBackgroundPainter` defaulted
`backgroundColor` to `BluePill` -- a colour picked outside the theme for a surface that is
almost never seen, since at the default `padding = 0.dp` the logo painter covers the rect
it fills. The default is gone and the one call site passes it, so the choice is visible
rather than buried.
**Two new assertions, one of which is about the constructor.** `ColorSchemeContrastTest`
grows to 9. The first checks both pairs of every extended family at 4.5:1. The second
checks that every extended role is **opaque**, because `RedPill`'s alpha is what made the
first assertion insufficient: a translucent container has no ratio of its own -- it has one
only once composited -- so a contrast test would have measured a colour the user never
sees. That is the bug this commit fixes, and it would have passed a naive contrast test.
**The audit stopped counting its own commentary.** Fixing these call sites left a comment
*explaining* what `Color.White`/`Color.DarkGray` had been, and `m3-audit.sh` counted it as
a hardcoded colour -- so the file stayed in the report after being fixed. The script now
drops comment lines before counting. Budget ratcheted 11 -> 9: the two real sites, plus the
false positive the filter removes.
**Tests.** 930 pass, 588 jvm over 71 classes and 342 android over 43, up from 926/586/340.
`:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` build,
`m3-audit.sh --check` exits 0. The nine remaining hardcoded colours are phase 3's, and are
listed by the audit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
@@ -32,6 +32,7 @@ import androidx.compose.ui.tooling.preview.Preview
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.lifecycle.viewmodel.compose.viewModel
|
||||
import press.mantra.compose.repository.MarmotRepository
|
||||
import press.mantra.compose.ui.theme.LocalExtendedColors
|
||||
import press.mantra.compose.ui.view.model.CreateProfileViewModel
|
||||
import press.mantra.compose.ui.view.state.CreateProfileUIState
|
||||
|
||||
@@ -292,6 +293,12 @@ fun CreateProfileScreen(
|
||||
)
|
||||
}
|
||||
is CreateProfileUIState.ProfileReady -> {
|
||||
// Both pills come from the extended families rather than from a
|
||||
// literal paired with Color.White/Color.DarkGray by eye. The old
|
||||
// pairings read 4.57:1 and 2.90:1 against their containers, and
|
||||
// RedPill carried alpha 0.749, so composited over the surface the
|
||||
// first was really 3.50:1. Both are below the 4.5:1 floor.
|
||||
val extendedColors = LocalExtendedColors.current
|
||||
|
||||
Spacer(
|
||||
modifier = Modifier.weight(1f)
|
||||
@@ -308,8 +315,8 @@ fun CreateProfileScreen(
|
||||
|
||||
Button(
|
||||
colors = ButtonDefaults.buttonColors(
|
||||
containerColor = press.mantra.compose.ui.theme.RedPill,
|
||||
contentColor = Color.White
|
||||
containerColor = extendedColors.redPill.color,
|
||||
contentColor = extendedColors.redPill.onColor
|
||||
),
|
||||
onClick = {
|
||||
createProfileViewModel.createAccount(writeSeed)
|
||||
@@ -326,8 +333,8 @@ fun CreateProfileScreen(
|
||||
|
||||
Button(
|
||||
colors = ButtonDefaults.buttonColors(
|
||||
containerColor = press.mantra.compose.ui.theme.BluePill,
|
||||
contentColor = Color.DarkGray
|
||||
containerColor = extendedColors.bluePill.color,
|
||||
contentColor = extendedColors.bluePill.onColor
|
||||
),
|
||||
onClick = {
|
||||
// TODO: Delete everything and close the app
|
||||
|
||||
@@ -14,7 +14,7 @@ import androidx.compose.ui.graphics.drawscope.translate
|
||||
import androidx.compose.ui.graphics.painter.Painter
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import androidx.compose.ui.unit.dp
|
||||
import press.mantra.compose.ui.theme.BluePill
|
||||
import press.mantra.compose.ui.theme.LocalExtendedColors
|
||||
import io.github.alexzhirkevich.qrose.options.QrBrush
|
||||
import io.github.alexzhirkevich.qrose.options.QrLogoPadding
|
||||
import io.github.alexzhirkevich.qrose.options.QrLogoShape
|
||||
@@ -26,7 +26,11 @@ import mantra.composeapp.generated.resources.compose_multiplatform
|
||||
|
||||
class QRCodeBackgroundPainter(
|
||||
private val painter: Painter,
|
||||
private val backgroundColor: Color = BluePill,
|
||||
// No default. It used to be the `BluePill` literal, which meant a colour chosen
|
||||
// outside the theme for a surface that is almost never seen: at the default padding
|
||||
// of 0.dp the logo painter covers the rect it fills. Making the one call site pass
|
||||
// it keeps that visible rather than buried in a constructor default.
|
||||
private val backgroundColor: Color,
|
||||
private val padding: Dp = 0.dp,
|
||||
): Painter() {
|
||||
override val intrinsicSize: Size = painter.intrinsicSize
|
||||
@@ -71,6 +75,7 @@ fun QRCodeView(
|
||||
painter = painterResource(
|
||||
Res.drawable.compose_multiplatform
|
||||
),
|
||||
backgroundColor = LocalExtendedColors.current.bluePill.color,
|
||||
)
|
||||
|
||||
val painter = rememberQrCodePainter(
|
||||
|
||||
@@ -2,8 +2,68 @@ package press.mantra.compose.ui.theme
|
||||
|
||||
import androidx.compose.ui.graphics.Color
|
||||
|
||||
val BluePill = Color(0xFF5D8DD6)
|
||||
val RedPill = Color(230, 32, 32, 191)
|
||||
// ---------------------------------------------------------------------------
|
||||
// Extended colour roles: the two pills
|
||||
// ---------------------------------------------------------------------------
|
||||
//
|
||||
// The choice on CreateProfileScreen -- commit to the account, or wipe and leave -- is
|
||||
// the Matrix pill choice, and the two colours carry that meaning rather than a place in
|
||||
// the scheme. M3 calls this an extended colour: a brand colour promoted to a full role
|
||||
// family (`color` / `onColor` / `colorContainer` / `onColorContainer`) so that contrast
|
||||
// is a property of the family rather than something chosen per call site.
|
||||
//
|
||||
// They were `val BluePill = Color(0xFF5D8DD6)` and `val RedPill = Color(230, 32, 32, 191)`,
|
||||
// paired at the call site with `Color.DarkGray` and `Color.White`. Both pairings were
|
||||
// below the 4.5:1 floor -- 2.90:1 and, because RedPill carried alpha 0.749 and composited
|
||||
// over the surface, 3.50:1 -- so this step could not leave the pixels alone. Any correct
|
||||
// version of these two buttons is a visible change.
|
||||
//
|
||||
// **Derivation**, the same rule as the gold palette in the fixed-roles block: maximum
|
||||
// in-gamut chroma at the source colour's Lab hue, sampled at M3's role tones. BluePill's
|
||||
// hue is 277.0 and RedPill's is 36.3.
|
||||
//
|
||||
// role light dark
|
||||
// color tone 40 tone 80
|
||||
// onColor tone 100 tone 20
|
||||
// colorContainer tone 90 tone 30
|
||||
// onColorContainer tone 10 tone 90
|
||||
//
|
||||
// A side effect worth having: at tone 40 the two pills are the same lightness, so they
|
||||
// read as a matched pair. Before, a saturated red sat beside a soft periwinkle and the
|
||||
// blue looked like the lesser option -- which is not what the screen is asking.
|
||||
//
|
||||
// No medium- or high-contrast variants. The whole surface is two buttons on one screen,
|
||||
// and the light family's weakest pair is 6.44:1 -- clear of the 4.5:1 floor by more than
|
||||
// the contrast schemes would need to add. Generating 32 more values for that would be
|
||||
// out of proportion to what they cover.
|
||||
|
||||
val bluePillLight = ColorFamily(
|
||||
color = Color(0xFF0060AB), // tone 40
|
||||
onColor = Color(0xFFFFFFFF), // tone 100
|
||||
colorContainer = Color(0xFFD7E2FF), // tone 90
|
||||
onColorContainer = Color(0xFF001C39) // tone 10
|
||||
)
|
||||
|
||||
val bluePillDark = ColorFamily(
|
||||
color = Color(0xFFACC7FF), // tone 80
|
||||
onColor = Color(0xFF00315C), // tone 20
|
||||
colorContainer = Color(0xFF004882), // tone 30
|
||||
onColorContainer = Color(0xFFD7E2FF) // tone 90
|
||||
)
|
||||
|
||||
val redPillLight = ColorFamily(
|
||||
color = Color(0xFFC00012),
|
||||
onColor = Color(0xFFFFFFFF),
|
||||
colorContainer = Color(0xFFFFDAD3),
|
||||
onColorContainer = Color(0xFF390C00)
|
||||
)
|
||||
|
||||
val redPillDark = ColorFamily(
|
||||
color = Color(0xFFFFB4A5),
|
||||
onColor = Color(0xFF690000),
|
||||
colorContainer = Color(0xFF93000B),
|
||||
onColorContainer = Color(0xFFFFDAD3)
|
||||
)
|
||||
|
||||
val primaryLight = Color(0xFF000000)
|
||||
val onPrimaryLight = Color(0xFFFFFFFF)
|
||||
|
||||
@@ -8,7 +8,9 @@ import androidx.compose.material3.MotionScheme
|
||||
import androidx.compose.material3.darkColorScheme
|
||||
import androidx.compose.material3.lightColorScheme
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.runtime.Immutable
|
||||
import androidx.compose.runtime.staticCompositionLocalOf
|
||||
import androidx.compose.ui.graphics.Color
|
||||
|
||||
// The six schemes are `internal` rather than `private` so ColorSchemeContrastTest can
|
||||
@@ -385,6 +387,40 @@ val unspecified_scheme = ColorFamily(
|
||||
Color.Unspecified, Color.Unspecified, Color.Unspecified, Color.Unspecified
|
||||
)
|
||||
|
||||
/**
|
||||
* Brand colours that are not part of the generated scheme.
|
||||
*
|
||||
* `ColorScheme` has no slot for them -- M3 calls them extended colours and expects them to
|
||||
* ride alongside -- so they travel on their own composition local, provided by [TorchTheme]
|
||||
* from the same `darkTheme` the scheme is chosen with. Reading `isSystemInDarkTheme()` at
|
||||
* the call site instead would be subtly wrong: it would ignore a caller who passed
|
||||
* `darkTheme` explicitly, and a preview forcing dark would show light pills.
|
||||
*/
|
||||
@Immutable
|
||||
data class ExtendedColors(
|
||||
val bluePill: ColorFamily,
|
||||
val redPill: ColorFamily,
|
||||
)
|
||||
|
||||
/**
|
||||
* The extended colours for the current theme.
|
||||
*
|
||||
* Defaults to the light families rather than to `unspecified_scheme` so that anything
|
||||
* composing outside [TorchTheme] renders a real colour instead of nothing. That should not
|
||||
* happen -- as of this commit nothing in the tree does -- but an invisible button is a worse
|
||||
* way to find out than a light-themed one.
|
||||
*/
|
||||
val LocalExtendedColors = staticCompositionLocalOf {
|
||||
ExtendedColors(bluePill = bluePillLight, redPill = redPillLight)
|
||||
}
|
||||
|
||||
internal fun extendedColorsFor(darkTheme: Boolean): ExtendedColors =
|
||||
if (darkTheme) {
|
||||
ExtendedColors(bluePill = bluePillDark, redPill = redPillDark)
|
||||
} else {
|
||||
ExtendedColors(bluePill = bluePillLight, redPill = redPillLight)
|
||||
}
|
||||
|
||||
/**
|
||||
* How much contrast the person using the device has asked for.
|
||||
*
|
||||
@@ -461,16 +497,18 @@ fun TorchTheme(
|
||||
val colorScheme = dynamicColorScheme(darkTheme, dynamicColor)
|
||||
?: appColorScheme(darkTheme, contrast)
|
||||
|
||||
MaterialExpressiveTheme(
|
||||
colorScheme = colorScheme,
|
||||
// Every animation in the app should come from here rather than from a literal
|
||||
// `tween`, so that the whole app's feel is one decision. Nothing reads it yet;
|
||||
// the motion phase is what puts it to work.
|
||||
motionScheme = MotionScheme.expressive(),
|
||||
shapes = MantraShapes,
|
||||
typography = AuxTypography,
|
||||
content = content
|
||||
)
|
||||
CompositionLocalProvider(LocalExtendedColors provides extendedColorsFor(darkTheme)) {
|
||||
MaterialExpressiveTheme(
|
||||
colorScheme = colorScheme,
|
||||
// Every animation in the app should come from here rather than from a literal
|
||||
// `tween`, so that the whole app's feel is one decision. Nothing reads it yet;
|
||||
// the motion phase is what puts it to work.
|
||||
motionScheme = MotionScheme.expressive(),
|
||||
shapes = MantraShapes,
|
||||
typography = AuxTypography,
|
||||
content = content
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -193,6 +193,61 @@ class ColorSchemeContrastTest {
|
||||
assertTrue(failures.isEmpty(), "unassigned roles —\n" + failures.joinToString("\n"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the extended brand families read at 4_5 to 1`() {
|
||||
// The two pills on CreateProfileScreen. They are not part of any ColorScheme, so
|
||||
// nothing else in this file reaches them, and their previous incarnation is
|
||||
// exactly why they need asserting: `BluePill` paired with `Color.DarkGray` by eye
|
||||
// was 2.90:1, and `RedPill` carried alpha 0.749 so its white label was 3.50:1 once
|
||||
// composited. Both looked fine to whoever wrote them.
|
||||
val failures = mutableListOf<String>()
|
||||
|
||||
listOf(
|
||||
"bluePill light" to bluePillLight,
|
||||
"bluePill dark" to bluePillDark,
|
||||
"redPill light" to redPillLight,
|
||||
"redPill dark" to redPillDark,
|
||||
).forEach { (name, family) ->
|
||||
listOf(
|
||||
"onColor on color" to contrastRatio(family.color, family.onColor),
|
||||
"onColorContainer on colorContainer" to
|
||||
contrastRatio(family.colorContainer, family.onColorContainer),
|
||||
).forEach { (pairName, ratio) ->
|
||||
if (ratio < SMALL_TEXT_MINIMUM) {
|
||||
failures += "$name: $pairName is ${ratio.format()}:1"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assertTrue(failures.isEmpty(), "below 4.5:1 —\n" + failures.joinToString("\n"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the extended brand families are opaque`() {
|
||||
// `RedPill` was `Color(230, 32, 32, 191)` -- the four-Int constructor, whose last
|
||||
// argument is alpha. A translucent container has no contrast ratio of its own; it
|
||||
// has one only once composited, and the test above would have measured a colour
|
||||
// the user never sees. Roles are opaque.
|
||||
val translucent = listOf(
|
||||
"bluePillLight" to bluePillLight,
|
||||
"bluePillDark" to bluePillDark,
|
||||
"redPillLight" to redPillLight,
|
||||
"redPillDark" to redPillDark,
|
||||
).flatMap { (name, family) ->
|
||||
listOf(
|
||||
"$name.color" to family.color,
|
||||
"$name.onColor" to family.onColor,
|
||||
"$name.colorContainer" to family.colorContainer,
|
||||
"$name.onColorContainer" to family.onColorContainer,
|
||||
)
|
||||
}.filter { (_, color) -> color.alpha != 1f }
|
||||
|
||||
assertTrue(
|
||||
translucent.isEmpty(),
|
||||
"translucent roles —\n" + translucent.joinToString("\n") { "${it.first} alpha ${it.second.alpha}" },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `outline separates from every surface it is drawn on at 3 to 1`() {
|
||||
val failures = mutableListOf<String>()
|
||||
|
||||
@@ -24,7 +24,7 @@ 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_HARDCODED_COLOR=9 # 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
|
||||
@@ -60,15 +60,21 @@ report() {
|
||||
fi
|
||||
}
|
||||
|
||||
# Lines that are comments rather than code. Without this a note *about* a hardcoded
|
||||
# colour counts as one -- which happened the first time a call site was fixed and the
|
||||
# commit explained what it had replaced.
|
||||
NOT_A_COMMENT='^[^:]*:[[:space:]]*(//|\*|/\*)'
|
||||
|
||||
# Count matches across the UI tree, optionally excluding the theme package.
|
||||
# $1 pattern, $2 "exclude-theme" | "all"
|
||||
count() {
|
||||
local pattern=$1 scope=${2:-all}
|
||||
if [[ $scope == exclude-theme ]]; then
|
||||
grep -rE "$pattern" "$UI" --include=*.kt 2>/dev/null \
|
||||
| grep -v "^$THEME/" | wc -l | tr -d ' '
|
||||
| grep -v "^$THEME/" | grep -vE "$NOT_A_COMMENT" | wc -l | tr -d ' '
|
||||
else
|
||||
grep -rE "$pattern" "$UI" --include=*.kt 2>/dev/null | wc -l | tr -d ' '
|
||||
grep -rE "$pattern" "$UI" --include=*.kt 2>/dev/null \
|
||||
| grep -vE "$NOT_A_COMMENT" | wc -l | tr -d ' '
|
||||
fi
|
||||
}
|
||||
|
||||
@@ -119,8 +125,10 @@ report 'roles falling to the baseline palette' "$unset_baseline" "$BUDGET_UNSET_
|
||||
|
||||
hardcoded=$(count 'Color\(0x|Color\.(Red|Blue|Green|Gray|LightGray|DarkGray|White|Black|Yellow|Magenta|Cyan)' exclude-theme)
|
||||
report 'hardcoded Color outside theme/' "$hardcoded" "$BUDGET_HARDCODED_COLOR"
|
||||
[[ $hardcoded -gt 0 ]] && grep -rEln 'Color\(0x|Color\.(Red|Blue|Green|Gray|LightGray|DarkGray|White|Black|Yellow|Magenta|Cyan)' \
|
||||
"$UI" --include=*.kt | grep -v "^$THEME/" | sed "s|$UI/| |"
|
||||
# No -n, so the line is path:content and NOT_A_COMMENT's single-colon prefix matches.
|
||||
[[ $hardcoded -gt 0 ]] && grep -rE 'Color\(0x|Color\.(Red|Blue|Green|Gray|LightGray|DarkGray|White|Black|Yellow|Magenta|Cyan)' \
|
||||
"$UI" --include=*.kt | grep -v "^$THEME/" | grep -vE "$NOT_A_COMMENT" \
|
||||
| cut -d: -f1 | sort -u | sed "s|$UI/| |"
|
||||
|
||||
alpha=$(count '\.copy\(alpha')
|
||||
report 'colours derived with .copy(alpha =)' "$alpha" -1
|
||||
|
||||
Reference in New Issue
Block a user