docs(android): M12 phase 8 as landed, and the two defects AC-5 found
Phase 8 was scoped as docs, coverage and the cutover. The AC-5 walk turned it into a phase with code in it, which is the outcome the walk existed to produce - so the record is longer than a phase-8 note would normally be. THEMING_AND_NAV.md gains "Phase 8 as landed": what the walk confirmed, the two defects it found and why they are phase 2's trap repeating, the role sweep that says which Material roles are still unmapped and which of them have a reader, the coverage-glob narrowing, and the adb-reverse rig note for the next walk. The phase 2 note is amended where it claimed the card shadow was the milestone's only visible change to an untouched instance - it is now one of three, and the third (an untouched instance drawing a hero, because brand.hero carries the env default) is a correction to the record with no code behind it. PLAN.md's M12 entry stopped at phase 2; it now runs through 6, records phase 8 including the defects, and drops the same "one deliberate change" claim. COVERAGE_PLAN.md gains an amendment section explaining why the ui/theme/** directory glob became wrong the moment M12 put three pure resolvers behind it, and the general rule it suggests: prefer file globs to directory globs once a directory is mixed. Its own §5 already forbade excluding testable code. Pairs with Android-app phase 8. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -665,7 +665,7 @@ PR against `main`.
|
||||
| **5** ✅ | **Public nav: label / order / hidden** | The path→route table (§6.2), `Routes.news(category)`, the merge, drawer wiring. AC-3 |
|
||||
| **6** ✅ | **Public nav: sections & added links** | Drawer groups, `pruneNav` port, link path validation, native-route resolution + Custom Tab fallback (§6.3) |
|
||||
| **7** | **Authenticated navs** | ❌ **cancelled** — was: `GET /api/v1/settings/nav` behind a session-keyed repository; label/hidden for the four mapped rows (§6.4). See "Phase 7, cancelled" below |
|
||||
| **8** | **Docs, coverage & cutover** | This doc's "as landed" notes and any amendments the build forces, the `PLAN.md` §9 M12 entry refreshed, Sonar coverage for the new modules, AC-5 on-device walk, then `edge` → `main` |
|
||||
| **8** ✅ | **Docs, coverage & cutover** | This doc's "as landed" notes and any amendments the build forces, the `PLAN.md` §9 M12 entry refreshed, Sonar coverage for the new modules, AC-5 on-device walk, then `edge` → `main`. **The walk found two theming defects and phase 8 fixed them** — see "Phase 8 as landed" |
|
||||
|
||||
Phase 0 is the one with a hard rule attached: **it must change nothing on
|
||||
screen.** Everything after it is additive on top of a store that is already
|
||||
@@ -762,7 +762,12 @@ proven not to have moved anything.
|
||||
|
||||
### Phase 2 as landed
|
||||
|
||||
- **The app is flat, so §5.4 is the milestone's one visible default change.**
|
||||
- **The app is flat, so §5.4 is a visible default change.** (It was written here
|
||||
as the milestone's *only* one. Phase 8 found a second — the card *container*
|
||||
colour — and a third that is not a change at all but a correction to this
|
||||
record: an instance with a `BRAND_HERO_IMAGE` and no `brand_assets` row now
|
||||
draws a hero on Home, because `brand.hero` carries the env default. See
|
||||
"Phase 8 as landed".)
|
||||
Material3 1.3.0's filled `Card` is `ElevationTokens.Level0` — 0dp, checked in
|
||||
the artifact's bytecode, not assumed — and `FeatureCard` was a `Box` with a
|
||||
clip, a gradient and a border, drawing none of the "soft shadow" its own KDoc
|
||||
@@ -1022,6 +1027,108 @@ Consequences, all of them wanted:
|
||||
|
||||
If it is ever picked up, §6.4 is the spec and the merge would reuse phase 5's.
|
||||
|
||||
### Phase 8 as landed
|
||||
|
||||
Phase 8 was scoped as docs, coverage and the cutover. The AC-5 walk turned it
|
||||
into a phase with code in it, which is the outcome the walk existed to produce.
|
||||
|
||||
**AC-5 confirmed what the JVM could not reach.** Two passes on the AVD against a
|
||||
local website, the themed one walked at every role rung. Everything phases 1–6
|
||||
had deferred to it held: the Fantasy palette on screen; radii visibly square
|
||||
against the untouched build's rounded; **EB Garamond arriving in the *sans* role**,
|
||||
which is phase 3's preset-bypass finding proved on a device rather than in a test;
|
||||
the logo replacing the top bar's text with the 6× width cap holding; the hero
|
||||
centre-cropped into its 180dp band with the top and bottom of a 4:3 image cut;
|
||||
reorder, relabel and hide; the section header with its indented children; the
|
||||
added link with a native route carrying **no** badge and the handing-off one
|
||||
carrying it; and the Custom Tab genuinely opening — confirmed as
|
||||
`CustomTabActivity` in `dumpsys activity`, not by eye. AC-3's role dimension held
|
||||
at every rung: an anonymous caller saw the reordered, sectioned public nav with
|
||||
**no player or staff row leaking**, and the player rung stopped at `My houses`.
|
||||
AC-4 fell out for free when the second AVD started against an unreachable server:
|
||||
shipped theme, coded menu, one retry, no error surface.
|
||||
|
||||
**A correction the walk forced, with no code behind it.** An untouched instance
|
||||
**gains a hero on Home**. `brand.hero` carries the `BRAND_HERO_IMAGE` env default
|
||||
— `/assets/img/runic-emblem.png` on this one — so "admin has set nothing" does not
|
||||
mean "the slot is empty", and phase 4's `BrandHero` draws it. Nothing is wrong
|
||||
here: the website's own Home shows the same emblem, and §5.6's rule is that a
|
||||
blank slot draws nothing, which still holds. What was wrong is the *record* —
|
||||
phase 2 claimed the card shadow was the milestone's only visible change to an
|
||||
untouched app, and this was the second before the card container made it three.
|
||||
Worth stating plainly because a shard operator upgrading the app sees it without
|
||||
having touched Appearance at all.
|
||||
|
||||
**Two defects, one root cause, and it is phase 2's.** Material takes some themed
|
||||
values as **default arguments** rather than from the theme. Phase 2 found this for
|
||||
card elevation and answered it by sweeping every `Card(` into `ShardCard(`. Nobody
|
||||
then asked whether *colour* and *shape* had the same problem. They did:
|
||||
|
||||
- **Every `ShardCard` drew in Material's grey, not the shard's panel colour.**
|
||||
`CardDefaults.cardColors()` takes its container from `surfaceContainerHighest`
|
||||
— `FilledCardTokens.ContainerColor`, checked in the material3 1.3.0 artifact's
|
||||
bytecode the same way phases 1 and 2 checked theirs — and `shardColorScheme`
|
||||
mapped `surfaceContainer`, `…High` and `…Low` but not `…Highest`. All 26 call
|
||||
sites in 20 files. `FeatureCard` was unaffected because it reads
|
||||
`LocalShardPalette` directly, which is exactly why Home looked right and every
|
||||
other screen did not — the single most misleading thing about the bug.
|
||||
- **The drawer's selected row ignored `--radius-pill`.** `NavigationDrawerItem`
|
||||
takes `shape` as a default argument (`CircleShape`); the three call sites set
|
||||
`colors` and never `shape`. On Fantasy every other radius went square while the
|
||||
selected row stayed a full pill.
|
||||
|
||||
**The card fix changes the untouched app, and that is the honest framing.** The
|
||||
grey cards are not an M12 regression — an untouched instance draws them too, and
|
||||
has since M5. M12 only made it visible by theming everything around them. So
|
||||
fixing it moves the shipped app's cards from Material's grey to `--panel-flat`:
|
||||
**the milestone's second deliberate change to a shard that has set nothing**,
|
||||
alongside phase 2's card shadow. §9's last bullet had already called this shot —
|
||||
"if a screen looks wrong under a warm preset, that is a token the screen should
|
||||
have been reading and did not" — and this is that, at 26 call sites.
|
||||
|
||||
There is no version of the fix that spares untouched instances: the shipped
|
||||
palette *is* the `runic-gateway` preset, so the value the cards should take is the
|
||||
same value either way. AC-1 therefore **records** the change instead of absorbing
|
||||
it — every other role is still asserted byte-for-byte against the verbatim pre-M12
|
||||
scheme, and the two that moved are named, given their new values, and checked to
|
||||
have genuinely differed before, so the test cannot pass by coincidence.
|
||||
|
||||
**The role sweep, so the next person does not redo it.** Of the roles
|
||||
`shardColorScheme` leaves at `darkColorScheme()`'s defaults, exactly one had a
|
||||
live reader in this app: `surfaceContainerHighest`. Checked against the token
|
||||
class of every Material component the app actually draws —
|
||||
`NavigationDrawerTokens`, `SheetBottomTokens`, `AssistChipTokens`,
|
||||
`FilterChipTokens`, `TopAppBarSmallTokens`, `BadgeTokens` — all of which read only
|
||||
roles already mapped. `scrim` is read by the modal drawer and **stays Material's
|
||||
black on purpose**; a tinted scrim is a design change, not a gap.
|
||||
`surfaceContainerLowest` is mapped for consistency with `…Low` and has no reader.
|
||||
`primaryContainer`, `tertiaryContainer`, `inverse*`, `surfaceBright`, `surfaceDim`
|
||||
and `surfaceTint` are latent — the app draws no Snackbar, tonal button, elevated
|
||||
or outlined card. Adding one of those components means checking this list again.
|
||||
|
||||
**Coverage: the exclusion list had gone stale under the milestone.**
|
||||
`sonar.coverage.exclusions` carried a `ui/theme/**` directory glob from the M11
|
||||
coverage push, when that directory held only constants and composables. M12 put
|
||||
three pure resolvers in it — `ShardPalette`, `ShardStructure`, `ShardTypeface`,
|
||||
the reason phases 1–3 could prove their no-op invariants as JVM assertions — and
|
||||
the glob was dropping all three out of the denominator at 98%, 100% and 100%. It
|
||||
is now the one file it was really about, `ui/theme/Theme.kt`. This did not rescue
|
||||
the gate: M12's already-measured code (`data/appearance/` and `ui/navigation/`,
|
||||
93–100%) clears `new_coverage ≥ 50` on its own. It makes the number honest about
|
||||
which code the tests hold, so deleting those tests would move it.
|
||||
|
||||
**Tests: 477 green** (476 + the card-container assertion), `lintDebug` and
|
||||
`assembleDebug` clean. The JVM limit from phase 4 is unchanged and is why this
|
||||
phase needed a device at all: no Robolectric, no `androidTest` source set, so a
|
||||
composable body cannot run in a unit test. Both defects were invisible to 476
|
||||
passing tests and obvious within two minutes of looking at a themed screen.
|
||||
|
||||
**Rig notes, for the next walk.** The emulator reaches a local website through
|
||||
`adb reverse tcp:3000 tcp:3000`, which lets the app use `http://127.0.0.1:3000`
|
||||
and the debug `network_security_config`'s loopback exception **unchanged** — do
|
||||
not add `10.0.2.2` to it for a test rig. The Custom Tab resolves against the same
|
||||
base URL, so hand-off links work under the same tunnel.
|
||||
|
||||
## 9. Out of scope
|
||||
|
||||
- **Light mode / a light preset.** The website cancelled its Parchment phase; the
|
||||
|
||||
Reference in New Issue
Block a user