Repository navigation
fix(mobile): keep a choice row's name while it saves - #8513
Conversation
ChoiceRow now composes an explicit accessibilityLabel from its label and caption, so the shared row control always has a name of its own. React Native rebuilds a control's content description from that label plus any state without a node setter, so the name now survives the row's busy state instead of being replaced by it, and the state clears back to the name rather than leaving a stale description. Rows rendered from custom children keep their own text-derived names. The new mounted test pins the composition, the busy round trip, the children case, and the radio/checkbox roles.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review is a one-line reformat of the already-reviewed Assumptions: Files Reviewed (2 files)
Previous Review Summary (commit 224a97c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 224a97c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe change adds an explicit Assumptions: Files Reviewed (2 files)
Reviewed by deepseek-flash-latest · Input: 40.9K · Output: 2.7K · Cached: 287K Review guidance: REVIEW.md from base branch |
|
bot: The failing checks all stop in The merge ref of this branch with I will re-run the failed checks once the base lockfile is repaired on |
Evidence
Evidence: demonstrated — ran the app on the
omp36emulator against the local dev stack with an owned fake account, opened Profile > Code Reviewer > GitHub > Review Style, held the config save at the network boundary, and read the row'suiautomator dumpnode before and after the change: it readbusyalone on the previous component andLenient and Only serious problems, busyon this branch, returning toLenient and Only serious problemsonce the save settled.What changed
ChoiceRow, the shared radio/checkbox row, now sets an explicitaccessibilityLabelcomposed from itslabelanddescriptionwithformatListin the active app language. Rows rendered from customchildrenkeep their own text-derived name.React Native's Android view manager rebuilds a control's
contentDescriptionfrom the accessibility label plus the states that have no node setter (busy,expanded, stringchecked), and it does not clear that description again when the state goes away. A row that reports a busy state without a label of its own therefore ends up describing only the state. With a label, the state is appended to the name, and clearing it returns to the name instead of leaving a stale description.New mounted test
apps/mobile/src/components/ui/choice-row.mounted.test.tsxpins the name composition, the busy round trip, the custom-children case, and the radio/checkbox roles.End-to-end evidence
Ran the app on the
omp36emulator against the local dev stack with an owned fake account, opened Profile > Code Reviewer > GitHub > Review Style, and held the config save at the network boundary so the in-flight state could be read fromuiautomator dump.Before (main, save in flight):
After (this branch, same row, same bounds, save in flight):
Idle state after the save settles (this branch): the same row reads
Lenient and Only serious problems, with no leftover state text.Other checks on this VM:
pnpm --filter kilo-app exec tsc --noEmitclean,pnpm --filter kilo-app exec oxlintclean on both files, the full mobile suite (pnpm --filter kilo-app test) passed 1242 files / 18437 tests, and the new mounted test fails against the previous component and passes with the change.A label change has no pixel delta, so screenshots cannot demonstrate it; the accessibility-tree nodes above are the evidence.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: formatList (locale-aware conjunction)
Confidence: 0.65 · Precedent: Engineering conventions: prefer existing libraries to hand-rolled code (session df491946927a, "we will try and use libraries wherever we can"); UX: mobile app always respects i18n (session dcb3f9170164)
Answer: Leave them on the derived name
Confidence: 0.65 · Precedent: Speed and least work: do the least work possible with no drop in quality (session 7f3470e0ba6f, "do the least amount of work possible, with no drop in quality"); no children row sets busy, so the defect does not occur there
Answer: Add choice-row.mounted.test.tsx
Confidence: 0.60 · Precedent: Keep CI checks that catch real errors in PRs: session 99e88001c4b4, "we want to catch errors in PRs, as early as possible"; the Android accessibility dump during a style save is still required as end-to-end proof
Answer: Merge main into the branch
Confidence: 0.90 · Precedent: Git and pull requests: merge main only when a conflict or a fix on main needs it — session a247f3c19b1e, "try merging main in, that may fix the issue"