diff --git a/android/COVERAGE_PLAN.md b/android/COVERAGE_PLAN.md index 0b4ab51..ce4afd2 100644 --- a/android/COVERAGE_PLAN.md +++ b/android/COVERAGE_PLAN.md @@ -202,3 +202,28 @@ class LoginViewModelTest { - `MainDispatcherRule` + a documented ViewModel test pattern exist and are reused. - Coverage exclusions list only genuinely non-unit-testable files (UI composables, Android-framework glue) — no ViewModel, repository, DTO, or pure core-logic file is excluded. + +## 6. Amendment (2026-08-08, M12 phase 8): `ui/theme/**` narrowed to one file + +**A directory glob in the exclusion list went stale as soon as another milestone put testable code +in that directory.** §2 phase 0 excluded `app/src/main/java/**/ui/theme/**` because at the time the +directory held `Color.kt`, `Type.kt` and the composables — constants and composable bodies, nothing +a JVM test could execute. M12 then added three *pure* resolvers to it: `ShardPalette`, +`ShardStructure` and `ShardTypeface`, which exist precisely so the theming milestone's no-op +invariants could be plain JVM assertions. JaCoCo measures them at **98%, 100% and 100%**, and the +glob was discarding every line. + +The exclusion is now the single file it was really about, `ui/theme/Theme.kt` (the composable, 52%). +Everything else in `ui/theme/` is measured and all of it covers at 93% or better. + +Two things worth carrying forward: + +- **This did not rescue the gate and was not meant to.** M12's already-measured code + (`data/appearance/` at 100%, `ui/navigation/` at 93–100%) clears `new_coverage ≥ 50` on its own. + The point is that a future change deleting those resolvers' tests would now move the number, where + before it would not have — the exclusion was hiding well-tested code, which is the opposite of what + §1 built the list for and what §5's third bullet asks for. +- **Prefer file globs to directory globs when a directory is mixed.** `ui/components/**` stays a + directory glob and correctly so: `BrandAssets.kt` sits at 11% because only `brandAssetUrl` is pure, + and the rest is composable bodies. The distinction is whether the directory is *uniformly* + untestable, not whether it is under `ui/`. diff --git a/android/PLAN.md b/android/PLAN.md index 4590373..e59689e 100644 --- a/android/PLAN.md +++ b/android/PLAN.md @@ -1048,9 +1048,42 @@ push, and Play (M6–M8) follow the designed app. the shipped palette reproduces the pre-M12 `ColorScheme` role for role. **Phase 2 landed 2026-08-08** (Android-app#35 / docs#114): the four radii scale the app's own dp by their ratio to the `runic-gateway` baseline, and `--shadow-card` maps onto card elevation. The shadow is - the milestone's **one deliberate change to an untouched instance** — the app has been flat - since M5 while the preset it was drawn from selects a shadow — and is why every `Card(` became - a `ShardCard(`: Material takes elevation as a default argument, not from the theme. + a **deliberate change to an untouched instance** — the app has been flat since M5 while the + preset it was drawn from selects a shadow — and is why every `Card(` became a `ShardCard(`: + Material takes elevation as a default argument, not from the theme. **Phase 3 landed + 2026-08-08** (Android-app#36 / docs#115): seven bundled families beside Cinzel, resolved from + the stacks the theme publishes. It **nearly tripled the APK** — 4.80 → 12.94 MiB against a + drafted 1.5–2.5 MB estimate, with Merriweather alone 6.08 MiB of the growth because upstream + ships it as a barely-compressible three-axis variable font; verbatim bundling was chosen with + the cheaper options costed and declined. **Phase 4 landed 2026-08-08** (Android-app#37 / + docs#116): logo in the drawer header and top bar, hero on Home, no new dependency and no APK + cost. **Phase 5 landed 2026-08-08** (Android-app#38 / docs#117): the public nav's label, order + and hiding, keyed by the path→route table — whose sort keys must be indices into the + *website's* sixteen-row nav, not the app's nine. **Phase 6 landed 2026-08-08** + (Android-app#39 / docs#118): the drawer gains admin-authored sections and added links, a link + the app can open natively doing so and one it cannot handing off to a Custom Tab. + + **Phase 8 landed 2026-08-08** (Android-app#40 / docs#119) and did more than its name. Scoped + as docs, coverage and the cutover, its **AC-5 on-device walk** — two AVD passes against a local + website, the themed one at every role rung — confirmed everything phases 1–6 had deferred to it + (including that a Custom Tab really opens, checked in `dumpsys`, and that an anonymous caller + sees the reordered, sectioned nav with no player or staff row leaking) **and found two theming + defects that 476 passing tests could not see**: + + - every `ShardCard` — 26 sites in 20 files — drew in Material's grey rather than the shard's + panel colour, because `CardDefaults.cardColors()` takes its container from + `surfaceContainerHighest` and the mapping had `surfaceContainer`, `…High` and `…Low` but not + `…Highest`; and + - the drawer's selected row ignored `--radius-pill`, because `NavigationDrawerItem` takes + `shape` as a default argument. + + Both are **phase 2's trap repeating**: Material takes these as default arguments, not from the + theme. Phase 2 found it for depth and swept the call sites; nobody asked whether colour and + shape had the same problem. The card fix also **changes the untouched app** — those grey cards + predate M12 and go back to M5 — so the milestone ships two deliberate changes to a shard that + has set nothing, the shadow and the card container, with AC-1 rewritten to record the second + rather than absorb it. Phase 8 also narrowed a stale `ui/theme/**` Sonar coverage glob that was + discarding three 98–100%-covered resolvers. **477 tests green.** ### Deferred (not a milestone) diff --git a/android/THEMING_AND_NAV.md b/android/THEMING_AND_NAV.md index e7ead72..756f786 100644 --- a/android/THEMING_AND_NAV.md +++ b/android/THEMING_AND_NAV.md @@ -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