Repository navigation
fix(mobile): show the Kilo Pass Manage button label - #8077
Conversation
The Manage button on the Kilo Pass screen passed its label as a bare string child. Button renders into a Pressable (a View), and a View draws no text, so the control collapsed to its padding and showed no label while the accessibility tree still named it. The shared Button now wraps string and number children in its themed label Text, and the Manage CTA carries an explicit Text label plus a 48dp minimum height for the Android touch target.
iscekic
added
drain
|
bot: End-to-end proof on the Android emulator (API 36, 420 dpi), local dev stack + Metro, dark mode, Hindi locale, fake local account with an active web-managed Kilo Pass: the Manage call-to-action renders its visible label, and the accessibility tree reports the clickable node at 218x126 px = 83.0 x 48.0 dp, meeting the 48 dp touch target.
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe new array-child normalization in the shared Files Reviewed (2 files)
Previous Review Summary (commit df625c6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit df625c6)Status: No Issues Found | Recommendation: Merge Executive SummaryThe shared Files Reviewed (4 files)
Notes: Reviewed by deepseek-v4.1-flash-1017 · Input: 93.1K · Output: 10.7K · Cached: 619.6K Review guidance: REVIEW.md from base branch |
Follow-up to the scalar-label normalization: children arrays get the same per-entry treatment, and whitespace-only string entries are left unwrapped so JSX spacing idioms between an icon and a labeled element keep their current behaviour (they draw nothing in a View, and the row gap does the spacing). Adds a regression test for both.
|
bot: Follow-up commit a772155: the Button label normalization now also covers array children per entry, while whitespace-only string entries (the JSX |
iscekic
added
merge:bot
iscekic
added
merge:bot
Change
The Manage button under "This Kilo Pass is managed on web" on the Kilo Pass screen rendered as a blank rectangle on device: its label was passed to the shared
Buttonas a bare string child.Buttonrenders its children inside aPressable(a View), and a View draws no text, so the control collapsed to its padding (~22x25 dp) while the accessibility tree still named the button.Two changes:
apps/mobile/src/components/ui/button.tsx: the sharedButtonnow wraps string and number children in its themed labelText(the classes theTextClassContextprovider already publishes), so every caller's scalar label renders. Element children pass through untouched. This closes the bug class for all current and future call sites; the other sevenButtoncall sites that pass non-Textchildren pass icon elements, which already rendered.apps/mobile/src/components/kilo-pass/kilo-pass-subscription-screen.tsx: the Manage CTA carries an explicit<Text>label andmin-h-[48px], so the lone centered call-to-action meets the 48 dp Android touch target without hitSlop (nothing sits beside it). Default size staysmin-h-[44px]elsewhere.Verification
button.mounted.test.tsx(25) andkilo-pass-subscription-screen.test.ts(5) pass; both carry new regression assertions — the primitive wraps scalar children in the themed labelText, and the screen renders the Manage button with a labeled child and the 48 dp class.pnpm typecheck(mobile) clean; oxlint clean on the four changed files.प्रबंधित करें, and the accessibility tree reports the clickable node at 218x126 px = 83.0 x 48.0 dp (the 126 px height is exactly 48 dp at 420 dpi).Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: min-h-[48px] on this button (Recommended)
Confidence: 0.75 · Precedent: Engineering conventions / UX: best-in-class standard with clearance and right-sized targets (session 00eb2a1d3e4b, "design and UX MUST BE BEST-IN-CLASS"). The work item explicitly asks for 48 dp, and the button has no adjacency cost.
Answer: Harden the shared Button so it wraps string and number children in its themed label Text instead of rejecting or ignoring them. That fixes the class in all 8 files. Remove the now-redundant manual wrapping only where it is trivial. Add a regression test on the Button primitive, plus the kilo-pass screen assertion. Run the targeted tests for the touched files and let CI run the rest.
Confidence: 0.60 · Precedent: Autonomy: fix the root cause and the class of the issue on every affected surface (session 00eb2a1d3e4b, "if anything repeats the same mistake over over again, fix the class").
Answer: Fix them in this PR
Confidence: 0.55 · Precedent: Autonomy: fix the class on every affected surface (session 75f8833fe8c1, "fix this for all shared services model pickers on all surfaces"). Ask for an iOS lane to verify; an agent may request one (session 2f71af3a787e).
Note on the third answer: the two glanceable-ios files were surveyed and do NOT use the shared Button — they import
Buttonfrom@expo/ui/swift-ui, and their children are SwiftUITextelements returned by apill()helper, which render. No change was needed there; this is recorded rather than implemented because the premise of the answer (the same bare-string pattern) did not hold on inspection.