feat: put M3's spacing scale in the theme, with semantic names over it
Phase 2, first step, of docs/material-design-conformance.md. 527 `.dp` literals in the UI
tree and no record of what any of them is for. This is what they migrate onto; the sweep
that moves them is the next commit.
**The scale is M3's own**, transcribed from m3.material.io/m3/pages/spacing/tokens: an 8dp
system where `space100 = 8dp`, including the sub-8 nested units (2, 4, 6) and the
non-multiples (10, 14, 20, 36) that Material defines because its own components need them.
Eighteen stops.
Worth being precise about what the audit found, because it changes what this phase is for.
The two dominant values in the tree are `10.dp` (132 uses) and `20.dp` (115), and **both
are already on the scale** -- `space125` and `space250`. Only 89 of 527 are genuinely
off-grid. So this is not mostly a sweep for wrong numbers. It is that nothing records
whether a given `10.dp` is padding, a gap or a margin, which are three things the spec
gives different rules to, and none of them can be adapted per breakpoint or per density
while they are literals.
**A `data class` behind a composition local, not a file of constants.** Nothing scales it
today and `Spacing()` is provided unmodified. It is shaped this way because two things are
coming that need it: spacing adapts across breakpoints, and M3 has a density setting for
data-heavy views. Both become a matter of providing a different instance rather than
touching a call site -- but only if the values arrive through the local. Top-level `val`s
would read identically and adapt to nothing, which is the version of this that looks done
and is not.
**Eight semantic names, because `space125` is no more readable than `10.dp`.** It says the
size and not the job. `screenMargin`, `containerPadding`, `compactPadding`, `relatedGap`,
`itemGap`, `sectionGap`, `emphasisGap`, `targetGap` say the job, and they are what call
sites should reach for; the raw stops are for the cases none of them fits.
They are split along the distinction the spec draws -- padding is inside an element, a gap
is between elements in a container, a margin is outside one -- and there is exactly **one**
margin, for the screen edge. That is deliberate: "define padding and gaps on the parent
container", "avoid defining margins on child elements as they usually aren't uniform, and
require more tokens". A semantic layer with a margin per element would have re-created the
problem in better-sounding names.
**Six assertions, and three of them are about failure modes that are invisible in review.**
- Every stop matches its published value. `space175 = 15.dp` would look entirely
plausible in the source, compile, and put every call site one unit off the grid.
- The token name predicts the value: the number after "space" is the value as a
percentage of the 8dp base, so `space250` is 20dp. A stop that does not obey that is a
stop nobody can predict from its name.
- Every semantic name resolves to a stop that is actually on the scale. The layer stops
being a scale the moment one of them is handed a literal, which is easy to do and
invisible to review.
- `targetGap` is at least 8dp, M3's minimum separation between adjacent touch targets --
the one semantic name with an external floor, and the one phase 3 will apply between
icon buttons.
- A scaled instance moves the semantic names with it. This is what the data class is
*for*: if a semantic name were a hardcoded `Dp` rather than a reference to a stop it
would stay behind at a wider breakpoint and the layout would half-adapt, which is worse
than not adapting.
**Tests.** 936 pass, 594 jvm over 72 classes and 348 android over 44, up from 930/588/342.
`:composeApp:compileDebugKotlinAndroid` builds. No call site changed, so no pixels moved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,141 @@
|
||||
package press.mantra.compose.ui.theme
|
||||
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import androidx.compose.ui.unit.dp
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* The spacing scale against M3's published token values.
|
||||
*
|
||||
* A scale is only worth having if it is the same scale everyone else is using. These are
|
||||
* transcribed from m3.material.io/m3/pages/spacing/tokens, and the point of asserting them
|
||||
* is that a transcription error is invisible: `space175 = 15.dp` would look entirely
|
||||
* plausible in the source, would compile, and would put every call site that reached for
|
||||
* it one unit off the grid.
|
||||
*/
|
||||
class SpacingScaleTest {
|
||||
|
||||
private val spacing = Spacing()
|
||||
|
||||
/** M3's system spacing tokens, token name to value. */
|
||||
private val published: List<Pair<String, Dp>> = listOf(
|
||||
"space0" to 0.dp,
|
||||
"space25" to 2.dp,
|
||||
"space50" to 4.dp,
|
||||
"space75" to 6.dp,
|
||||
"space100" to 8.dp,
|
||||
"space125" to 10.dp,
|
||||
"space150" to 12.dp,
|
||||
"space175" to 14.dp,
|
||||
"space200" to 16.dp,
|
||||
"space250" to 20.dp,
|
||||
"space300" to 24.dp,
|
||||
"space400" to 32.dp,
|
||||
"space450" to 36.dp,
|
||||
"space500" to 40.dp,
|
||||
"space600" to 48.dp,
|
||||
"space700" to 56.dp,
|
||||
"space800" to 64.dp,
|
||||
"space900" to 72.dp,
|
||||
)
|
||||
|
||||
private val declared: List<Pair<String, Dp>> = listOf(
|
||||
"space0" to spacing.space0,
|
||||
"space25" to spacing.space25,
|
||||
"space50" to spacing.space50,
|
||||
"space75" to spacing.space75,
|
||||
"space100" to spacing.space100,
|
||||
"space125" to spacing.space125,
|
||||
"space150" to spacing.space150,
|
||||
"space175" to spacing.space175,
|
||||
"space200" to spacing.space200,
|
||||
"space250" to spacing.space250,
|
||||
"space300" to spacing.space300,
|
||||
"space400" to spacing.space400,
|
||||
"space450" to spacing.space450,
|
||||
"space500" to spacing.space500,
|
||||
"space600" to spacing.space600,
|
||||
"space700" to spacing.space700,
|
||||
"space800" to spacing.space800,
|
||||
"space900" to spacing.space900,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `every stop matches its published value`() {
|
||||
declared.zip(published).forEach { (mine, theirs) ->
|
||||
assertEquals(theirs.second, mine.second, "${mine.first} is ${mine.second}, M3 says ${theirs.second}")
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the token name is the value's relation to space100`() {
|
||||
// The naming rule, which is what makes the scale readable: the number after
|
||||
// "space" is the value as a percentage of the 8dp base. space250 is 20dp because
|
||||
// 20 is 250% of 8. A stop that does not obey it is a stop nobody can predict.
|
||||
val base = 8.0
|
||||
declared.forEach { (name, value) ->
|
||||
val percent = name.removePrefix("space").toInt()
|
||||
assertEquals(
|
||||
base * percent / 100.0,
|
||||
value.value.toDouble(),
|
||||
absoluteTolerance = 0.001,
|
||||
message = "$name should be ${base * percent / 100.0}dp to match its name, is $value",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the scale rises`() {
|
||||
declared.zipWithNext().forEach { (a, b) ->
|
||||
assertTrue(b.second > a.second, "${b.first} (${b.second}) is not greater than ${a.first} (${a.second})")
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `every semantic name resolves to a stop on the scale`() {
|
||||
// The semantic layer exists so call sites say the job rather than the size. It
|
||||
// stops being a scale the moment one of them is given a literal instead, which is
|
||||
// an easy thing to do and an invisible thing to review.
|
||||
val stops = declared.map { it.second }.toSet()
|
||||
val semantic = listOf(
|
||||
"screenMargin" to spacing.screenMargin,
|
||||
"containerPadding" to spacing.containerPadding,
|
||||
"compactPadding" to spacing.compactPadding,
|
||||
"relatedGap" to spacing.relatedGap,
|
||||
"itemGap" to spacing.itemGap,
|
||||
"sectionGap" to spacing.sectionGap,
|
||||
"emphasisGap" to spacing.emphasisGap,
|
||||
"targetGap" to spacing.targetGap,
|
||||
)
|
||||
|
||||
semantic.forEach { (name, value) ->
|
||||
assertTrue(value in stops, "$name is $value, which is not on the scale")
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `adjacent touch targets are held at least 8dp apart`() {
|
||||
// M3: "targets separated by 8dp of space or more promote balanced information
|
||||
// density and usability." targetGap is what the phase 3 sweep applies between
|
||||
// icon buttons, so it is the one semantic name with an external floor.
|
||||
assertTrue(
|
||||
spacing.targetGap >= 8.dp,
|
||||
"targetGap is ${spacing.targetGap}, below M3's 8dp minimum separation",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a scaled instance moves every stop and every semantic name with it`() {
|
||||
// What the data class is for. The breakpoint phase provides a wider Spacing at
|
||||
// larger windows; if a semantic name were a hardcoded Dp rather than a reference
|
||||
// to a stop, it would stay behind and the layout would half-adapt.
|
||||
val wide = Spacing(space200 = 24.dp, space300 = 32.dp)
|
||||
|
||||
assertEquals(24.dp, wide.screenMargin, "screenMargin did not follow space200")
|
||||
assertEquals(24.dp, wide.containerPadding, "containerPadding did not follow space200")
|
||||
assertEquals(32.dp, wide.sectionGap, "sectionGap did not follow space300")
|
||||
assertEquals(spacing.itemGap, wide.itemGap, "itemGap moved without space100 moving")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user