diff --git a/android/PLAN.md b/android/PLAN.md index 42079bd..c4043e7 100644 --- a/android/PLAN.md +++ b/android/PLAN.md @@ -1030,14 +1030,19 @@ push, and Play (M6–M8) follow the designed app. `nav_admin` two (`/player`, `/account`, `/admin`, `/admin/moderation`); the sidebar's other ~18 rows are admin *configuration* the app excludes, and two of the app's four staff entries are aggregates with no single web row. That is why they honor `label` and `hidden` only — and - why that phase is scheduled **last and marked optional**, so it can be dropped on its merits - once the rest is working. + why that phase was scheduled **last and marked optional**, so it could be dropped on its + merits. **It was: phase 7 is cancelled (2026-08-08).** The app makes no authenticated settings + call, `nav_admin` / `nav_player` are not consumed, and the player and staff drawer rows keep + their coded — and therefore localized — labels. Nothing in phases 0–6 was scaffolding for it. - **Excluded**, in the same class as M10's and M11's exclusions: the admin *configuration* panels themselves. The app does not gain Appearance or Navigation editors; it is a consumer. - Nine phases into a fresh `edge` in both repos, reaching `main` as one `edge` → `main` merge — - the same shape the website side used. Phase 0 (the contract and the appearance store) carries a - hard rule: it must change nothing on screen. + Eight phases (0–6 and 8; **7 cancelled**) into a fresh `edge` in both repos, reaching `main` as + one `edge` → `main` merge — the same shape the website side used. Phase 0 (the contract and the + appearance store) carries a hard rule: it must change nothing on screen. **Phase 0 landed + 2026-08-08** (Android-app#33 / docs#112): `SiteAppearance` now holds the brand, the resolved + theme tokens and the parsed `nav_public`, refreshed on resume, with nothing yet reading the last + two — see THEMING_AND_NAV.md "Phase 0 as landed". ### Deferred (not a milestone) diff --git a/android/THEMING_AND_NAV.md b/android/THEMING_AND_NAV.md index 5eeb66e..cd60828 100644 --- a/android/THEMING_AND_NAV.md +++ b/android/THEMING_AND_NAV.md @@ -54,7 +54,7 @@ No backend work. Everything below is live on `website/main` today. | `GET /public/settings` | `theme` | `Record` — 15 colors, 4 radii, `--shadow-card`, 3 font stacks. **Absent** when no row exists | | `GET /public/settings` | `brand.accent` / `.logo` / `.hero` / `.favicon` | Already **effective** values (override → env). The app reads `accent` today | | `GET /public/settings` | `nav_public` | Raw JSON **string**: a bare items map, or `{items, sections, links}` | -| `GET /api/v1/settings/nav` | `nav_admin`, `nav_player` | Raw JSON strings. Gate is `requireAuth`, **no role check** — a player may read it | +| ~~`GET /api/v1/settings/nav`~~ | ~~`nav_admin`, `nav_player`~~ | Raw JSON strings. Gate is `requireAuth`, **no role check** — a player may read it. **The app does not call this**: phase 7 cancelled (§6.4, §8) | Two shapes to get right on the wire: @@ -87,7 +87,7 @@ is no resolved counterpart — the merge is the *client's* job on the web too. | Light mode | Still **out of scope**. Every v1 preset is dark; the website's Parchment preset was cancelled (website §8 phase 9). The app stays dark-only, and `Theme.kt` keeps its single `darkColorScheme` | | Favicon | **No app surface.** Ignored, and not modeled | | Added links | A path matching a known app route opens the **native screen**; anything else hands off to a **Custom Tab** — see §6.3 | -| `nav_admin` / `nav_player` | **`label` and `hidden` only.** No order, no group — see §6.4 | +| `nav_admin` / `nav_player` | **Not consumed at all** — phase 7 cancelled 2026-08-08 (§6.4, §8). Was: `label` and `hidden` only | | Refresh | On connect, on **process start**, and on **resume** alongside the existing role re-validation — see §5.5 | | Failure posture | Forgiving on read, field by field. A failed settings call renders the shipped app, never an error | @@ -268,10 +268,11 @@ refreshed: changing the theme on a laptop and picking the phone up should see it, and the app already pays for a resume round-trip. -The authenticated `GET /api/v1/settings/nav` is fetched only when the session is -signed in, and re-fetched when the session changes — the same lifecycle -`ShardFeaturesRepository` already uses (M11). Signing out drops the cached admin -and player overrides. +~~The authenticated `GET /api/v1/settings/nav` is fetched only when the session is +signed in, and re-fetched when the session changes.~~ **Phase 7 is cancelled +(§6.4, §8): the app makes no authenticated settings call, and there is nothing +session-keyed to tear down on sign-out.** `GET /public/settings` is the whole +lifecycle. Every one of these is best-effort. A failed refresh keeps the last good appearance; there is no loading state and no error surface. @@ -435,6 +436,14 @@ and its teardown. It is scheduled last precisely so that decision can be taken with the rest of the milestone already working — dropping it costs nothing that phases 0–6 depend on. Unmapped keys are ignored either way. +> **Decision (2026-08-08): phase 7 is cancelled.** The measurement above was the +> whole case for building it and it did not carry. `nav_admin` and `nav_player` +> are therefore **not read by the app at all** — the player and staff drawer rows +> keep their coded `@StringRes` labels and their coded visibility, and this +> section stands as the record of why rather than as a spec. See §8. + + + **A label override replaces a `@StringRes`.** `MenuEntry.labelRes` is an int; the resolved entry carries `label: String?` beside it and the drawer prefers it. That means an admin's label is **not localized** — it is one string for every locale, @@ -474,20 +483,84 @@ PR against `main`. | # | Phase | Ships | |---|---|---| -| **0** | **Contract & appearance store** | `SettingsDto` gains `theme: Map?` and `nav_public: String?`; `SiteAppearance` replaces the bare `BrandDto` in `AppViewModel`; second-stage JSON parse; resume refresh (§5.5). **No visual change** — this phase must be invisible | +| **0** ✅ | **Contract & appearance store** | `SettingsDto` gains `theme` and `nav_public`; `SiteAppearance` replaces the bare `BrandDto` in `AppViewModel`; second-stage JSON parse; resume refresh (§5.5). **No visual change** — this phase must be invisible | | **1** | **Colors** | `ShardPalette` + `LocalShardPalette`; all direct `Color.kt` imports migrated; `RunicGatewayTheme(appearance)`; AC-1 + AC-2 tests | | **2** | **Radii & shadow** | Ratio-scaled `Shapes` (§5.2), elevation map (§5.4) | | **3** | **Fonts** | Seven bundled families + licenses; stack → `FontFamily` resolution; `Type.kt` takes its three families from the resolved theme. APK size recorded | | **4** | **Brand assets** | Logo in the drawer header and top bar, hero on Home (§5.6). Coil + `LocalAssetResolver` already exist; renders nothing when unset | | **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** | `GET /api/v1/settings/nav` behind a session-keyed repository; label/hidden for the four mapped rows (§6.4). **Optional — reconsider before starting it** | +| **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` | 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 proven not to have moved anything. +### Phase 0 as landed + +- **`theme` is modeled as a raw `JsonElement`, not `Map?`.** The + table in §5.1 is a closed set of string-valued tokens and the server validates + every one on write, so a typed map is what the contract says. But + `kotlinx.serialization` fails the decode of the *whole* object on a value of an + unexpected kind, and `theme` shares its payload with `brand` and `push` — so + one odd token would have blanked the branding and dropped the push relay URL, + which is the opposite of §2's forgiving-on-read. It is coerced field-by-field + in `SiteAppearance.from` instead: a non-string or blank value costs exactly its + own token. +- **The second-stage parse stops at "is this a plain object".** + `data/appearance/SettingsJson.kt` is the Kotlin counterpart of the web's + `lib/settingsJson.js` and makes the same single judgement — absent, malformed, + or a stored `null`/number/string/array all read as **absent**. Reading `items`, + `sections` and `links` out of the parsed object belongs to phases 5 and 6, so + phase 0 ships no half-built nav model. +- **`SiteAppearance.NONE` is the shipped app**, and three different things + resolve to it: no settings rows, a backend that predates the feature, and a + settings call that failed outright. That is §2 expressed as a value rather than + as a rule to remember. +- **A failed *refresh* keeps the last good appearance** rather than falling back + to `NONE`. §5.5 said "best-effort"; the distinction it did not draw is that a + moment of no connectivity on resume must not repaint a themed shard back to the + defaults. Only the initial load can produce `NONE`. +- **The resume refresh lives in `MainActivity`, not `RunicApp`.** The appearance + feeds the theme, which wraps the whole tree including the connect screen, so it + sits beside the theme rather than inside the app shell. It is a second + `LifecycleResumeEffect` next to the session's, and it no-ops unless the state + is `Ready`. +- **`RunicApp` still takes `brand: BrandDto?`.** Threading `SiteAppearance` + further down is phase 1's and phase 5's business; leaving the shell's signature + alone is what makes this phase's diff provably invisible. `AppState.Ready` is + the only place the type changed. +- Tests: `SettingsJsonTest` (6), `SiteAppearanceTest` (8), plus three decode + cases in `PublicDtoTest`. 360 unit tests green, `lintDebug` and `assembleDebug` + clean. `AppViewModel` itself stays untested — its four collaborators are + concrete classes with `Context`/prefs dependencies, and all of the phase's + logic is in the two pure modules above. + +### Phase 7, cancelled + +Reading `nav_admin` / `nav_player` is **not scheduled**. The measurement in §6.4 +is the reason and it stands: the two authenticated navs reach four app rows +between them (`/player`, `/account`, `/admin`, `/admin/moderation`), because the +website sidebar's other ~18 rows are admin *configuration* the app deliberately +excludes and two of the app's four staff entries are aggregates with no single +web row to be renamed from. Four rows does not pay for a new authenticated fetch, +a session-keyed cache and its teardown on sign-out. + +Consequences, all of them wanted: + +- the app never calls `GET /api/v1/settings/nav`, and the "a player may read it" + note in §3 becomes moot for this client; +- the player and staff drawer rows keep their coded `@StringRes` labels, so those + labels stay **localized** — which is the one thing the app gives up by not + honoring an admin's override, and the trade reads better in this direction for + four rows; +- §6.4's label-and-hidden-only rule and the `label: String?`-beside-`labelRes` + mechanism still ship, because **phase 5 needs both** for the public nav. Nothing + in phases 0–6 was scaffolding for phase 7. + +If it is ever picked up, §6.4 is the spec and the merge would reuse phase 5's. + ## 9. Out of scope - **Light mode / a light preset.** The website cancelled its Parchment phase; the