085e598ec575bec9a8a541c52e6a24b0e2e2820d
666 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kgothatso Ngako
|
085e598ec5 |
feat(subgroups): a key state that names its parent, or is dropped for claiming one it cannot back
Phase 3 of docs/subgroups.md. `GroupKeyStateEvent` grows two optional tags and
`GroupKeyStateManager.stateFrom` grows the checks that make them mean something.
Every state signed before this reads exactly as it did: the tags are emitted only
when a parentage is passed, and the new checks fire only on a state carrying one.
```
["parent_group", <the parent room's id>]
["birth_certificate", <the parent's signed 30329, whole, as JSON>]
```
**Both or neither, and the type says so.** They arrive as one `SubgroupParentage`
rather than as two nullable parameters a caller could half-fill, because a parent
named with no certificate is a claim with the checkable part removed and a
certificate with no parent named beside it has nothing to be an index of. The
certificate is the claim; the parent tag is an index into it, since the
certificate already carries the same value in its own tag and as its author.
**Four checks, and a failure drops the whole state.** Both tags present and
readable; the certificate parses; `SubgroupBirthCertificateEvent.certifies` says
the named parent signed it for *this* room; and the certificate's `subgroup_key`
is the state's own threshold key. The last is belt and braces -- the room's id
already derives from that key and the certificate's id already derives from the
key it names -- and is stated anyway because the two facts live in different files
and a change to either should have to notice this one.
Keeping a failed claim as a *parentless* state was the alternative and is worse.
It is not a state with one field wrong; it is a device asserting a relationship
the parent never agreed to, and filing it would record a group as top-level here
and as a subgroup on every device that could check the certificate.
**`claimsParentage` exists because absent and unreadable are not the same.** A
`birth_certificate` tag carrying `{not json` reads as absent through
`parseBirthCertificate`, so a state claiming a parent in a form nothing can check
would otherwise be filed as an ordinary top-level group. It looks at the tag names
alone, which is the only reading that can tell the two apart.
**The three outcomes needed a wrapper.** `parentageOf` returns `Parentage?` where
null means refused and a present null value means none claimed -- a bare nullable
carries two of the three, and flattening "cannot prove it" into "did not claim
one" is exactly the bug the paragraph above describes.
`propose` takes the same optional parentage and re-runs `certifies` before
opening the session. Every device that receives the state runs that check and
drops it when it fails, so proposing one this device would not believe spends a
quorum's attention on a statement nobody will keep.
**The certificate travels whole rather than as its signature**, argued in
`SubgroupBirthCertificateTag`: a signature plus a rule for rebuilding the event it
covers breaks silently the first time the certificate's shape changes, since a
rebuild differing by one byte hashes to an id whose signature fails and is
indistinguishable from a forgery. Every state already signed would stop being
believed at once, for a reason nothing logs.
Ten tests appended to `GroupKeyStateTest`, using the existing `signedByRoom`
helper so the certificates are real FROST signatures by a real second group: the
happy path keeps both fields; no claim keeps both null; each half alone is
dropped; an unparseable certificate is dropped; a real certificate for another
child stapled on is dropped; one signed by this room's own group rather than the
named parent is dropped; the index disagreeing with the claim is dropped; a
certificate naming another key is dropped; and the tags round-trip, with a
top-level state carrying neither. 396 common tests and 683 jvm tests pass.
The end-to-end through a real session and two databases lands with Phase 9, once
`SubgroupManager` exists to drive one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
f6217ce6ea |
feat(subgroups): schema 17 -- four nullable columns, and not one of them a foreign key
Phase 2 of docs/subgroups.md. Somewhere to put a parent, now that Phase 1 can prove one. | table | column | filled from | trusted? | |---|---|---|---| | GroupKeyState | parentChatRoomId | the state's parent tag | yes -- the certificate was checked | | GroupKeyState | birthCertificateJson | the state's certificate tag | yes -- same | | ChatRoom | parentChatRoomId | the verified key state | yes | | DkgSession | parentChatRoomId | a tag on a ceremony proposal | **no** -- a screen's title | The trust column is the point of the table and is written into the KDoc of each one. Three of these are written only after a signature has been checked; the fourth is an unverified claim off a wire message, and a column that mixed the two would be a column no reader could act on. Nothing may be granted on the strength of `DkgSession.parentChatRoomId` that would not be granted without it. **None of the four is a foreign key, and that is the change most likely to be "fixed" by somebody later.** `GroupKeyState`, `DkgSession` and `GroupSignedEvent` all declare `ForeignKey(onDelete = CASCADE)` onto ChatRoom, so pointing a parent column at ChatRoom the same way is the obvious next move. It would mean deleting a parent room deletes every subgroup row beneath it -- and then, by their own cascades, each subgroup's messages, participants, key state, signing sessions and signed events. A user tidying away a group they had left would silently destroy a group they are still in. RESTRICT is no better: it would make a parent undeletable while any child row exists, which is a foreign key deciding a product question. And neither would work anyway, because a parent pointer routinely names a room this device does not have at all -- a member of a subgroup who was never in its parent holds the id off a certificate and nothing else. A dangling reference is the normal, expected state here, and readers resolve it with a lookup allowed to return null. **The certificate is stored whole, as JSON, rather than as its signature.** A signature plus a rule for rebuilding the event it covers is a rule that breaks silently the first time the certificate's shape changes: a rebuild differing by one byte hashes to an id whose signature fails, and is indistinguishable from a forgery. A few hundred bytes inside an encryption removes the class. The parent column beside it is an index into that event, never a second source of truth -- the two are written together or not at all. Four DAO reads, each with the limits of what it answers written down. `GroupKeyStateDao.getByParentChatRoomId`/`observe` list the children whose state this device holds, which is *verified* but not complete -- a certified child whose room was never created here leaves no state at all. `ChatRoomDao.observeByParentChatRoomId` lists the ones there is something to open, excluding soft-deleted rooms so a room the user cleared away does not reappear because its parent lists it. `DkgSessionDao.getByParentChatRoomId` is how a member gets back into a subgroup flow they closed the app during, since before the certificate is signed the ceremony is the only thing on the device that knows the flow was started. `AutoMigration(16, 17)`: nullable additions are a shape Room migrates itself, and 17.json exports with no new foreign key on any of the three tables. Nine tests in `SubgroupDaoJvmTest`, all on properties the compiler cannot see: a state and a room may each name a parent this device holds no room for; deleting a parent leaves its child, its child's key state and its child's lineage standing; the parent lists its children newest-first and filters on *which* parent rather than on having one; a soft-deleted subgroup drops out; and a ceremony round-trips the parent it was opened for. 386 common tests and 673 jvm tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
51d6a8841a |
feat(subgroups): a birth certificate, and the six questions that make one mean anything
Phase 1 of docs/subgroups.md. A group can now say, with a quorum, that another group is its child -- and any device holding the event can check it without a database, a lookup or a key it has to be told. This is the whole of what a subgroup relationship is. A child gets its own ChillDKG key, its own room, its own quorum and its own admins; nothing is inherited and nothing is delegated. What the certificate carries is one checkable claim: the group holding key P said, with a quorum, that the room C is its child. **Kind 30329**, past `GroupKeyStateEvent` (30326) and the chronicle pair (30327-30328), in the same private inner-event space. Like them it says something *about* a room rather than carrying the room's work, and like them it only ever exists inside an encryption a relay cannot open -- so the addressable semantics of the 3xxxx range never fire, and the `d` tag is this app's own newest-wins rule rather than a relay's. **Content is the child's room id, exactly as specified; the tags are what make it checkable.** Taken literally a certificate is 32 opaque bytes, and a parent admin would be asked to put the group's signature to a number they cannot check, produced by a ceremony most of them were not in, on behalf of people they have only the coordinator's word about. So the tags carry the child's threshold key, the derivation path, its founding admins and its name. The signature covers all of it, since an event id hashes over its tags, so nothing is added to the *claim* by putting it there -- only to what a signer can see before agreeing. The one that earns its place is `subgroup_key`: with it a signer's device can check `marmotGroupId(key, path) == content` for itself, which is the difference between approving a hash and approving a group. A coordinator who lies about who is in the child is then lying in a field the parent's signature covers. **`certifies` is six questions and no trust.** It is a certificate at all; it is about this child in both the content and the `d` tag, which have to agree; it names this parent; the child's id rederives from the key and path it carries; the parent room signed it; and all of it inside a `runCatching`, because every input is off the wire and a key that is not a point, a signature that is not 64 bytes and hex that is not hex all mean the same thing here. The fourth is the half that does not care who is speaking -- a certificate cannot be pointed at a room the key it names did not make -- and the fifth is the half that does. The fifth is `GroupKeyStateEvent.isSignedByRoom` used verbatim rather than reimplemented. It already asks "did *this room* sign this", and a room id is a public key here, which is the economy docs/member-chronicle.md is built on. Hex is compared case-insensitively as that check compares the author, since a certificate differing in case from what was signed fails the signature anyway -- so all this decides is whether a caller holding the same id in another case gets a silent drop. **What `certifies` deliberately does not check, and a test that fails if anybody adds it.** The name and the `p` tags are the *founding* roster. A certificate is signed once; members join and leave and rooms get renamed afterwards, and none of that reaches a signature already made. Comparing either against a room's current state would start rejecting valid certificates the first time somebody joined a subgroup, and the rejection would look exactly like a forgery rather than like a rule. `a certificate still verifies once the subgroup has been renamed and re-staffed` is there to make that failure loud instead of subtle. `parseAdminPublicKeys` uses `PTag.parseKey` rather than `PTag.parse`: the relay hint a full PTag carries is not part of what the parent agreed to, and a hint that failed to normalise would drop an admin from the roster rather than the hint from the admin. Two tag classes in the `FrostDerivationPathTag` shape. `SubgroupParentTag` checks nothing beyond having a value, because what makes a parent claim mean anything is the signature and a shape check in front of it would only decide which of two rejections a bad value gets. `SubgroupKeyTag` borrows its shape check from `GroupKeyStateEvent.parseThresholdPublicKey` rather than restating it, so two readers of one value cannot disagree about what a threshold key is. 13 tests, all pure: three independent groups from `Frost.trustedDealerKeygen`, a real FROST aggregate through the same nonce/signer-set/partial/aggregate shape `FrostSigningManager.advance` runs, and the author and the signer pulled apart so that both halves of question five are exercised separately. 386 common tests and 664 jvm tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
b50b1762d4 |
feat: put a room's signing key first on its detail screen, and the signed event behind it
A group's `GroupKeyState` had no surface anywhere in the app. It decides which share a member signs with and which identity a reader will see on everything the group signs, and the only way to learn either was to read the logs. The group detail screen now opens with it, and tapping it shows the event a quorum actually put its signature to, with a button to take that event somewhere it can be checked. **First on the screen, above the description.** Which key a room signs as is the fact the rest of the room's signed work stands on -- a dialect, an artifact and a chapter are all worth exactly what the identity behind them is worth -- so it goes before the library and the dialects rather than into the settings-ish tail of the screen with reindexing and leaving. It is absent rather than empty on a room the group has said nothing about: there is no half state to report, since a room either has one a quorum signed or has none, and the shared key entry further down is already where somebody goes to make one. **The row's subtitle is the identity, not the threshold key.** Those are different values -- the group's root ChillDKG key, and that key walked to the room's path -- and only the second one appears on anything. It is what a reader checks a signature against and it is the room's own id, so it is the value a member is most likely to want to compare against something. The whole of it, along with the root key it came from, is one tap away in the sheet. **The state and the event are read separately, and neither is derived from the other.** A `GroupSignedEvent` carries every field the `GroupKeyState` row does, so one read would have done -- but the two mean different things when they are missing. The row is this device's reading, which is what the app resolves a signing request against; the event is the group's statement, with the signature on it, which is the only part that can be checked. A device holding the reading and not the statement should not be shown fields as though they were signed, and the sheet says so instead. It never happens the other way round: a state is only ever written from an event that passed both checks. **`signedEventFor` looks wherever the event is filed, which is not this room.** Since the previous commit a group agrees its key state before the room exists, so the event lives under the NIP-17 room its ceremony ran in and is authored by the Marmot room it is about. Finding it by room would find nothing. It is found by its `d` tag instead, through the same `stateFrom` that lets one be believed at all, so nothing is shown that this device would not have acted on. `stateAmong` and the new `signedEventFor` are now one walk returning both halves, because an event that produces no state must not be shown as though the group had settled anything. **The sheet shows and copies the canonical compact event JSON.** Pretty-printing it would read better in the block and was rejected: the point of copying it is to hand somebody something they can verify, and the moment the display and the copy diverge the button stops being "copy what you are looking at". What is shown is `Event.toJson()`, byte for byte, which is what a nostr tool expects to be given. **The hex is grouped in eights, which the render caught and reading did not.** Captured off a real desktop composition at 360dp, the JSON wrapped fine -- it has quotes and commas to break on -- and every key ran off the side of the sheet with its last characters unreadable. A 64-character key has no space in it, so Compose lays the whole run on one line and lets it overflow. The spaces are the break opportunities. They are also how a value meant to be compared character by character against another member's screen should have been shown in the first place, for the same reason a fingerprint or an account number is grouped. Nothing is copied from those fields, so shaping them for reading costs a paste nothing. **The value colour is stated rather than inherited.** The labels are deliberately quieter at `onSurfaceVariant` and the values are what a member came for, so they name `onSurface` instead of taking whatever `LocalContentColor` happens to be. The JSON block sits on `surfaceVariant`/`onSurfaceVariant`, which `ColorSchemeContrastTest` already measures in all six schemes. **The sheet's body is a composable of its own, and that is what makes it testable.** A `ModalBottomSheet` is a popup in its own window, which a layout test cannot reach into, so `GroupKeyStateSheetContent` is separated from the sheet that contains it. `GroupKeyStateSheetLayoutJvmTest` then renders it at phone width and asserts the *height* of the JSON: a parent that narrow caps the text's layout width whether it wraps or clips, so width would pass either way. The threshold is calibrated against the real measurement rather than guessed -- it renders 224dp wrapped, against roughly 16dp for a single clipped line, so 60dp separates them with room to spare. A second case renders the sheet for a device holding no signed event, since that branch returns early and would otherwise never be laid out. The screen's `@ConformancePreviews` gains a key state, so the row renders under all five conditions rather than only in a group that has held a ceremony. 651 jvm tests and 373 common tests pass; `m3Audit` meets every budget, with the string count unchanged at 39 -- the eleven new pieces of UI text are in the catalogue in sentence case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
113eda9f4d |
feat: sign a group's key state before its room exists, and put FROST on NIP-17
A room's `GroupKeyState` was the new #admins room's first application message: the coordinator created the room, added the members, and only then asked the group to agree what it signs with. The order is now reversed. The group agrees it while it is still just a ceremony and a NIP-17 chat, and the room is created already knowing. **Two things were wrong with the old order, and neither was cosmetic.** The room's founding fact was settled after the founding, so a session that never reached a quorum left a live room whose every member fell back to rederiving -- which works, but only at the one path the constant names, and says nothing about which ceremony a device should take its share from. And the members who had to sign it were exactly the ones the room had just been created to hold: a member whose key package could not be found was excluded from the room *and* from a decision they held a share of, while `createAdminGroup` refuses to create the room at all in that case. Agreeing first makes the state a precondition of the room rather than an afterthought. **Signing therefore has to work in a NIP-17 room, and `broadcast` is the only place that knows.** In a Marmot room a signing message stays an ordinary inner event, encrypted to the group and addressed to nobody, because who is in the group is the MLS tree's business. In a NIP-17 room it goes out as one sealed gift wrap per member and has to name them all, or the members it left out never hear. Neither shape lets a recipient list decide anything -- the signer set comes from the ceremony's host keys either way -- so tagging somebody does not put them in it and failing to tag somebody only stops them hearing. Everything above `broadcast` is the same protocol; `NostrDao` dispatches the 3032x kinds off the gift-wrap path beside the DKG's, and the outbound path needed no change because `sealGiftWrapPayload` already seals to the room's participants and already refuses MLS rooms. **`signingPath` gains the one case that cannot be self-checked.** Every other candidate is right exactly when walking it reaches the room, which makes the resolution self-checking rather than trusting. A NIP-17 room's id is an aggregation of its members' keys, so no path reaches it and nothing can be checked that way. What the group signs as there is the room it is about to make: the ceremony's key at the app's admin path. That is admitted only when the ceremony is *this room's own* -- `key.chatRoomId == chatRoomId`, read from this device's database -- and the path is the constant rather than anything off the wire, so a proposer still chooses nothing. Naming some other ceremony this device holds a share for gets no path at all, and `completedKey` will not even find a key for a NIP-17 room that did not host one, so such a room cannot open a session; both are tested. **A state's subject is now its own `d` tag, not the room it arrived in.** Those used to be required to agree, and a mismatch was dropped -- the right rule while a state was made in the room it described, and the wrong one now that the two differ by design. Nothing is given up. The check that drop was standing in for is still made and made against the *named* room: `GroupKeyState.verifies` has to rederive it, and `isSignedByGroup` has to find a signature by the key that rederivation reaches. A state can therefore only ever be about a room it derives, whatever room it turned up in, so nobody can point one room at another room's key by putting it through the wrong door. The arrival room survives only as the fallback for a state carrying no `d` tag at all. **`record` holds what it cannot file; `adopt` files it when there is a room.** `GroupKeyState.chatRoomId` is a foreign key, so a state signed before its room exists has nothing to hang on -- which is now the normal case rather than an error. `record` says so and keeps the signed event; `adopt` reads it back off `GroupSignedEvent` and files it the moment a room appears. Both ways into a room end there: the member who creates it, in `createAdminGroup` and before the members are added, since filing is local and doing it while the room is certain to exist beats doing it after a step that can partly fail; and the member who arrives on a Welcome, in `NostrDao`, off the same event they were already holding because it was signed in the room they were already in. Nothing goes on the wire in either case. A member who was not in the ceremony holds no such event and gets nothing, which is right -- they hold no share either, so there is nothing for them to pick the wrong one of. **The screen watches the signed event, not a state row, and that is not interchangeable.** There is no row until there is a room, so the only thing that can say the agreement was reached is the event. `observeSignedGroupKeyState` is a flow over `GroupSignedEvent` by kind for the same reason the button it gates exists. Gating on the session's own items instead was rejected twice over: `complete` writes `stage = COMPLETE` *before* `recordSignedEvents`, so a collector woken by the session row can read before the event lands; and an item can hold a signature that has not been verified yet -- `complete` is where each one is checked against its id and author, and throws if it is not. **The button is one control and two steps, in the order they have to happen.** "Agree the group's signing key" until a quorum has signed, "Create the #admins group" after. Offering both at once would be the old order still available, and `createAdminGroup` refuses it in the view model as well, since the screen not drawing something is not a guard. A failed session re-offers the propose button and nothing else does, because a retry has to be a *new* session: the failed one's nonce seeds have already been published against an aggregate, and reusing one produces two partial signatures under a single secret nonce, which is how a share is extracted. `propose` mints a fresh session id every time, so tapping it is the safe retry by construction. **One bug found in review, which the tests now pin.** `replayStoredMessages` read only `marmotInnerEventDao`, so in a NIP-17 room a message arriving before the proposal it belongs to -- routine on a fresh sync, where a relay hands over a backlog in whatever order it likes -- was stored in the gift-wrap payloads and never read back. It now reads whichever store the room's transport writes to, which has to be the same reading `broadcast` makes. `a nonce arriving before the proposal is replayed out of the gift wraps` fails against the old code. **One wart, taken deliberately.** `GroupSignedEvent.chatRoomId` means the room a signature was made in, which for every event but this one is also the room whose key signed it. The key state is filed under the ceremony's room and authored by the #admins room, so `GroupSignedEvent.verifies` cannot pass on that row -- check it with `GroupKeyStateEvent.isSignedByGroup`, which asks the question the row cannot. Both columns are documented to say so. Re-filing the row under the #admins room once it exists was the alternative and buys nothing: a key state is not chroniclable, so no reader wants it there, and moving a row to keep one helper honest is worse than saying where the helper stops. `ChronicleManager` and `docs/member-chronicle.md` both argued for the `isChroniclable` filter from "every room signs a `GroupKeyStateEvent` as its first act", which is no longer true of any Marmot room. The filter stays and the argument is restated: what it stops is a member replaying any group-signed statement *about* the record as though it were work, and `applyPage` refuses the same kinds coming the other way. The two are a pair and neither is safe to drop on the strength of the other. `ChronicleAssemblyJvmTest` now puts its key state on file by hand, which makes that test sharper rather than hypothetical. `SignedGroupKeyStateTest`'s harness flattens the two transports into one `Queued` shape and each device declares whether its room has MLS state, so every existing test keeps testing the Marmot path and the seven new ones read the same. `GroupKeyStateTest`'s "a state naming another group's key is dropped" splits in two: one holding the room fixed and varying the key, which is still a drop, and one varying both, which is another group's true statement and is now attributed to that group's room rather than refused. 649 jvm tests and 373 common tests pass; `m3Audit` meets every budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
3382586501 | Merge branch 'mantra' into claude/key-recovery-functionality-00e269 | ||
|
Kgothatso Ngako
|
e05e4fd051 | Merge branch 'mantra' into claude/sign-in-alpha-message-fd21ca | ||
|
Kgothatso Ngako
|
64181bfcaa |
fix: say sign in is not available yet, instead of offering a flow alpha cannot finish
The sign in screen asked for an nsec or npub, walked through a confirmation step and reported an error when the sign in failed. None of that can succeed while the app is in alpha testing, so the screen is now the notice and nothing else, centred in the window because the message is the only thing on it. The screen no longer reads any state, so it takes no arguments and the navigation host stops handing it the repository. SignInToProfileViewModel, SignInToProfileUIState and SignInToProfileFormState are left where they are. Nothing references them now, but they are the implementation to restore when sign in ships, rather than something to write again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
bc83762274 | Merge branch 'mantra' into claude/key-recovery-functionality-00e269 | ||
|
Kgothatso Ngako
|
5106f31332 |
Merge branch 'mantra' into claude/key-recovery-functionality-00e269
Brings in the eight phases of Material Design 3 conformance work, which rewrote every screen this branch had touched. Both conflicts were in files the M3 work reindented wholesale, so they were resolved by taking that side and re-applying the key recovery change on top of it: - ActiveProfileScreen: the entry that phase 4 had externalised as "Profile keys" is now `key_recovery` in the catalogue, and opens KeyRecoveryRoute rather than the pending-implementation route. Its icon takes `Decorative`, since the label sits beside it. - MantraNavHost: the two new destinations were re-added inside the NavHost that now lives under MantraNavigationSuite. The two new screens were then brought up to the conventions CLAUDE.md now states: their 27 UI strings moved into the catalogue in sentence case (the word index became a `%1$s` format string), spacing comes from MaterialTheme.spacing, both content roots take readableContent(), the error branch is the shared ErrorState -- with no retry offered where no wallet is open, since retrying cannot help -- the checkbox row carries minimumInteractiveComponentSize() now that the whole row is the target, icons beside their own labels are Decorative, and the previews are ConformancePreviews. m3-audit.sh --check passes on every budget, and the string literal count is back to the 39 the document quotes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
c7b2dd25d9 |
feat: give the profile's key entry a recovery screen instead of a dead end
"Profile Keys" was an ImplementationPendingRoute: a key icon that led nowhere. It is now "Key Recovery", and it opens the recovery hub the Machankura app already has -- recovery phrase, cloud backup, emergency kit, and YOLO -- because a mantra profile *is* its seed. The npub that signs and the wallet that holds coins both come off one seed that never leaves the device, so a phone lost before a backup takes the account with it, and there is no server to ask for it back. The recovery phrase screen is the part that does the work: it decrypts the seed file through the existing loadAndDecryptSeed expect/actual, matches the active wallet's id against it, and shows that wallet's 12 words numbered in two columns behind an explicit reveal. The two backup confirmations are stored in that wallet's InternalPrefs, so they survive a re-install on the same seed, and the not-backed-up warning they clear is also what the hub reads for its status line. Cloud backup and the emergency kit route to the app's "coming soon" screen; they are placeholders in Machankura too. The read runs on Dispatchers.IO -- it is keystore-backed and blocks -- and the state it writes is a MutableStateFlow rather than a mutableStateOf, because a ViewModel built during composition silently loses an off-main-thread write made inside that first composition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
7b57fcb84c | Merge branch 'mantra' into claude/amazing-roentgen-929dc5 | ||
|
Kgothatso Ngako
|
b942e74449 | Update android build | ||
|
Kgothatso Ngako
|
a56295b0e2 |
docs: record what phase 8 built, and what is left for a person across all nine
The plan's last phase becomes a record, and the document gains a closing status: every count the audit was written to move, from the state in "Where this app stands" to what `m3-audit.sh` reports today, and a gathered list of what a person still has to look at — the eight screens with competing filled buttons, the two list-detail families the pane work did not reach, the container transform, desktop keyboard traversal, and the avatar picker's selected state. **The audit caught the previous commit.** `ThemeGallery` added eight string literals in composables, taking the count 39 -> 47, which is exactly the drift the budget exists to notice. They are sample text — the words are chosen to be words, so that colour pairings can be looked at — and putting them in the catalogue would add eight entries no screen shows and a translator would have to be told to ignore. So the audit grows a third exemption marker beside `m3-color-exempt` and `m3-spacing-exempt`: `m3-string-exempt`, per file rather than per line, because the exemption is a property of what the file is for and eight markers down one gallery would say less than one at the top of it. Back to 39, and the report now says how many files are exempt so the mechanism cannot be used quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
5f0afdf34d |
feat: render every preview under the five conditions a screen has to survive
Phase 8, item three. The tree had 51 previews and every one of them rendered one thing: a light theme, at whatever width the preview pane happened to be, at 100% text. That is the only condition under which this app has never had a defect. `@ConformancePreviews` replaces the bare `@Preview` at all 53 sites -- the two outside `ui/` included -- and renders each under five: light, dark, 200% text, compact 400dp, expanded 1000dp. `@Preview` is `@Repeatable`, so this is one annotation rather than five copied onto every preview and drifting apart. Each of the four new conditions is where a defect in this app has actually been: a colour that only fails in dark, a fixed-height container that clips at 200%, a layout that stretches because nothing held it, a row that reflows badly at phone width. It also gives phase 6 the check it could not make. "Every screen renders correctly at 400dp, 700dp, 1000dp, 1400dp and 1800dp" was verified structurally -- the measure applied at every root and asserted at those widths -- but never looked at per screen. Two of those widths are now one click away on every screen in the app. **High contrast is deliberately not in the annotation**, and the argument is worth stating because the omission looks like a gap. Contrast is a property of the *scheme*, not of a screen: the app declares six, `ColorSchemeContrastTest` measures every pair in all six, and a screen right in the default scheme is right in the high-contrast one by construction. Per-screen high-contrast previews would be 51 more renders of something already proved -- and there is no `@Preview` parameter for it in any case, since it needs `TorchTheme(contrast = …)` in the body. `ThemeGallery` covers them instead, once, over components rather than screens: all six schemes side by side, with body copy on surface, a card holding a list item -- the arrangement that rendered a headline at 1.00:1 before phase 3 -- and the three button emphases. `dynamicColor = false` on purpose, or an android 12+ preview paints all six columns from the wallpaper and the gallery shows nothing. It is the only place the medium and high contrast schemes can be seen at all: in the app they are reachable only through a platform setting, and on android only with dynamic colour off. 99 lines changed across 50 files, all of them one annotation and its import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
9f09f48133 |
build: make the conformance audit part of check, and gate the branch on it
Phase 8, the first two items. The audit has existed since phase 0 and has been run by hand at the end of every phase since, which is exactly the arrangement it was written to end: a budget nobody checks at the moment the number moves is a number that drifts. **`:composeApp:m3Audit`, wired into `check`.** It shells out to `docs/scripts/m3-audit.sh --check` and fails the build when a budget is exceeded or a floor is undercut. Verified to bite: adding one `Color(0xFFAABBCC)` to `LoadingScreen.kt` reports `hardcoded Color outside theme/ 1 over budget 0` and takes the build down with it. The task declares the script and the ui source tree as inputs and a marker file as its output, so it is up-to-date-able rather than re-running on every `check`. On a machine with no bash it warns and skips instead of failing, because a build that dies for a reason unrelated to the change under it teaches people to pass `-x`. **A Gitea Actions workflow**, since the remote is a Gitea 1.25 instance. Two jobs, deliberately: - `budgets` is grep over the source tree -- no gradle, no android SDK, no submodules, no network. This job is the reason the audit is a shell script rather than a gradle plugin, and it should stay runnable on a bare container. - `tests` needs a compiler and therefore the whole composite chain: four levels of submodule and a cross-compile of secp256k1's C sources, so a cold run is minutes rather than seconds. Split out so a runner can be pointed at `budgets` alone where that is all the capacity there is. Its two non-obvious steps carry the reasons at the site -- `submodules: recursive` or configuration fails with `Project with path ':library' not found`, and the android SDK is needed even for a jvm-only test run because `:secp256k1-kmp:jni:android` is in the graph. **The workflow is unverified**, and that is worth saying plainly: this repository has had no CI of any kind, so there is no runner registered to try it against. The syntax is valid and the commands are the ones used by hand throughout this work. The gradle task is the half that is proven, and it is the half that runs on every developer machine regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
410ece2df9 |
feat: fade between a screen's states instead of cutting between them
Phase 7, the second half. Every screen in this app is a `when` over a UI state -- loading, error, empty, loaded -- and every one of those changes was an unannounced cut: the spinner is there in one frame and the content is there in the next, with nothing saying they are the same screen answering the same question. `ScreenStateTransition` is M3's fade-through, which is the transition for content that replaces other content without being spatially related to it: the outgoing state fades out, the incoming one fades in and grows the last 8% into place. `SizeTransform(clip = false)`, so a tall loaded state does not stretch a short spinner on its way in. Specs from the theme's `MotionScheme`, effects for the fade and spatial for the scale. **The content key is the state's class, not the state.** This is the half that is easy to get wrong and impossible to see: keyed on the value, a screen re-runs the whole fade every time its loaded data changes -- a message arriving, a list growing by one -- so the screen flickers whenever anything happens, and every screenshot of it looks perfect. Keyed on the class, the animation runs when the state does and the data flows through untouched. There is a test for exactly that, and it is the more useful of the two. **Applied to 20 screens, and not to 15 others.** `AnimatedContent` is a layout node, so it can only wrap a `when` that is a composable's whole body. Where the `when` sits inside a `Column` whose branches use `Modifier.weight` -- the sign-in and create-profile flows, the frost signing and proposal screens, the two feed detail widgets, the four render helpers still on view models -- wrapping it would take those branches out of `ColumnScope`. The rule is mechanical, the reason is recorded once in `ScreenState.kt` rather than at each site, and the screens it excludes are named here rather than silently skipped. Reduced motion keeps the crossfade and drops the scale, which is the same position the navigation transitions take: what WCAG 2.3.3 and M3 ask to remove is movement, not the signal that something changed. **Most of this diff is indentation** -- 3,699 lines of it against 157 lines of substance, which is 21 screens gaining a wrapper and one helper being written. `git diff -w` shows the second number. **Verified by holding the clock still and looking at one frame**, which is the only frame that can tell a crossfade from a cut: during a transition both states are composed, and during a cut only ever one is. 639 jvm tests green; android and desktop compile. The audit's motion count goes 11 -> 13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
61793e2779 |
feat: give navigation its transitions from the motion scheme, and honour reduced motion
Phase 7, the first half. All 43 routes took navigation-compose's default, which turns out not to be the hard cut the plan expected: on android and desktop it is `fadeIn(tween(700))` / `fadeOut(tween(700))`, written into the library's own internals. Both halves of that are worth changing. 700ms is roughly three times M3's duration for a full-screen change, and a literal inside a dependency is not a decision this app made -- phase 1 wired a `MotionScheme` into the theme precisely so that there would be one place to make it. **The plan named an API that an app cannot reach.** It says every spec should come from `MotionSchemeKeyTokens`; that enum is `internal` to material3, so the tokens are not addressable by name from outside. `MaterialTheme.motionScheme` is the public surface and offers the same six specs. Two private helpers name which of them this app uses for what -- `defaultSpatialSpec` for the slide, `defaultEffectsSpec` for the fade -- which is the distinction the scheme draws: spatial motion is springy because it moves something, effects motion is not because a fading colour that overshoots looks like a fault. **The shape is M3's shared axis.** The arriving screen slides in from the trailing edge while the leaving one slides out toward the leading edge, both fading; going back mirrors it, so the direction of travel is legible rather than a dissolve that looks the same either way. `slideIntoContainer` is layout-direction aware, so an RTL locale gets the mirror for free. **Reduced motion, on the three platforms, in the shape phase 1 established.** `platformReducedMotion()` is an expect/actual beside `platformThemeContrast()`, observed rather than read once, because somebody who turns it on because motion makes them ill should not have to restart the app. Android has no "reduce motion" switch -- it has **Remove animations**, which sets the animation duration scales to zero. The platform applies that scale to `ValueAnimator` and **not to Compose**, which runs on its own clock and ignores it entirely, so an app that draws its own transitions has to read the setting itself. A `ContentObserver` on `ANIMATOR_DURATION_SCALE` catches the change without a restart. iOS is the one platform where it is a single documented call, `UIAccessibilityIsReduceMotionEnabled`, with the same notification shape as the darker-system-colours one already observed there. Desktop answers `false`, and says at the site why that is honest rather than a stub: Windows, macos and the freedesktop desktops each have the setting and none of the three reaches AWT. That is the same wall `platformThemeContrast` hits on linux and macos, and the same eventual answer -- a preference with the platform as its default. Reduced motion does not mean *no* transition. The screen still fades; what goes is the movement, which is what M3 and WCAG 2.3.3 are both about. **Two tests, and the first one found a design flaw in the second.** The claim "this transition slides and that one does not" cannot be asserted on the values -- `EnterTransition` has no public shape to inspect -- so it is measured: hold the clock, navigate, advance a third of the way, and read where the arriving screen is. Sliding, it is 54dp from home; reduced, it is already there. The reduced case read 54dp at first, because the test provided `LocalReducedMotion` *around* `TorchTheme` and the theme overwrote it. The fix is not in the test: `reducedMotion` is now a `TorchTheme` parameter defaulted to the platform, exactly as `contrast` is, because a value nothing can override is a value nothing can test -- and because the desktop actual is a hardcoded `false` that a settings screen will eventually need to override anyway. 637 jvm tests green; android and desktop compile. The audit's motion count goes 2 -> 11 and navigation transitions 0 -> 3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
dd40eeda76 |
fix: break the transcript's same-second tie on write order, so a burst reads forwards
`ChatMessageDao`'s two transcript queries ordered `createdAt DESC` and nothing
else. `MantraConverters` stores an `Instant` as epoch seconds, so lines written
inside one second tie -- a ceremony puts a dozen into a room faster than that,
and a request with the answer it triggers routinely lands inside one -- and with
no second key SQLite hands them back in scan order, which is rowid *ascending*.
Under a descending query drawn bottom-up by the feed's `reverseLayout`, that
draws a same-second burst backwards. Three lines written in one second, read
back through the old query:
expected:<[third, second, first]> but was:<[first, second, third]>
**The list and the room it opens already disagreed.** `ChatRoomDao` picks each
room's preview with `ORDER BY createdAt DESC, id DESC`, and
|
||
|
Kgothatso Ngako
|
16775bf6c7 |
docs: record what phase 6 built, and give the audit a floor to defend it
The plan's phase 6 becomes a record rather than a proposal, in the shape the earlier phases took: what was built, what was decided and why, what a person still has to look at. Two decisions in it were the product owner's rather than the code's -- promoting search and profile to navigation destinations, and doing chat alone rather than all three list-detail families -- and both are named as such with the date. **The audit learns two things.** It counted `NavigationBar(`, `NavigationRail(` and friends, and reported **zero** for an app that had just grown a navigation bar: `NavigationSuiteScaffold` is what chooses between them per breakpoint, and the concrete component never appears in the source. It now counts the scaffold and its items. And it grew a `floor()` beside `report()`. Every other budget in the file is a ceiling that ratchets down as a phase lands, which is the right shape for literals, hardcoded colours and untriaged nulls -- things a careless edit *adds*. The adaptive work is the opposite: a screen that stops reading the breakpoint still compiles and still renders, and the count goes down. So `--check` now also fails when the adaptive API count drops below 12 or the navigation component count below 2. **Two `contentDescription = null` that the audit caught in this phase's own work** -- the navigation item's icon and the new-chat button's -- now say `Decorative`. Same null, and the same convention phase 3 established: recording that somebody looked is the whole point, and a budget of zero only holds if new code obeys it too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
3f78eef1e8 |
feat: put the chat list beside the conversation, from the expanded breakpoint up
Phase 6, step 4, chat first as the plan asks. On a window 840dp or wider the home screen is now the room list at a fixed width and the selected conversation filling the rest; on anything narrower it is exactly what it was. **Below expanded is not caution, it is the spec.** The breakpoints page says not to put two dense panes in a medium window, and `calculatePaneScaffoldDirective` in `material3-adaptive` says the same thing in code -- `maxHorizontalPartitions = 1` for compact and medium alike. A chat transcript is precisely the dense content that rule is about. It is also what this app can support. `ChatRoomMessagingRoute` is navigated to from **eleven** places -- a DKG ritual finishing, room-type selection, the npub dialog, a profile -- so the conversation has to remain a pushed destination whatever the window is doing. The list pane is a second way to reach it on a wide window, not a replacement for the first. **Why not `ListDetailPaneScaffold`.** The dependency is available and resolves for every target; the scaffold was not used, and the reason is the paragraph above. It earns its API surface -- a navigator, a destination history, an `AnimatedPane` per pane, three experimental opt-ins -- by owning the single-pane case as well: showing the detail *instead of* the list on a phone and animating between them. This app cannot hand it that, so it would sit permanently in its two-pane state and amount to a `Row` with more words and a history nothing reads. What it does have that is worth keeping is its numbers, and `Panes.kt` takes them: 360dp of list at expanded, 412dp from large upward, 24dp between. A hand-built pair measures the same as the scaffold would. **Three smaller decisions.** The floating action button moves into the list pane when there are two. The `Scaffold`'s slot is the bottom-right of the *window*, which with two panes is on top of the transcript's send button; M3 puts a list-detail layout's primary action in the list pane. It is one composable used from both branches so the two cannot drift. `readableContent()` comes off the pair. Capping two panes together to one column's measure is the opposite of what a second pane is for -- each pane holds its own content instead, and the conversation already did. The detail pane says "Pick a conversation to read it here" rather than being an unexplained empty half of a window, and the conversation is keyed on the room so switching rebuilds its view models rather than feeding a new id to ones already subscribed to another room's relays. **Measured in real windows of the widths the phase names.** 400 and 700 are one pane; 1000 splits with a 360dp list; 1400 splits with a 412dp list. The repositories are the no-op ones with the two reads this screen makes delegated to a fixed answer -- Kotlin's interface delegation makes that ten lines rather than a reimplementation of two large interfaces. Five more unit tests pin the widths against the directive's, including that the detail pane still clears a 40-character line in the narrowest window that allows two of them. 635 jvm tests green; android compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
714354ae9a |
feat: give the app a navigation component, and stop the app bar duplicating it
Phase 6, step 3. The app had no navigation component of any kind: 43 screens reached by pushing a route, and one home screen whose top app bar carried the only two peer surfaces -- a profile avatar in the leading slot, a search icon in the trailing one. **This is an information-architecture change and was taken as one.** With a single top-level destination, a navigation bar would have held one item and been strictly worse than the app bar it replaced -- M3's caution is to swap only functionally equivalent components. Promoting search and profile to peer destinations is what makes a navigation component mean anything here, and it was put to the product owner rather than inferred. Answered: promote them. The consequence is in `HomeScreen`: the app bar now carries a title and nothing else. Two routes to one destination is the thing the caution is about, and the navigation component is now the one route, at every breakpoint. **Which component, at which breakpoint**, straight from the layout foundation: | compact | navigation bar | | medium, expanded | collapsed rail | | large, extra-large | expanded rail | `NavigationSuiteScaffoldDefaults.navigationSuiteType` is not used, and the difference is the last row -- it stops at the collapsed rail, because it classifies with the three-value window size class rather than the five breakpoints the May 2026 revision published. Deriving from `Breakpoint` reaches the row the library's default cannot, and keeps one source of truth for window width in the app. `NavigationSuiteType.None` on everything else. A navigation bar belongs on the destinations it switches between; on a chat room, a signing screen or an onboarding step -- pushed to and left by coming back -- it is a permanent invitation to lose your place. **Two things the wiring needed.** `ActiveProfileRoute` is addressed by metadata event id, not by public key, and only the home screen ever had one. The nav host now observes it for as long as a key is signed in, and the profile item is *disabled* until it arrives rather than absent -- an item that appears late moves the two beside it, and a bar whose items move under a thumb is worse than one briefly unavailable. The item click pops to `HomeRoute`, not to the graph's start destination. The android docs give the second shape and it would be wrong here: this graph starts at `LoadingRoute`, and onboarding clears the stack with `popUpTo(0)` on its way to home, so by the time these items exist the start destination is not on the stack at all -- popping to it would leave the loading screen underneath as the thing back returns to. **Tests, and one that could not be written.** The breakpoint-to-component table is a pure function so all five rows are asserted; the two rail rows differ only in whether labels are drawn, and nobody opens a 1200dp window on purpose. Four more compose the component around a real nav graph, because `TopLevelDestination.of` matches by `hasRoute` -- reflection over the serialized route -- and a renamed route would fail by never showing the component at all. Navigation in those is driven through the controller rather than by tapping an item. That is a harness limitation, established rather than assumed: a click handler that navigates trips navigation-compose's own main-thread assertion under `runDesktopComposeUiTest`, reproducible in twenty lines containing no app code -- a `NavHost`, two routes and a `TextButton`. What an item's `onClick` builds is asserted where it is a pure function instead. Also `material3-adaptive-navigation-suite`, versioned with material3 rather than with the adaptive library: it is published by the material3 group, and its 1.10.0-alpha05 is what names adaptive 1.2.0 in the first place. Most of the `MantraNavHost` diff is indentation -- the `NavHost` call gained an enclosing composable. `git diff -w` shows the 38 lines that are not. 626 jvm tests green; android compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
a53a9c3e11 |
feat: open the desktop window at a width the layouts are now written for
Phase 6, step 6. The window opened at 480x900 under a comment that said why: *"The layouts have only ever been exercised at phone widths. This is a starting size that does not immediately misrepresent them, not a considered desktop layout."* That was honest, and it has stopped being true. 1100dp is inside the expanded breakpoint (840-1199), which is the narrowest window M3 recommends two panes in and so the smallest opening size at which a desktop user sees a desktop layout rather than a phone one stretched sideways. The content does not stretch to fill it: screens are held to a readable measure and centred, so the extra width becomes margin. Also a minimum size, which the window never had. Compose Desktop's `WindowState` carries no minimum, so the window could be dragged narrower than anything in the app was written for; 400x600 is the narrowest of the five widths this phase is meant to be checked at, and the compact breakpoint's own floor is a phone rather than nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
e95ece9027 |
fix: give form helper text and the review list the leading edge of what they describe
Phase 6, step 5, second half: the plan asked to revisit the 91 `TextAlign.Center`
uses, on the grounds that start alignment is what gives the rulers something to
align to. Revisited, and 84 of them are right.
Centring is correct for a block that is the only thing on the screen, because
there is nothing for it to align to: an empty state, a loading or error message,
one of the six onboarding status screens, a "coming soon" placeholder, the
landing screen's hero, a dialog's title. Converting those would have been a
restyle wearing a conformance argument.
Seven were wrong, and they share one shape -- text sitting in a column *beside a
full-width element*, so there was a leading edge and it was being ignored:
- the two helper lines under `CreateProfileScreen`'s name and bio fields, and
the two under `ChatRoomCreationScreen`'s. Each `TextField` is
`fillMaxWidth()`, and its label, placeholder, leading icon and supporting
text all begin at the same edge; the sentence explaining the field floated
centred at whatever width it happened to be;
- `SelectChatRoomTypeScreen`'s "this decides who can change the group later",
which sits directly above three full-width cards;
- `CreateProfileScreen`'s confirmation list, where "Name" and the name below it
were each centred at their own width, so the label and the value it labels
started in different places. Five texts there now share one edge.
The parent columns are still `Alignment.CenterHorizontally`, which is why each of
these needed `fillMaxWidth()` and not merely the removal of `textAlign`: a `Text`
without a width in a centred column is centred as a box, so dropping the text
alignment alone would have changed nothing visible.
Nothing else in the sweep moves. The remaining 84 are listed above by category
rather than site because the category is the reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
05e80bf099 |
feat: hold every screen's content to a readable line, and centre it in the window
Phase 6, step 5, first half. Every one of the 40 screens rendered a single column that filled whatever width it was given, so on a 1800dp desktop window a paragraph became a 1800dp line -- long enough that the eye loses the start of the next one -- and a six-character text field stretched to 1700dp. M3: *"across all breakpoints, adjust margins and type styles to keep text between 40–60 characters per line."* **The measure is derived, not written down.** `readableContentWidth()` is `bodyLarge`'s font size converted through the current density, times half an em per character, times sixty: 480dp at the default text size. Writing `480.dp` instead would be the same number today and wrong for anybody who has turned text size up -- at 200% the same column holds thirty characters, silently, because the text still fits. Deriving it means the column widens with the type and keeps its sixty. `AverageCharacterAdvance` is the one estimate in it, named and documented, because a proportional face has no character width and half an em is the standard figure for mixed-case Latin prose. Only the ceiling is enforced. The floor needs nothing: a 400dp compact window less its two 16dp margins holds about 46 characters, which is inside the range, and no cap can add characters to a window that has none. There is a test for exactly that, so the claim is checked rather than asserted in a comment. **The column is centred; the text is not.** Those are opposite things and it is worth being explicit, because "centre it" is how the second one gets done by accident. A centred column still has one straight leading edge for every row, avatar and icon to align to, which is what the grids-and-spacing page asks for. Centred text has none. The 91 `TextAlign.Center` uses are a separate question and a separate commit. **Applied at 49 sites in one pass**, at the point every screen consumes its `Scaffold`'s padding -- the one place in each file that is reliably the top of the content. Below 480dp it is not a cap, an inset or a centring; it is nothing, so no phone layout moves. **Verified by measuring a real composition, not by reading the code.** `readableContent()` is `fillMaxWidth` then `wrapContentWidth` then `widthIn`, and every permutation of those three compiles and renders something that looks right in a phone-width preview. This needed `compose.desktop.uiTestJUnit4` in `jvmTest` -- pinned to the same 1.11.1 as the rest of Compose Multiplatform, test-only -- and `runDesktopComposeUiTest(width = 1400)`, which gives a window that genuinely is 1400 pixels across at density 1. Four assertions, and they bite: swapping the last two modifiers makes the 1400dp case report `Actual width is 1400.0.dp, expected 480.0.dp`, which is the "centred but never capped" failure the doc comment names. The same test also pins `currentBreakpoint()` to the real window at all five widths -- 400, 700, 1000, 1400, 1800 -- with the screen margin following. A version of it that measured the parent's constraints rather than the window would answer `Compact` everywhere and pass every unit test in the suite. 37 theme tests green; android and desktop both compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
b5bb0042b1 |
refactor: take the chat transcript out of the view model it was living in
Phase 6, step 7, and the prerequisite for the pane work rather than a tidy-up: the transcript has to render at 400dp as a whole screen and at 900dp as the detail half of a two-pane layout, and a layout that lives in a view model cannot be composed twice or previewed once. `ChatMessageListViewModel` was 1,113 lines, of which 380 were `RenderMessages` -- a `@Composable` member function holding a `LazyColumn`, a `DropdownMenu`, `Card`s and both of the app's only two `BoxWithConstraints` -- plus three private composables under the class. It is now 356 lines of state and coroutines, and `ui/composable/widgets/chat/ChatTranscript.kt` is 779 lines of layout. **The move is verbatim.** `ProposalsAwaitingYouNotice`, `PrivateMessageNotice` and `RitualNotice` are byte-identical -- `diff` says so. `RenderMessages` becomes `ChatTranscript` and differs by exactly the signature line and fourteen references that had been resolving against the enclosing class and now say `viewModel.`. Nothing was rewritten while it was in the air; the diff is small enough to read line by line, which is the only reason to move 760 lines in one commit. **Why a parameter and not a receiver.** Keeping it as `fun ChatMessageListViewModel.ChatTranscript(...)` would have made the diff a single word, and left every one of those fourteen reads bare. `openMessageActionsFor` read bare says nothing about where it is kept; `viewModel.openMessageActionsFor` says it survives the composition, which is the fact a reader of a transcript needs and the one a pane split will make load-bearing. **Two imports the extraction nearly lost.** `androidx.compose.runtime.getValue` and `setValue` are used implicitly, by `by mutableStateOf`, so a "drop imports whose name does not appear" pass drops both and the five delegated properties stop compiling. The compiler caught it; noting it because the same pass over the next file will do the same thing. The earlier version of that pass also required an import's name not to follow a dot, which silently dropped every `Modifier.fillMaxWidth()`-shaped extension. `:composeApp:compileDebugKotlinAndroid`, `:composeApp:compileKotlinJvm` and the jvm test suite all green. The three `Icons.Filled` deprecation warnings in the new file came with the code and are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
03d1e8e3b1 |
feat: give the app the five breakpoints, and let the screen margin follow them
Phase 6, steps 1 and 2. The app had no notion of window width at all -- two
`BoxWithConstraints` in 30,000 lines of UI, both inside a view model -- so every
layout decision in it was made once, for a phone, and then rendered unchanged
into a 1800dp desktop window.
**The dependency question the plan asked to settle first.** `material3-adaptive`
publishes multiplatform under `org.jetbrains.compose.material3.adaptive`, with
android, desktop and ios variants; the ios ones carry `ios_arm64` and
`ios_simulator_arm64` attributes despite the `uikit*` artifact names, so the
targets this project declares on a mac resolve. Version **1.2.0**, not the newer
1.3.0-beta02, because that is the version the pinned material3 itself resolves:
`material3-adaptive-navigation-suite:1.10.0-alpha05` names `adaptive:1.2.0` in
its pom, and 1.3.0 would pull window-core 1.5.0 in beside the 1.4.0 the pinned
material3 compiled against. Nothing is lost by staying: 1.2.0 already computes
the large and extra-large breakpoints through `supportLargeAndXLargeWidth`, and
carries `ListDetailPaneScaffold` for the pane work. So steps 3-4 can use the
library scaffolds rather than a hand-rolled equivalent.
**`Breakpoint`** is the five-value enum -- compact / medium / expanded / large /
extra-large at 0 / 600 / 840 / 1200 / 1600dp -- with `ofWidth` as a pure function
so the thresholds are assertable without a Compose runtime. `TorchTheme`
classifies once and provides `LocalBreakpoint`, so no two screens can disagree
about the window they are both in.
It reads `currentWindowDpSize()` rather than `currentWindowAdaptiveInfo()`.
The latter also computes a `Posture` from the platform's fold state, which on
android reaches for `WindowInfoTracker` and an activity; this call sits in
`TorchTheme`, which wraps all 51 `@Preview` bodies in the tree, and a preview
context is not an activity. The pane scaffolds ask for posture themselves, at
the one place a fold changes the answer.
**Spacing now adapts, and exactly one value moves.** M3 publishes a margin per
breakpoint -- 16dp compact, 24dp everywhere wider -- and publishes nothing else
that varies with window width. The scale itself is absolute: `space200` is 16dp
on a phone and 16dp on a desktop, and what adapts is which token a job reaches
for, not the token. So `screenMargin` goes 16 -> 24 at medium and holds there,
and `containerPadding`, `itemGap` and the rest do not move -- a card does not
become a different component because the window grew. Widening all of them is
the "everything breathes on a big screen" instinct, and it reads as a zoomed
phone rather than as a layout. A test asserts the non-movement, because that is
the edit a later reviewer would wave through.
Mechanically this made the eight semantic names constructor parameters instead
of `get()`s over the scale, so a breakpoint can reassign one without moving the
stop underneath it. Kotlin resolves a default expression against the parameters
before it, so each still reads its stop by name and still follows it when the
scale is overridden -- phase 2's `Spacing(space200 = 24.dp)` assertion holds
unchanged. The two instances are singletons because `LocalSpacing` is a
`staticCompositionLocalOf` and invalidates on identity, not equality.
**A test found a real defect while being written.** `ofWidth` was
`entries.last { width >= it.minWidth }`, which throws `NoSuchElementException`
below 0dp. A desktop window reports a zero size for the frame before its first
layout pass, and this is called from the theme on every composition, so the
crash would have arrived on a resize rather than on anything a user did. Now
total.
`:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` both
green; 28 theme tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
043d725599 |
feat: draw the expressive loading indicator, and stop shouting the sign-out button
Phase 5, second step, of docs/material-design-conformance.md. Two smaller pieces, and a
correction to the plan.
**41 loading states stopped being a gold spinner.** `LoadingDataIndicator` wraps every wait
in the app, and it drew a `CircularProgressIndicator` hardcoded to 80dp in
`colorScheme.secondary` -- the brand gold, which reads as a warning rather than as a wait,
on a component that has a size of its own. It now draws `LoadingIndicator`, which is M3's
component for an indeterminate wait with no progress to report and the one
`MaterialExpressiveTheme` expects to be paired with. One wrapper changed; 41 call sites
follow.
**The profile screen had two maximum-emphasis buttons, and one of them was Sign out.**
Seven actions in one list: five `TextButton`s (edit profile, key packages, change account,
profile keys, network relays) and two filled `Button`s. A filled button is M3's highest
emphasis and is meant for one action per screen, so this was two competing primaries -- and
the more prominent of the pair was the list's most destructive item.
Sharing is now `FilledTonalButton`: it is the useful action, at medium emphasis rather than
maximum. Signing out is a `TextButton` in the error colour, which is not a new pattern --
it is how leaving and deleting a group are already treated in `ChatRoomDetailScreen`.
Screenshot verified on emulator-5554: one tonal button, one red text button, five plain
ones, and a hierarchy a reader can follow.
**The plan was wrong about disabled FABs, and the code was right.** It said five screens
should stop hand-computing a container colour from a `can…` flag and pass `enabled`
instead. **No `FloatingActionButton` overload in material3 1.10 takes `enabled`** -- checked
in the source, zero matches for `enabled: Boolean` in FloatingActionButton.kt -- because
the spec's own position is that an unavailable FAB should not appear at all. Hand-computing
is the only way to show a disabled one.
More to the point, the existing code is already better than the plan assumed: it pairs the
colour with `Modifier.semantics { disabled() }` and a comment saying "looking unavailable
is not being unavailable: without this a screen reader still announces a button it is happy
to press." Left alone, and the plan corrected.
**Eight screens are left for a person.** LandingScreen puts "Sign in" beside "Create
profile", SocialPreconditionScreen puts "Invite a friend" beside "View invites", and six
others do the same. Both members of each pair are filled buttons. Which one is primary is a
product decision about what the screen is *for*, and picking wrong quietly weights a choice
the user is supposed to make freely -- so this is listed in the plan rather than guessed at
here.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, unchanged. Both
changes are composition-time rendering, which this repo has no UI test infrastructure to
assert; the device screenshot stands in for it. `:composeApp:compileDebugKotlinAndroid`
builds and the apk runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
44bf2a01f0 |
feat: give the app somewhere to report an outcome, and every dead-end error a way out
Phase 5, first step, of docs/material-design-conformance.md. Two absences, both structural.
**Sixteen copies of the same dead end.** The tree held sixteen instances of
Column(horizontalAlignment = CenterHorizontally) {
Spacer(Modifier.height(48.dp))
Text("Something went wrong")
}
and five of the same shape saying "No events were found". **Not one of the sixteen offered
a retry.** Every failure in this app named no cause and had no way forward but the back
button.
`ErrorState` and `EmptyState` replace all 21. Deliberately plain -- an icon, a line, and
for errors an action when the caller has one to give. `ErrorState`'s `onRetry` is nullable
so that passing null is a *decision* a reader can see, rather than the absence of a
parameter nobody thought about.
`EmptyState`'s message is **required**, with no default, and that is the point of the
change rather than a detail. "No events were found" was shown for five different absences:
nobody you follow, nobody following you, an empty feed, no replies, no search results. A
shared default would have preserved exactly that. They now read "You aren't following
anyone yet.", "Nobody is following you yet.", "Nothing in this feed yet.", "No replies to
this yet." and "Nothing matched that search." -- and `no_events_were_found` is deleted.
**Zero snackbars across 43 Scaffolds.** No `Snackbar`, no `SnackbarHost`, no
`SnackbarHostState` anywhere. Every transient outcome -- an invite failing, a key package
published, a message not sent -- had nowhere to be reported, so the code either said
nothing or navigated away and hoped.
`LocalSnackbarHostState` is a composition local rather than a parameter because of where
the reporting happens: a view model coroutine finishing a call is several composables below
the `Scaffold` that owns the host, and threading the state down would be the same plumbing
repeated 43 times and forgotten on the 44th. One host is provided in `MantraApp`; only one
Scaffold is composed at a time under a NavHost, so the message renders on whichever screen
is on top.
It **throws** rather than defaulting to a detached `SnackbarHostState()`. A default would
make `notify(...)` a silent no-op on any screen that forgot the host, which is precisely
the failure this file exists to end.
**Wired to a real action, not left as infrastructure.** `publishNewKeyPackage` and
`rotateKeyPackage` were fire and forget: you tapped, a coroutine ran, and nothing on screen
changed -- indistinguishable from a tap that missed. Both take an `onDone` and the screen
reports it. Verified on emulator-5554: tapping Publish shows "Key package published" and
the count goes 2 -> 3.
**Externalising the strings made four copy problems visible, which is the argument for
having done it.** With 364 strings in one file rather than scattered through 60
composables, `%1$s Key Packages`, `replying To %1$s` and **three surviving mentions of the
old product name** were sitting in plain sight. All corrected. (They had been fixed once
already and lost: the previous commit reverted the tree to fix an unrelated import bug and
re-ran the extractor over the original text. Worth recording, because it is what a
revert-and-redo costs when a script is the thing being iterated on.)
**And it made the title-case checker stop covering anything.** `m3-title-case.py` scanned
`.kt` files, so when phase 4 moved the strings out it went on reporting zero while the four
above sat in `strings.xml`. It now reads the catalogue too, and that path is verified by
flipping one entry to "Try Again" and watching it fail. Externalising narrows what a source
scan can see; the check has to follow.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, unchanged. The state
composables and the snackbar host are composition-time behaviour and this repo has no
Compose UI test infrastructure; what stands in for it is the device run above.
`m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
2117e22d48 |
refactor: make the 40 interpolated UI strings format strings, and assert the argument order
Phase 4, third step, of docs/material-design-conformance.md. `Text("Add chapter to
${uiState.artifact.name}")` becomes a resource holding `Add chapter to %1$s` and a call
passing the expression. 49 call sites. Literals in composables go 76 -> 39;
`stringResource` goes 374 -> 424.
**A silent bug in the previous commit's extractor, found by this one.** Imports were
tested with `statement in source`, and the generated accessors are named after their
strings -- so `import mantra.composeapp.generated.resources.translate` is a *prefix* of
`...resources.translate_into_which_dialect`. The substring test decided the import was
already there, and the compiler reported "Unresolved reference 'translate'" in a file
whose imports looked complete. Both extractors now match whole lines, and the helper
carries the explanation.
**Four filters, each earned by something the dry run got wrong.**
*A template that is only interpolation has nothing to translate.* `Text("$name")` would
have become a resource holding `%1$s` -- longer, slower, and no more localisable than the
code it replaced.
*A leading or trailing space means it is being glued to a neighbour.* " \\u00b7 %1$s" is a
separator. The test has to be on the format string rather than on the literal halves: a
template opening with an interpolation leaves the first part empty and the second starting
with the separating space, which makes "%1$s Key packages" look like a fragment when it is
a whole label.
*`\\uXXXX` and `\\"` are Kotlin syntax, not XML.* Left alone they would have shipped as the
six visible characters of the escape. They are decoded into the resource, which is UTF-8
and can hold `·` directly. `\\n` is **not** decoded, because
StringCatalogueJvmTest shows Compose Resources processes that one and a real newline in an
XML value would be reflowed by the parser.
*A term of a `+` concatenation is still not a string.* Same rule as the plain extractor.
**Three copy problems surfaced only here, because interpolated strings had never been
checked.** `m3-title-case.py` excludes anything containing `$` -- an interpolation is not a
literal -- so `"$count Key Packages"` had been invisible to every pass so far, as had
`"replying To ${…}"`. And a third instance of the old product name, in
`"...once they're on Torch."`. All three fixed. Worth noting as a gap in the checker rather
than a one-off: title case inside a template is still unchecked, and there are 83
concatenation fragments left where it could hide.
**Two new assertions, on the two things a compiler cannot see.** Argument *order* is
decided by where each `${…}` sat, and a transposition compiles and reads plausibly --
"Recovered 3 of 12" against "Recovered 12 of 3" -- so a two-argument and a three-argument
string are asserted end to end. The three-argument one doubles as the check that `·`
was decoded rather than passed through.
**What is deliberately left.** 83 literals that are terms of a `+` concatenation.
Reassembling `"a " + x + " b"` into one format string means deciding what the whole
sentence is, and several are pluralisations -- `(if (n == 2) "event" else "events")` --
which want a real plural resource rather than a format argument, and that is an API choice
rather than a rewrite. `m3-extract-formatted.py --remaining` lists them.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, up from 947/598/349.
`:composeApp:compileDebugKotlinAndroid` builds; the debug apk installs and runs on
emulator-5554 through onboarding, the message list and a chat room with its text intact.
`m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
419504c982 |
refactor: move 315 UI strings into the resource catalogue, and prove the escapes survive
Phase 4, second step, of docs/material-design-conformance.md. 251 distinct strings, 315
call sites, from literals inside composables to `stringResource(Res.string.…)`. Literals
in composables go 332 -> 76; `stringResource` goes 0 -> 374.
**The extractor took four attempts, and each failure is why it is checked in.**
*A bare `text = "…"` is not a Compose string.* `text` is an ordinary parameter name and
this tree uses it on data classes: `NavigationUIState.Loading(text = "…")` is not a
composable, and rewriting it failed with "@Composable invocations can only happen from the
context of a @Composable function". So `Text(`/`BasicText(` calls are brace-matched and
only literals genuinely inside one are touched.
*A regex over quote pairs is not a Kotlin lexer.* Matching `"[^"]*"` over a whole file
pairs one string's closing quote with the next string's opening quote, so "literals" came
out as several lines of Kotlin. Restricting the body to one line fixed that and left a
subtler version: `"a ${if (n == 1) "chunk" else "chunks"} b"` has two inner literals
belonging to an outer template, and left-to-right matching lifts them out as strings of
their own. The script decided `"chunk"` and `"note"` were UI text worth translating. It now
scans properly -- on an opening quote, walk forward tracking `${` depth, recursing over
nested literals, and stop at the closing quote at depth zero.
*A fragment is not a string.* `"a " + x + " b"` is one sentence in three pieces, and " b"
is not something a translator can work with -- word order differs between languages. Three
filters, because the fragments hide in three shapes: adjacent to a `+`, leading or trailing
whitespace or no letters at all (", " and ":"), and -- the one that needed a fourth pass --
a pluralisation where the *parenthesis* is adjacent to the `+` and neither literal is:
(if (proposal.eventCount == 2) "event" else "events") +
Testing the line rather than the literal catches those four sites while leaving a genuine
either/or alone: `if (session == null) "Start key ceremony" else "Try again"` has no `+`
and both branches are whole strings.
**Compose Resources is not aapt, and that was a bug this commit nearly shipped.** The
first version escaped apostrophes as `\'` and doubled `%`, which is what android's resource
compiler requires. Compose Resources does neither. `getString(Res.string.don_t_sign)`
returned
Don\'t sign
backslash included, and there are 30-odd apostrophes in this catalogue. Every one of them
would have rendered with a visible backslash, on screens nobody opens often.
What makes this worth a permanent test rather than a fixed script: escape handling is
**partial**, not absent. The same run showed `\n` *is* processed --
"Currently no messages have been shared.\nBreak the ice." comes back with a real newline.
So there is no family rule to rely on, and the next escape somebody adds needs checking on
its own.
`StringCatalogueJvmTest` asserts all three cases through `getString`, which is the
non-composable reader for the same resources and needs no composition. It found the bug
before a device did.
**Names are derived from content**, snake_cased and truncated at a word boundary:
`something_went_wrong`, `add_artifact_to_the_group_library`. The conventional shape for an
automated extraction, with a known cost -- rewording the copy leaves the name slightly
stale. The alternative, naming by *purpose*, needs somebody to read 315 call sites, and a
name asserting the wrong purpose is worse than one that is a little dated.
**1101 dead strings out, 251 live ones in.** The catalogue previously held the phoenix
wallet fork's entire string table with nothing referencing it; it now holds this app's own,
plus `app_name`.
**What is left, and why.** 76 literals: 46 interpolated, which need format placeholders and
an argument order decided per site, and 30 concatenation fragments, which need their
sentences reassembled first. Both are the next commit, and both are jobs where a script
should not guess.
**Tests.** 947 pass, 598 jvm over 73 classes and 349 android over 44, up from 944/595/349 --
three new assertions in one new class. `:composeApp:compileDebugKotlinAndroid` builds, the
debug apk installs and runs on emulator-5554 with its text reading correctly through
onboarding and the message list. `m3-audit.sh --check` exits 0.
`ChronicleApplyJvmTest` failed once during this commit's verification and passed on rerun;
it is the pre-existing 1-in-8 flake filed during phase 3, and nothing here touches
chronicle code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
0304aca62a |
fix: sentence-case every UI string, settle the product name, and empty the dead catalogue
Phase 4, first step, of docs/material-design-conformance.md. M3's style guide is
unambiguous: "All text, including titles, headings, labels, menu items, navigation
components, app bars, and buttons should use sentence-style capitalization. ... Don't use
title case capitalization." The tree was title case throughout.
**100 occurrences across 60 distinct strings**, in two passes, and the second pass is the
interesting one.
The first pass matched `[A-Z][a-z]+( [A-Z][a-z]+)+` in a `text =`, `Text(` or
`contentDescription =` position and found 41 strings, 73 occurrences: "Add Chapter",
"Sign In", "Key Package Management", "Publish New Key Package". Then the audit reported
zero and the app still had "Invite a Friend" on its first screen.
Two holes. The pattern required every word after the first to be capitalised, so anything
with an article in it survived -- "Invite a Friend", "Add to Group", "Name of Artifact",
"Sign in to Npub". And it read one line at a time, so a `Text(` whose literal sat on the
next line was invisible. A whole-file scan allowing lowercase articles found 19 more
strings, 27 occurrences.
**Sample data is deliberately left in title case.** "Steve Biko", "John Doe", "Frank
Talk", "To Kill a Mockingbird", "Man With A Plan", "Woman Of Few Words" are people and
titles of works, and title case is how those are written. The first audit swept them up
and reported 67 offenders where the real number was 41, which is the kind of number that
teaches a reader to ignore the tool.
Also untouched: the KDoc reference to iOS's own "Increase Contrast" setting, which is
Apple's capitalisation of Apple's setting, and `logger.d("Queried Sync")`, which is
written for whoever is reading logcat.
**Two strings changed meaning rather than just case.** "Sign in to Npub" became "Sign in
with an npub" -- npub is a protocol term, lowercase everywhere else in this app, and you
sign in *with* one rather than *to* it. "Lightning Bolt", a content description, became
"Lightning payment": M3's rule for a description is to name the purpose rather than the
picture, and "bolt" is the picture.
**The product has one name now, and it is Mantra.** The launcher label, the desktop window
title, the landing screen and the package all said Mantra; the home screen's app bar said
"Torch" and `composeResources`' `app_name` said "Machankura". The app bar is fixed.
`UserAgent.APP_NAME` still says "Torch" and is left alone on purpose -- it goes on the wire
to relay operators, so it is a network identity question rather than a content one, and a
comment at the call site says so.
**The two destructive actions now say what they do.** "Leave group" and "Delete group" are
`TextButton`s that fire immediately, with no confirmation step and nothing stating the
consequence. M3: "Tell users what will happen if they take an action and how they can undo
it."
Read out of the repository rather than guessed, because saying the wrong thing about a
destructive action is worse than saying nothing. `leaveChatRoom` sets `leftGroupAt` and
posts a line to the room; `softDeleteChatRoom` sets `deletedAt` on the local row and
nothing else. So: "Posts a line to the room saying you left, and lets you delete it from
this device afterwards", and "Removes the room from this device. The messages stay on the
relays and with the other members." The second matters most -- a button labelled "Delete
group" with no qualifier invites the belief that the messages are gone, which is the
opposite of true.
**1101 dead strings deleted.** `composeResources/values/strings.xml` held the phoenix
wallet fork's whole catalogue -- notification channels, electrum settings, swap timeouts
-- and **nothing referenced any of it**. The tree's only two `stringResource` calls are
both commented out, and one of them names an `R.string`, which does not exist in a Compose
Multiplatform resource set at all. Keeping them made the file look like the app's
catalogue while the app's actual 332 strings sat in composables. It now holds `app_name`
and a note about what happens next.
A trap for the next person, recorded in the file: the compose resources plugin reports an
XML comment containing a double hyphen only as "XML file ... is not valid. Check the file
content." XML forbids `--` inside comments, and this commit hit it while writing that
note.
**The audit's check is now a script, for the reason the second pass exists.**
`docs/scripts/m3-title-case.py` scans whole files, allows articles, excludes sample data by
name and skips logger calls. Budget ratcheted to 0. The grep it replaces was wrong in three
ways and reported success anyway, which is worse than not checking.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. The debug
apk installs and runs on emulator-5554. `m3-audit.sh --check` exits 0. The 332 literals
themselves are the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
1f24aaf4bb |
fix: give the two single-field screens their initial focus, and check 200% text on a device
Phase 3, final step, of docs/material-design-conformance.md. The tree had **zero** uses of
`FocusRequester`, `LocalFocusManager` or `focusProperties`, so no screen defined where
keyboard focus starts.
**Two places get it, and only two.** M3's flow guidance asks for an initial focus per
screen and, for a dialog, that "focus is set to the dialog component, likely to a specific
interactive element within the dialog such as a text input field":
- `StartDirectMessageToNpubOrNip05Dialog` -- one field and two buttons. Without this the
dialog opens with nothing focused, so a keyboard or switch user tabs in from wherever
focus happened to be.
- The desktop `PassphraseGate` -- the first screen of the desktop app, whose entire
content is one field, and where there is no tap to give it focus. Somebody who opens
the app and starts typing should not have to reach for the mouse first.
The other seven text-field screens deliberately do **not** auto-focus. Requesting focus
raises the software keyboard, and on a screen that leads with content somebody wants to
read -- AddArtifact's chapter list, WriteNewNote's reply preview -- that covers the thing
they came for. M3 asks for the initial focus to be *defined*, not for a field to be
grabbed; on those screens the definition is "the top of the content".
**Large text verified on a device rather than reasoned about.** Two passes:
A static one first, since the failure mode is a fixed height around text. All 23 fixed
vertical dimensions outside `Spacer`s are icons, images and progress indicators -- 12 to
40dp `.size()` calls, a 200dp image, a 180dp `heightIn` cap. Nothing wraps text in a fixed
box.
Then at `font_scale 2.0` on an API 36 emulator, three screens: onboarding, the message
list, and a chat room. All reflow without clipping. The chat room is the useful one --
system messages wrap to two lines and their timestamps and chevrons stay aligned, the
composer keeps its full width, and the transcript stays readable. `font_scale` was put
back to 1.0 afterwards.
The physical device attached to this machine was left alone. `font_scale` is a
system-wide setting and changing it on somebody's actual phone to test an app is not a
reasonable thing to do; a fresh emulator was booted for it instead.
**An unrelated flaky test, measured and left alone.** `ChronicleApplyJvmTest > an answered
catch-up leaves one line, whatever it took to deliver` failed once during this commit's
verification with
expected:<[chronicleRequested, chronicleReceived]> but was:<[chronicleReceived, chronicleRequested]>
and reproduces at **1 failure in 8** consecutive `--rerun` invocations on this tree. Both
transcript lines are written within the same second and the DAO's ordering has no
documented tie-break, so either order can come back. That is chronicle and database code;
nothing in this branch touches it. Whether it is a test bug or a real one -- two lines
swapping places in a user's transcript on reload would be a defect -- wants deciding by
somebody in that code, so it is filed rather than patched here.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. Focus and
window insets are properties of a running composition; there is no Compose UI test
infrastructure here, and a test asserting the modifier is present would restate the diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
9dc748f2d3 |
fix: lift the eight text-field screens above the software keyboard
Phase 3, third step, of docs/material-design-conformance.md. Nine screens and a dialog carry text fields. One of them called `imePadding()`; the audit had said seven screens, and was wrong about that too. `Scaffold`'s `contentWindowInsets` defaults to `systemBars`, which does **not** include the ime, so a `Scaffold` on its own does nothing about a keyboard covering the field being typed into. `Modifier.imePadding()` on the Scaffold lifts the whole screen, which is the standard shape and the one that needs no per-field handling. Eight screens get it: AddArtifact, AddChapter, AddDialect, TranslateChunk, ChatRoomCreation, CreateProfile, SignIn, WriteNewNote. Each carries a one-line comment saying why, since a bare modifier in a Scaffold argument list is the kind of thing that gets deleted in a cleanup. **ChatRoomMessagingScreen is deliberately not one of them**, and the reason is now written where somebody would look for it. Its composer already reserves its own bottom inset with `navigationBarsPadding()`. Adding `imePadding()` to the Scaffold as well would pad twice while the keyboard is up, because the ime inset already covers the navigation bar area that row is separately reserving. Getting that combination right wants a device with a keyboard open, not a compiler, and it is the one screen where the existing code shows signs of having been tuned by hand. **Not device-verified, and worth saying so plainly.** The emulator's account boots to a populated home screen, and reaching any of the eight means several hops through onboarding; what was confirmed is only that the keyboard interaction works on the path that was reachable -- the npub dialog's field moved from a bottom edge of y=1250 to y=840 with `mInputShown=true`, so ime handling is live on this build. The eight Scaffolds themselves were not each opened with a keyboard up. `ChatRoomCreationScreen`, reached through New Chat -> Start a group chat, is the shortest path for whoever checks. **Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. Window insets are a property of a running composition against a real window; there is no Compose UI test infrastructure here to assert them, and a test that the modifier is present would only restate the diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
831d1c0ad4 |
fix: give every tappable element a real target, and every icon a decided description
Phase 3, second step, of docs/material-design-conformance.md. Two accessibility rules the
tree had no way to hold: M3's 48x48dp touch target and 44x44dp pointer target, and its
requirement that a decorative visual be *annotated* as decorative rather than merely left
undescribed.
**Nineteen `.clickable` chains had no minimum size, and three were text-sized.**
`ArticleCard` and `LiveStreamCardContent` each make an author's name tappable -- a
`labelMedium`, around 16dp tall -- and `LinkPreview` does the same to a `bodyLarge` url
with 2dp of vertical padding. The other sixteen are cards, rows and full-screen boxes that
are already far larger.
`minimumInteractiveComponentSize()` is applied to all nineteen rather than to the three,
because it is a no-op on anything already 48dp and that makes the rule checkable by a
script instead of by measuring. Worth being precise about what it does, since the modifier
is easy to describe wrongly: it reserves 48x48dp of **layout**, not of touch handling --
touch expansion happens at the input layer regardless. Layout is what keeps adjacent
targets from overlapping, what satisfies M3's 8dp separation, and what a mouse pointer on
the desktop build actually has to land on.
**`Clickable.kt` had it built in and moved house.** The vendored ACINQ helper defaults to
`RectangleShape` and `PaddingValues(0.dp)`, so a `Clickable` is exactly as big as its
content -- and its call sites wrap a 20dp emoji and a row of wallet text. It now applies
the modifier unconditionally, before `.padding(internalPadding)`, since a size modifier
after it would re-impose the smaller constraint.
It also stopped declaring `package com.machankura.compose.ui.composable.widgets.buttons`
while living under `press/mantra/`. That is the second of the three package namespaces the
UI was spread across; `Type.kt` was the first.
**Eighteen `contentDescription = null` were indistinguishable from eighteen oversights.**
`null` is the *correct* API -- M3 asks that decorative visuals be "annotated as decorative
in order to hide them in code", and null is how that annotation is spelled in Compose. The
problem is that it reads identically whether somebody decided or never looked.
So `Decorative` is introduced -- a `String?` that is null -- and fifteen sites now say
`contentDescription = Decorative`. Same bytes, same behaviour, and the difference between
a decision and a gap is now visible in the source and countable by the audit. Each of the
fifteen has adjacent text saying what the icon says: a lock beside "Private to Ada", a
check beside "The group has a shared key.", an icon inside a button whose label is right
there.
**Three were not decorative and now carry their state.**
- `DkgRitualScreen`'s participant list -- a filled or empty circle beside each member.
The name says who; only the icon says whether they have contributed. Now "Contributed"
/ "Not yet contributed".
- `DkgRitualScreen`'s round header -- the title says which round and the count says how
far along; only the icon says whether it finished. Now "Complete" / "In progress".
- `ProposalListScreen`'s leading icon, which is the one this commit could not have left
alone: the previous commit took the red away from the failure state on the highlighted
card, because `error` is 2.67:1 there. The shape is now the only cue a sighted user
gets and the description is the only cue anyone else gets. Now "Awaiting your
signature" / "Signed" / "Failed" / "Waiting on others".
Descriptions follow M3's rule -- name the purpose, not the picture, and never the role.
"Contributed", not "green check", and never "Contributed icon", since the role is added
automatically and a screen reader would say it twice.
**Two new checks, replacing one that was asking the wrong question.**
`docs/scripts/m3-touch-targets.py` finds `.clickable` chains with no minimum size,
including chains broken across two lines. The audit used to count `.clickable` outright,
which is not a defect count: a clickable `Card` is fine and a clickable `Text` is not, and
only the modifier tells them apart. The audit also now separates `contentDescription =
null` (untriaged, budget 0) from `Decorative` (decided, reported at 15).
Both budgets ratcheted to 0, dated in the file.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged -- these
are layout and semantics properties, and this repo has no Compose UI test infrastructure to
assert them against a running composition. What stands in for it is the two scripts, which
check the property that *can* be checked statically: that the modifier and the decision are
present at every site. `:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh
--check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
4fe47c7d46 |
fix: derive every call-site colour from its container, ending nine contrast failures
Phase 3, first step, of docs/material-design-conformance.md. The generated palette was
already sound -- every `onX`-on-`X` pair in all six schemes clears 4.5:1 -- and every
failure in the app came from a colour reached for at the call site instead of derived from
what it sits on.
**The worst one made the app's most important rows invisible.** `ProposalListScreen` put a
`ListItem` inside a `Card` and overrode only the card's container:
Card(colors = CardDefaults.cardColors(containerColor = primaryContainer)) {
ListItem(colors = ListItemDefaults.colors(containerColor = Color.Transparent),
`cardColors(containerColor = …)` does derive `contentColor = contentColorFor(…)`, so
`LocalContentColor` inside the card was correct. `ListItem` does not read
`LocalContentColor`. Its headline comes from `ListTokens.ItemLabelTextColor`, which is
`onSurface`, and in the light scheme `onSurface` and `primaryContainer` are both `#1B1B1B`.
Measured on that card:
headline (onSurface) 1.00:1 invisible
leading icon (primary) 1.22:1
supporting (onSurfaceVariant) 1.84:1
"could not be read" (error) 2.67:1
onPrimaryContainer 4.61:1 the only one that worked
Four of five below the floor, and the card is applied to exactly `proposal.awaitsYou` --
the proposals waiting on your signature. Dark was fine throughout, because there
`primaryContainer` is black, so this only ever showed in the light scheme.
The card's colours are now computed once and everything inside derives from
`cardColors.contentColor`: the six `ListItemColors` slots, the leading icon tint, the
"Review" label, and the unreadable-count line. `primaryContainer` is kept as the highlight
so this stays a fix rather than a restyle -- `secondaryContainer`, the brand gold, would
read more like "this needs you", and that is a design call recorded in a comment rather
than taken here.
On the highlighted card the failure state loses its red, because `error` is 2.67:1 there.
The signal survives in the icon and in the sentence "could not be read", which is the more
robust cue anyway and the only one available to somebody who cannot distinguish the red.
**`HomeScreen`'s top bar lost its override entirely.** `containerColor = primaryContainer`
with `titleContentColor = primary` is `#000000` on `#1B1B1B`: **1.22:1**, a black title on
a near-black bar. `TopAppBarDefaults` gives `surface`/`onSurface` and needed no help.
**Three of the four `alpha = 0.5f` sites were not text, which changes what they failed.**
The audit called them caption text; they are `CircularProgressIndicator` colours, so the
threshold is 3:1 rather than 4.5:1. At 2.49:1 they fail either way, but the plan said the
wrong thing and is corrected. The one that really is text -- `ArticleCard`'s published-at
timestamp at `alpha = 0.7f`, 3.96:1 -- is the fourth. All five now use `onSurfaceVariant`
at full opacity, 7.25:1, which is the role for secondary text and needed no alpha to
become one.
**The LIVE badge was a hand-mixed red.** `Color(0xFFE53935)` with a white label is 4.23:1,
under the floor for `labelSmall`. `error`/`onError` is the role for a red that has to be
read and is 6.46:1.
**The avatar picker used a content colour as a background.** `onSurface` at 50% composited
to a mid grey 2.49:1 from the unselected cells beside it -- so which emoji was selected was
close to unreadable. Now `secondaryContainer`, M3's role for a selected item. Worth being
straight about the limit: that role is 1.65:1 against the surface in this palette, which M3
accepts because its own selected states carry a second cue, an outline or a checkmark. This
grid has neither. Adding one is component work, and the comment and the plan both say so
rather than leaving it looking finished.
**Three colours stay hardcoded, and each says why at the site.** A new
`// m3-color-exempt: <reason>` marker, matching the spacing convention from phase 2, and
the audit honours it:
- `QRCodeView` -- a QR code is read by a camera. Scanners need maximum luminance
contrast between the modules and their background, and under dynamic colour
`onSurface`/`surface` could be two mid tones and unscannable.
- `FullScreenImageViewer`'s close button -- it floats over an arbitrary photograph, so
no role is safe behind it. A translucent scrim with white on it is M3's own
full-screen media treatment and the only pairing that holds over both a white sky and
a black one.
- `LoadingAsyncImage`'s spinner, but only when a blurhash placeholder is behind it. With
no placeholder the surface is known and the role is used.
Exemptions belong at the call site: the reason travels with the code and a reviewer sees it
in the diff that adds it, rather than in a list of file names in the audit script.
**Two colours were tokenised without moving a pixel.** `Color.Black` on the blank route's
`Surface` and on the image viewer's backdrop are both `scrim`, which is `#000000` in every
one of this app's six schemes. Same bytes, and the value now travels with the theme.
**A new assertion for the case the others structurally cannot catch.** A translucent
container has no contrast ratio of its own -- it has one only once composited -- so
`ColorSchemeContrastTest` grows an eleventh test that composites the two remaining tinted
containers over `surface` and measures the result, in all six schemes, naming the call site
in the failure. The pairings this commit *fixed* are not restated: once the proposal card
derives its colours, the pair it produces is `onPrimaryContainer` on `primaryContainer`,
which the first assertion already walks.
**Audit budget for hardcoded colours ratcheted 9 -> 0**, dated in the file.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, up from 942/594/348.
`:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
8cd0f3f44d |
refactor: take the last 353 spacing literals onto the scale, and reach zero
Phase 2, final step, of docs/material-design-conformance.md. Every `.dp` literal in a
spacing position in the UI tree is now a token. 431 reads of `MaterialTheme.spacing.*`, one
reasoned exemption, and `m3-spacing-positions.py` exits 0.
**Shape decides the token, not just the value.** The migration script grew a per-shape
mapping because the same number means different things in different positions: 8dp of
padding is `compactPadding`, 8dp of gap is `itemGap`, and 8dp under a `Spacer` is neither
of those and stays `space100`. Where the pair determines the meaning the semantic name is
used, and nowhere else:
padding + 8dp -> compactPadding 10 sites
padding + 16dp -> containerPadding 10
gap + 4dp -> relatedGap 8
gap + 8dp -> itemGap 10
That is 38 of 353. The rest take the raw stop, and deliberately: assigning a semantic name
needs somebody to have read what the container *is*, and a name that asserts a meaning the
code does not have is worse than a stop that asserts none. `screenMargin` in particular is
unassignable mechanically -- it is 16dp of padding, exactly like `containerPadding` -- so
it has no call sites yet and gets them when someone reads the screens.
**Two spacers were standing in for zero.** `WriteNewNoteScreen` renders
`Spacer(Modifier.height(1.dp))` twice, in the `LazyColumn` item that shows a reply preview
when there is one. There is nothing to show and the item still has to render something;
1dp was the placeholder. Now `space0`, with a comment, because a 1dp gap that nobody
intended is the kind of thing that gets copied.
**One value is exempt, and says so at the site.** `SovereignWalletStartupScreen`'s
`Spacer(Modifier.height(128.dp))` is room to scroll the last wallet clear of the bottom of
the window -- reserved space, not a step in the rhythm. The scale tops out at `space900`
(72dp) and rounding to it would put the row back under the edge.
Rather than exempt it in the script by value, the classifier now honours an inline
`// m3-spacing-exempt: <reason>` comment on the lines directly above. Exemptions belong at
the call site: the reason travels with the code, a reviewer sees it in the diff that adds
it, and the tool stops accumulating a list of numbers that mean nothing on their own -- the
mistake the first version of this audit made with `DIMENSION_EXEMPT`.
**Where the tokens landed.** `space125` (10dp) 128 times and `space250` (20dp) 107 -- the
two values that already dominated the tree, now named. `space600` (48dp) 52 times, which is
the empty-state spacer from the previous commit. The long tail is 2, 4, 6, 12, 14, 16, 24,
32, 40 and 64dp.
**Verified that nothing moved.** The landing screen was captured on emulator-5554 before
and after and compared pixel by pixel on a 4px grid: **47 differing samples out of
162,000, 0.03%**, and they are the status bar clock. The sweep is a rename.
**Budget ratcheted 353 -> 0**, dated in the file. Phase 8 wires `--check` into CI, at which
point a new `.dp` in a `padding()` fails the build.
**Tests.** 942 pass, 594 jvm over 72 classes and 348 android over 44, unchanged --
`SpacingScaleTest` already asserts the scale, and there is nothing to assert about a
call site having been renamed that the compiler does not.
`:composeApp:compileDebugKotlinAndroid` builds, the debug apk installs and runs,
`m3-audit.sh --check` exits 0. 75 files, 432 insertions, 348 deletions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
f7a732d68a |
refactor: move the 89 off-grid spacing values onto the M3 scale
Phase 2, second step, of docs/material-design-conformance.md. 77 of the 89 literals that
were off M3's spacing scale sat in spacing positions and now read
`MaterialTheme.spacing.spaceNNN`; the remaining 12 are dimensions and are out of scope.
One drifted corner moved onto the shape scale.
**The mapping, and why each is the nearest stop rather than the nicest number.**
5.dp x10 -> space50 (4dp) padding and gaps in dense rows
15.dp x14 -> space200 (16dp) card and dialog padding, two gaps
30.dp x1 -> space400 (32dp) the spacer under LoadingDataIndicator's spinner
50.dp x52 -> space600 (48dp) the spacer above an empty or error message
Nearest-stop throughout, so the largest move is 2dp and most are 1. `5.dp` is equidistant
between `space50` and `space75`; it goes to 4dp because `spacedBy(4.dp)` is already the
idiom elsewhere in the tree and a scale with two answers for the same input is not one.
The 52 at 48dp are the same three lines copied into 16 files -- a `Spacer` pushing
"Something went wrong" down the screen. Phase 5 retires them into a shared empty-state
composable; migrating them first means that composable inherits a token rather than
another literal.
**One shape, and it is the argument for having a scale at all.**
`RoundedCornerShape(30.dp)` in `TextNoteEventDetail` was the only hand-written corner off
the M3 scale, at 30dp against `extraLarge`'s 28. Two units: invisible beside any single
other card, and exactly the drift that happens when the value is a literal. It is now
`MaterialTheme.shapes.extraLarge`, the first call site for the scale `Shape.kt` documented.
**Rewritten by a script that reads call shapes, not values, and it is checked in.**
`docs/scripts/m3-migrate-spacing.py` brace-matches three call shapes -- `padding(...)`/
`PaddingValues(...)`, `Arrangement.spacedBy(...)`, and a `.height()`/`.width()` whose
enclosing call is `Spacer(` -- and rewrites only literals that fall inside one. A
`.size(18.dp)` icon, a non-Spacer `.height()`, a `RoundedCornerShape` or a `BorderStroke`
can never be caught, which a regex over `\\d+\\.dp` would have done to all of them. It
inserts the two imports where they are missing and skips comment lines. Dry run by default.
**The audit was measuring the wrong thing, and this is where that showed.** It split
literals by value against a hardcoded `DIMENSION_EXEMPT` list -- and the split is not a
property of the value. `16.dp` is a spacing stop *and* a plausible icon size. `50.dp` was a
`Spacer` height in 52 places and a divider width in one, and no list of numbers separates
those. `docs/scripts/m3-spacing-positions.py` replaces it with the same brace-matching
parse the migration uses, so the audit and the migration agree by construction; the audit
now reports **353 spacing literals** left and 76 dimensions out of scope, and the exemption
table is gone.
That reframes phase 2's acceptance criterion into something checkable: spacing positions to
zero, dimensions untouched. The script exits 1 while any spacing literal remains.
**What is left off-scale, and why none of it is a defect.** Twelve dimensions: avatar sizes
at 35, 55, 70 and 75dp, icon sizes at 18 and 22dp, and a 50dp divider width. Avatar and
icon sizing is a component-spec question rather than a spacing one -- M3 gives icons 18/20/
24/40/48 and says nothing about avatars -- and the plan puts per-component specs after the
adaptive phase. They are reported rather than exempted so the number stays visible.
**Tests.** 942 pass, 594 jvm over 72 classes and 348 android over 44, unchanged --
this commit adds no assertions, and the ones it could add (`SpacingScaleTest`) landed with
the scale. `:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh --check` exits 0.
Pixels move by at most 2dp, in 30 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
73d99f9a41 |
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>
|
||
|
Kgothatso Ngako
|
47bdb6f976 |
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>
|
||
|
Kgothatso Ngako
|
52b57769e1 |
feat: adopt MaterialExpressiveTheme, and give shape, type and motion a named home
Phase 1, steps 3, 4 and 6 of docs/material-design-conformance.md. `TorchTheme` passed
`MaterialTheme` a colour scheme and a typography and nothing else, so shape and motion were
whatever the library defaulted to and there was nowhere to write down what any of it was
for.
**Expressive, by decision rather than by drift.** The plan deliberately left
`MaterialExpressiveTheme` vs `MaterialTheme` open, because it changes component defaults
app-wide and is a product call. Put to the product owner on 2026-09-08 and answered
expressive. The pinned material3 1.10.0-alpha05 ships the whole set -- `ButtonGroupKt`,
`SplitButtonKt`, `FloatingToolbarKt`, `LoadingIndicatorKt`, `ShortNavigationBarKt`,
`WideNavigationRail`, `MaterialShapesKt` -- and the tree already opts into
`ExperimentalMaterial3ExpressiveApi` in 66 places, so this makes explicit what the imports
had already assumed.
**All four slots are passed explicitly, and that is the point.**
`MaterialExpressiveTheme` defaults its colour scheme to `expressiveLightColorScheme()` and
its shapes and typography likewise -- Material's values, not this app's. Leaving any slot
to that default is the same class of accident as the twelve unassigned fixed roles fixed
two commits ago: it compiles, it renders, and it renders somebody else's design.
**No visual change on the screens checked, and that is worth stating rather than
assuming.** Measured on the API 36 emulator: the "Invite a Friend" button is byte-identical
before and after -- same fill `#4E5E8B`, same 357px box at the same y -- because the
expressive default for a `Button` at default size matches the baseline in this version.
What expressive actually buys is elsewhere: `LocalUsingExpressiveTheme` gating component
behaviour, the three increased shape steps, the fifteen `...Emphasized` type roles, and the
components phases 5 to 7 are built on.
**`MantraShapes` is baseline `Shapes()`, on evidence.** The corners hand-written across the
tree already land on the M3 scale --
RoundedCornerShape(4.dp) x3 = extraSmall
RoundedCornerShape(12.dp) x11 = medium
RoundedCornerShape(16.dp) x2 = large
RoundedCornerShape(30.dp) x1 ~ extraLarge (28dp)
-- so overriding the scale would restyle the app for no reason. What is wrong is that they
are literals, which is how the last one drifted two units off the scale and why none of
them can move per breakpoint later. `Shape.kt` documents the eight steps and what each is
for; migrating those seventeen call sites is a later phase, and this is what they migrate
onto. Declaring it explicitly rather than relying on the default gives the note somewhere
to live.
**`MotionScheme.expressive()` is wired and unused.** Nothing in the app animates today --
one `animateScrollToPage`, no `AnimatedVisibility`, no navigation transitions -- so this
buys nothing yet. It is here so that when the motion phase starts, every spec comes from
the scheme rather than from a literal `tween`, and the app's feel is one decision instead
of forty.
**`Type.kt` left `com.example.ui.theme`.** It has been declaring that package while living
under `press/mantra/compose/ui/theme/`, one of three namespaces holding live UI code in
this tree. The move is mechanical; the doc comment on it is not. It records what each type
family is *for* -- `display*` for a screen's identity, `headline*` for section tops,
`title*` for headers and list headlines, `body*` for anything read as a sentence, `label*`
for **component text only** -- because the audit's finding is not that the scale is wrong
but that 92 of 240 reads are `label*` while `display*` and `headline*` carry 9 between them
across 43 screens. A UI at one pitch. The file stays baseline; the rule now has a home for
the sweep that fixes the call sites.
**The desktop unlock screen renders in the app's theme for the first time.**
`PassphraseGate` sat in the `else` branch beside `MantraApp`, which applies `TorchTheme`
itself -- so the gate composed under the default `MaterialTheme` and its
`colorScheme.error` and `typography.headlineSmall` were baseline M3. It is the first screen
a desktop user sees. `TorchTheme` now wraps both branches.
That wraps the unlocked branch twice, deliberately. `MantraApp` keeps its own `TorchTheme`
because android and ios enter through it and would lose the theme entirely if it moved out;
a second application of identical values costs one `CompositionLocalProvider` composition.
The comment says so, since the redundancy looks like an oversight.
**Dynamic colour stays on, by decision.** Also put to the product owner: on Android 12+
`dynamicColor = true` wins unconditionally, so the six schemes are used only below Android
12, on ios and on desktop, and a modern phone paints the wallpaper palette. Answered keep
as-is. A comment on the selection in `TorchTheme` now says this outright, because otherwise
the next person to change `Color.kt` and see nothing happen on their phone will assume the
change did not work.
**Tests.** 926 pass, 586 jvm over 71 classes and 340 android over 43, unchanged -- this
commit adds no assertions, because what it changes is either a library default (nothing to
assert that the compiler does not) or a doc comment. `:composeApp:compileDebugKotlinAndroid`
and `:composeApp:compileKotlinJvm` build, the debug apk installs and runs on emulator-5554
under the expressive theme, `m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
21d57eba54 |
feat: honour the platform's contrast setting, reaching four schemes that were dead code
Phase 1, step 2 of docs/material-design-conformance.md. `Color.kt` has carried medium-
and high-contrast variants of both themes since it was generated -- 156 colour values,
wired into `lightColorScheme`/`darkColorScheme` in `Theme.kt`, and never selected.
`TorchTheme` chose between `darkScheme` and `lightScheme` and nothing else, so a user who
turned contrast up in Accessibility settings got no change at all.
M3's accessibility foundation leads with *honour individuals*: "supporting varying
preferences and choices that allow individuals to address how their changing conditions,
individual knowledge, and varying needs are met." The work to do that was already done and
disconnected.
**The expect/actual boundary moved, because it was in the wrong place.** `themeColorScheme`
took four arguments and did two unrelated jobs -- decide the contrast-free light/dark
scheme, and decide whether to prefer a wallpaper palette. Adding contrast to it would have
meant passing six schemes across the boundary and repeating the selection table in three
actuals. It splits instead into `platformThemeContrast()` and `dynamicColorScheme()`, each
answering one narrow platform question, with the six-way table as a plain function
`appColorScheme(darkTheme, contrast)` in common code. `dynamicColorScheme` returns null
rather than falling back internally so the fallback stays in one place.
**Android reads the setting and listens for changes.** `UiModeManager.getContrast()` is
API 34; the app's minSdk is 26, so below that the answer is Standard. The float is snapped
to the nearest of the platform's three documented positions rather than matched exactly, so
a future finer-grained slider degrades to the closest scheme this app has instead of
falling back to Standard.
The `ContrastChangeListener` is the part that is easy to leave out and matters most. A
contrast change does not restart the activity and does not arrive as a `Configuration`
update, so without it the new setting would take effect on the next cold start -- which is
precisely the case the setting exists for. `context.mainExecutor` rather than
`ContextCompat.getMainExecutor`: it needs API 28, this branch is already gated on 34, and
composeApp does not declare androidx.core -- it only arrives transitively through
activity-compose, which is not a dependency to lean on.
**iOS observes the notification for the same reason** --
`UIAccessibilityDarkerSystemColorsEnabled` plus
`UIAccessibilityDarkerSystemColorsStatusDidChangeNotification`. It is a boolean, not a
slider, so iOS reports High or Standard and never Medium.
**Desktop is honest rather than complete.** Windows publishes high contrast as the
`win.highContrast.on` AWT desktop property and fires a property change when it is toggled,
so that path is real and live. macos "Increase contrast" and the linux desktop equivalents
do not reach AWT, and reading them means a native call per platform, so on those two the
answer is Standard and the file says so. This is the right place for a user-overridable
preference later; a desktop app cannot always see what the desktop was told.
**Verified on an emulator, at the pixel.** API 36, dynamic colour temporarily switched off
(see below for why that is necessary), sampling the `onPrimaryContainer` pixel of the "Skip
for now" label as `settings put secure contrast_level` moved:
standard (0.0) #848484 onPrimaryContainerLight
medium (0.5) #A7A7A7 onPrimaryContainerLightMediumContrast
high (1.0) #D0D0D0 onPrimaryContainerLightHighContrast
The three declared values exactly, and **the app was not restarted between them** -- only
the setting changed, four seconds apart. That is the listener working end to end. The probe
that switched dynamic colour off is reverted in this commit; the emulator's contrast_level
is back at 0.0.
**A finding that came out of the verification, and is not fixed here.** `TorchTheme`
defaults `dynamicColor = true`, and on Android 12+ dynamic colour wins unconditionally --
so on essentially every current Android device **none of the six schemes is used at all**
and the app renders in whatever the user's wallpaper produced. The first screenshot of this
session shows the onboarding screen in Material lavender; switching dynamic colour off
reveals the black-and-gold brand for the first time. Nobody on a modern Android has been
seeing this app's palette.
That is a product decision, not a conformance one, so it is recorded in the plan's "What
this plan does not cover" rather than changed. It does bound what this commit buys: on
Android 14+ with dynamic colour on, contrast is honoured by the platform anyway (the
`system_*` resources shift with it, confirmed on the same emulator -- buttons went
slate-blue to near-black navy). What this commit reaches is Android below 12, Android 12-13,
iOS, and desktop.
**Three new assertions.** `AppColorSchemeSelectionTest` covers the table itself, because its
failure mode is silent and specific: a scheme wired to the wrong cell still renders a
complete, plausible UI, and somebody who turns contrast up and gets the medium scheme back
cannot tell it apart from a high-contrast scheme that is not very high. It asserts each of
the six cells by identity, that all six are distinct objects (a copy-paste leaving two cells
on the same scheme would pass the first test only if it also mislabelled one), and that
`onSurface` on `surface` never *falls* as contrast rises -- the one direction that must
hold, and deliberately not the full monotonicity assertion that ColorSchemeContrastTest
explains is false.
**Not compiled: the iOS actual.** The ios targets are declared only on macos (see
docs/jvm-target.md), so `Theme.ios.kt` is written against the UIKit and Foundation bindings
rather than checked by a compiler. Its file comment says so. The android and jvm actuals of
the same two functions are compiled, and the android one is verified on a device.
**Tests.** 926 pass, 586 jvm over 71 classes and 340 android over 43, up from 920/583/337.
`:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` build,
`m3-audit.sh --check` exits 0. The 54 existing `TorchTheme { }` call sites are untouched --
the new parameter is defaulted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
86c9628eee |
fix: assign every ColorScheme role, so no component can fall back to Material lavender
Phase 1, step 1 of docs/material-design-conformance.md. `Theme.kt` assigned 36 of the
49 roles `androidx.compose.material3.ColorScheme` declares. The other thirteen took
`lightColorScheme()`/`darkColorScheme()` defaults, and for twelve of them that default
is the Material baseline palette: `primaryFixed` -> `ColorLightTokens.PrimaryFixed` ->
`PaletteTokens.Primary90` -> **#EADDFF**. Lavender, in an app whose primary is
`#000000`, in both themes, in all six schemes.
Nothing in the tree reads a fixed role today, which is why nobody has seen it. That
also means it could not have been found by looking at the app -- it springs the first
time an expressive component reaches for one, and it will look like a rendering bug
rather than a missing assignment.
**The tones were computed, not chosen.** M3 defines the family by tone: `xFixed` =
tone 90, `xFixedDim` = 80, `onXFixed` = 10, `onXFixedVariant` = 30, and ColorLightTokens
and ColorDarkTokens carry identical values for all twelve -- theme-independence is what
"fixed" means. Tone is CIE L*, so for a chroma-0 palette a tone is exactly the sRGB grey
at that L*, and inverting L* -> Y -> sRGB reproduces this palette's own greys **to the
byte**:
tone 0 #000000 primaryLight
tone 10 #1B1B1B primaryContainerLight, onSurfaceLight
tone 20 #303030 onPrimaryDark, inverseSurfaceLight
tone 40 #5E5E5E inversePrimaryDark
tone 80 #C6C6C6 primaryDark, inversePrimaryLight
tone 90 #E2E2E2 onSurfaceDark, surfaceContainerHighestLight
tone 95 #F1F1F1 inverseOnSurfaceLight
tone 100 #FFFFFF onPrimaryLight
Eight independent hits. The primary and tertiary palettes are the standard M3 neutral
tonal palette at chroma 0, so their fixed families are derived rather than invented.
**The secondary palette is gold at Lab hue 87.5 degrees, and its dark half is maximum
in-gamut chroma at that hue.** Generating tones off that ramp regenerates
`onSecondaryDark` (#3D2F00, tone 20) and `secondaryLight` (#745B00, tone 40) byte for
byte, which is what licenses using it for tones 10 (#241A00) and 30 (#584400).
Its tones 90 and 80 are **reused rather than regenerated**. The palette already ships
#FFDE82 at tone 90 (as `secondaryDark`) and the brand gold #EFBF04 at tone 80 (as
`secondaryContainer`, identical in light and dark -- someone hand-set it, no generator
emits that). Regenerating would have produced #FFDF99 and #F1C100: a second gold two
units from the one already on screen, indistinguishable in isolation and wrong beside
it. A near-duplicate brand colour is worse than none.
**Sanity check on the whole derivation.** The four ratios these families produce land
within 0.1 of M3's own baseline fixed family --
onFixed on Fixed 13.30 (baseline 13.32)
onFixedVariant on Fixed 7.17 (baseline 7.23)
onFixed on FixedDim 10.08 (baseline 10.08)
onFixedVariant on Dim 5.44 (baseline 5.47)
-- because tone, not hue, sets the ratio. Two palettes with nothing in common landing
on the same four numbers is the check that the tone mapping is right.
**Containers hold across the contrast setting; content darkens.** That is the move
`Color.kt` already makes everywhere else -- `onSurfaceLight` goes #1B1B1B -> #111111 ->
#000000 while `surfaceLight` stays #F9F9F9 through all three -- so the fixed family
follows it: content tones 10/30, then 5/20, then 0/10. The weakest pair ladders
5.44 -> 7.73 -> 10.08. Shifting the containers instead would have moved the brand-visible
half for a setting that is about legibility.
**`surfaceTint` is the thirteenth, and it was never a defect.** Its default is `primary`,
which is correct: `surfaceColorAtElevation` composites it over `surface` at 2-8% alpha,
so an elevated light surface darkens toward primary and an elevated dark one lightens --
M3's own behaviour, and this app sets no elevations anywhere, so nothing reads it. It is
assigned explicitly anyway, with that reasoning in a comment, so that "every role is
assigned" is a property a reader can check by looking rather than by knowing which
omissions were deliberate. m3-audit.sh reports the two kinds apart for the same reason.
**Three new assertions, and the two that matter cannot be satisfied by accident.**
`ColorSchemeContrastTest` grows from 4 to 7:
- both content roles on both fixed containers at 4.5:1, across all six schemes;
- the fixed roles are the same colour in light and dark, which is the definition and
would otherwise only fail on a screen that puts one beside a themed surface;
- no role is left at the Material baseline palette -- the twelve baseline hex values
read out of `PaletteTokens.kt` and asserted absent.
Verified by deleting `primaryFixed = primaryFixed,` from `lightScheme` alone: two tests
fail, naming the role and printing back `Color(0.917, 0.866, 1.0)`. Reverted.
**Audit budget ratcheted 12 -> 0**, dated in the file. Per the header's contract that is
the only direction a budget moves, and the commit that lowers it is the one that earns it.
**Tests.** 920 pass, 583 jvm over 70 classes and 337 android over 42, up from 914/580/337
-- three new assertions counted once per target. `:composeApp:compileDebugKotlinAndroid`
builds, `m3-audit.sh --check` exits 0. No visual change: every role that had a value keeps
it, and the thirteen that gain one were rendering baseline defaults nothing reads yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
2b0ce8d73b |
test: measure M3 conformance instead of asserting it, with a budgeted audit and a contrast test
Phase 0 of docs/material-design-conformance.md. Every count in that document was produced by hand, which makes the eight phases after it opinions rather than work with acceptance criteria. This is the harness that turns them back into numbers. **`docs/scripts/m3-audit.sh` regenerates the whole audit, and can fail a build.** Plain invocation reports; `--check` exits 1 when a budget at the top of the file is exceeded. The budgets are the tree as it stands -- 11 hardcoded colours, 33 bare `.clickable`, 18 null content descriptions, 12 unassigned colour roles -- and the contract written into the header is that they ratchet **down**, in the same commit that earns the reduction, and are never raised. Counts a phase has not reached yet are `-1`, which reports but never fails. Phase 8 wires `--check` into CI, at which point a raised budget is the diff a reviewer is looking for. Verified both directions: `--check` exits 0 on the clean tree, and appending a single `Color(0xFF00FF00)` to LoadingScreen.kt makes it exit 1 naming the budget. **Two counts are reported apart from each other on purpose.** Thirteen ColorScheme roles are never assigned in Theme.kt, and reporting that as one number would overstate it. Twelve are the `*Fixed*` family, which default to `ColorLightTokens.PrimaryFixed` -> `PaletteTokens.Primary90` -> `#EADDFF`, so a monochrome app renders Material baseline lavender the moment anything reads one. The thirteenth is `surfaceTint`, whose default is `primary` -- correct, and not a defect. The script labels the first group "lavender" and the second "not a defect". The `.dp` histogram splits three ways for the same reason. 527 literals: 419 on the M3 spacing scale, 19 dimensions rather than spacing (a 1dp hairline, an avatar, an image height), and 89 genuinely off-scale. The naive split reported 101 off-scale by counting 1dp borders as bad spacing, which would have sent phase 2 chasing hairlines. `DIMENSION_EXEMPT` is deliberately short and the header asks for a justification in the commit that lengthens it. **`ColorSchemeContrastTest` walks the real schemes, which cost a visibility keyword.** Four assertions over all six declared schemes: every content role on its container at 4.5:1, `onSurface` on each of the seven tonal surfaces at 4.5:1, `outline` against every surface it is drawn on at 3:1, and `primary`/`error` against `surface` at 3:1. WCAG relative luminance from first principles -- the 0.03928 knee and the 2.4 exponent, not a gamma-2.2 approximation, because the approximation moves borderline pairs by enough to change a verdict and the tightest pair in this tree is 4.56:1. `Theme.kt`'s six schemes went from `private val` to `internal val` so the test can see them. The alternative -- rebuilding the schemes inside the test from `Color.kt`'s public values -- keeps production visibility untouched and was rejected: it would assert the palette and miss the wiring, and the wiring is the half that fails silently. `surfaceContainerHigh = surfaceContainerHighestLight` is a one-character slip, compiles, and reads fine in review. A comment above the first scheme says this, so the keyword is not quietly widened back. **Verified that it bites.** Nudging `onSurfaceVariantLight` from `#4C4546` to `#9C9496` -- a plausible "soften the secondary text" edit that nothing else in the build would object to -- fails with `light: onSurfaceVariant on surfaceVariant is 2.29:1`, naming scheme, pair and ratio. Reverted; the committed value is unchanged. **Monotonicity across the contrast ladder is deliberately not asserted.** The obvious invariant -- high-contrast beats medium beats default for every pair -- looks right and is false. Ten pairs move the other way, and correctly: in the light high-contrast scheme `surfaceContainerHighest` goes darker to separate it from `surface`, which drops its ratio against `onSurface` from 13.30 to 12.29 while raising the separation that the change exists for. `onErrorContainer on errorContainer` drops 7.24 -> 5.19 from default to medium for the same kind of reason. Asserting the ladder would have meant either a red test or nine exemptions; the floor is the real invariant and every one of those values is comfortably above it. The test's doc comment records this so the next reader does not add the assertion. **Also not asserted: `outlineVariant`, and the call sites.** `outlineVariant` reads 1.61:1 against surface, which looks alarming and is not a defect -- M3's own baseline sits in the same range and the role is a decorative divider, so `outline` is what gets the 3:1 assertion. The seven call-site pairings that are genuinely below threshold, including the 1.00:1 one in ProposalListScreen, belong to phase 3; adding them now would mean checking in a red test. **Doc reconciled to the script rather than the other way round.** Three hand counts were wrong and are corrected in docs/material-design-conformance.md: 520 `.dp` literals -> 527 (the earlier figure omitted the exempt dimensions), 90 `label*` typography uses -> 92 (it missed `labelSmallEmphasized` and `labelLargeEmphasized`, which are label roles too), and 101 off-scale -> 89. The phase 0 section is rewritten from a plan into what was built, including what was decided against. **Tests.** 914 pass, 580 jvm over 70 classes and 334 android over 42 classes, up from 906/576/69 and 330/41 -- the four new assertions, in one new class, counted once per target because commonTest flows into both. `:composeApp:compileDebugKotlinAndroid` builds. No app behaviour changes: the only production edit in this commit is `private` -> `internal` on six vals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
7c41992ea7 |
fix: hold the wallet state where a background thread can be heard, so a device with no seed boots
Since
|
||
|
Kgothatso Ngako
|
f301a924fd |
fix: build a release apk, by dropping the app's copies of what the library already ships
`:composeApp:assembleRelease` dies in `mergeDexRelease`:
Type com.machankura.compose.ui.composable.widgets.nfc.ComposableSingletons$HceMonitorKt
is defined multiple times:
composeApp/build/intermediates/project_dex_archive/release/dexBuilderRelease/out/...
lightning-kmp-app/library/build/.transforms/.../bundleLibRuntimeToDirAndroidMain_dex/...
Both paths are ours. composeApp compiles a class, lightning-kmp-app's `:library`
compiles the same fully-qualified name, and D8 will not merge the two into one
apk. Debug never objected, because it packages the per-project dex archives as
they stand and only the release merge walks the whole set looking for
collisions -- so this has been true for a while and surfaced the first time
anyone asked for a release. There were two such copies, and fixing the first
only uncovered the second.
**The NFC widgets were renamed by directory, not by package.**
|
||
|
Kgothatso Ngako
|
224d6009dc | Merge branch 'mantra' into claude/marmot-profile-metadata-loading-e2a067 | ||
|
Kgothatso Ngako
|
e6524e201d | Merge branch 'mantra' into claude/proposals-signature-card-090e46 | ||
|
Kgothatso Ngako
|
f146afd49e |
fix: keep asking who a Marmot group's members are, instead of once and never again
A member of a Marmot group shows as "LOADING..." and stays that way. The same
member in a NIP-17 room starts as "LOADING..." and then turns into their name.
The difference is not that Marmot forgets to ask. It asks exactly once, and
NIP-17 is the one that gets asked again.
**"LOADING..." is a row, not a spinner.** Every pubkey this device sees gets a
Profile row immediately, because Participant.participantPublicKey and
ChatRoom.userPublicKey are both foreign keys onto Profile and nothing can be
filed until one exists. What gets written is a placeholder:
Profile(
displayName = "LOADING...",
publicKey = ...,
createdAt = GENESIS_AT,
nostrEventId = nostrEvent.id, // Will get overwriting by sync,
)
GENESIS_AT (1231006505000L, the Bitcoin genesis block) is the marker: a row
carrying it has never been read off a kind:0. `NostrDao.indexNostrEvent` writes
one, `MarmotInboundManager.processGroupMembershipChanges` writes one, the Welcome
branch of `NostrDao.indexNostrEvent` writes one,
`NostrDao.getOrCreateNip17ChatRoom` writes one. Each of them then queues the
kind:0 request that is supposed to replace it. Each queues it once.
Once is a whole lot of load-bearing. `Relays.DefaultDMRelayList` is
`listOf(ephemeral)` -- one relay -- so "ask the relays" is one negentropy
reconciliation against one host, at whatever moment the pubkey first appeared. If
that host has not got the member's kind:0 yet, that is the end of the enquiry.
**NIP-17 gets a second chance twice over.** Opening a NIP-17 room runs
`ChatMessageListViewModel.scheduleSynchronization`, which fetches each
participant's kind:10050. That request is queued at level 0, so the kind:10050 it
brings back is *indexed* at level 0 -- and the top of `indexNostrEvent` says:
} else if (profile.createdAt == GENESIS_AT && level == 0) {
logger.i("This is a placeholder profile... that might need to get synced...: $profile")
...
profilePublicKeysToSync[relayURL]?.add(nostrEvent.pubKey)
}
which queues the full `profileEventKinds` set, kind:0 included. So the name
arrives on the bounce: we asked for a relay list, we got an event that member
signed, indexing it noticed the placeholder was still there, and it asked again
for the profile. Any other event of theirs we happen to index does the same thing.
**A Marmot group has neither half.** The first half is gated off explicitly:
if (localChatRoom.chatRoom.mlsGroupState == null) {
which is the whole body of `scheduleSynchronization`. Opening a Marmot room asks
for nothing, by construction -- and reasonably so on its own terms, since an MLS
room does not need a member's kind:10050 to address a message to them.
The second half cannot fire, because a Marmot member never authors anything this
device indexes under their own key. A kind:445 is signed by a throwaway keypair
minted for that one event (`MarmotOutboundDao`, two sites: `NostrSignerInternal(KeyPair())`),
and the real sender is inside the MLS frame, recovered in `indexMarmotGroupEvent`
as `mlsGroup.memberIdentityHex(it.senderLeafIndex)` -- long after the pubkey check
at the top of `indexNostrEvent` has already run against `nostrEvent.pubKey`. That
check does fire on every kind:445; it just fires on the throwaway key, mints a
placeholder for a key that will never exist again, and queues a profile sync for
it. The member it is standing next to is not looked at.
So: one ask at the Welcome (or at the commit that added them), and then nothing,
ever, for the life of the room. Lose that one ask and the room is full of
"LOADING...".
**Two smaller holes, same shape.** Both Marmot mint sites test `profile == null`:
val profile = database.profileDao().getProfileByPublicKey(newParticipant.participantPublicKey)
if (profile == null) {
// create placeholder AND queue the sync
}
A placeholder is not null. A member we already hold one for -- seen in another
room, or removed from this one and added back -- takes the `false` branch and is
never queued at all. Not even the single ask.
**The change.**
- New `nostr/MemberProfileSync.kt`. Picks out, from a set of rooms, the members
nobody has read a kind:0 for -- missing row and placeholder row treated the
same, ourselves excluded because our own profile is not something a relay
teaches us -- and builds the kind:0 requests for them. Authors are chunked 100
per filter: a relay may refuse a filter it thinks is too big, and one refusal
should not take every member down with it. Requests go out at level 0, which
is deliberate: it is what marks a request as one somebody is waiting on, and
it is what lets the arriving kind:0 pull the rest of the member (DM relay
list, key packages) in behind it via the placeholder branch quoted above.
- `LiveSubscriptionManager.queueCatchUpSynchronization` now also asks about every
member it cannot name, across every room on the account. This is the main
repair. It is the right home for it: the foreground catch-up already holds the
room list (it was fetching it for `groupIdsFrom` and throwing the rooms away
-- `liveGroupIds` is gone, the rooms are kept), it already exists to answer
"what did I miss", and running there covers the chat list, the member lists
and the message feed at once rather than one screen at a time. It re-runs on
every foreground, so an ask that comes back empty is retried rather than lost.
- `ChatMessageListViewModel.scheduleSynchronization` asks too, for both kinds of
room, before the NIP-17-only relay-list block it already had. This closes the
gap between foregrounds: join a group while the app is open, and the names
resolve without backgrounding it first.
- `MarmotInboundManager.processGroupMembershipChanges` and the Welcome branch of
`NostrDao.indexNostrEvent` now treat a placeholder as unresolved. The
placeholder insert still only happens when there is no row (it is an @Insert
and would throw on conflict); it is the *ask* that now happens either way.
**Left alone, deliberately.** The `mlsGroupState == null` gate below the new code
stays: kind:10050 genuinely is NIP-17-only, and an MLS room's messages go to the
group's own relays. The placeholder minted for a kind:445's throwaway signer is
untouched -- it is waste, not a bug, and removing it means deciding what
`indexNostrEvent` should do with an event whose author is by design nobody, which
is a bigger question than this. `DefaultDMRelayList` being a single host is left
as it is; widening profile lookups to the directory relays (purplepag.es,
user.kindpag.es, directory.yabu.me are all already in `Relays`) would find more
kind:0s than asking one relay repeatedly, and is worth doing on its own.
**Verified.** `:composeApp:compileDebugKotlinAndroid` builds. `:composeApp:jvmTest`
is green: 576 tests over 69 classes, including 7 new ones in
`MemberProfileSyncTest` covering placeholder-vs-null, self-exclusion, a member in
several rooms counted once, the filter shape (kind:0, level 0, one request per
relay) and the 100-author chunking.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
73cba2ae0e |
feat: say at the foot of the transcript what the group is still waiting for you to sign
The transcript carries a proposal past as it happens, and |