fix(mobile): session header pill spacing, centering, hide on load - #4863
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental update only adds a documentation/workflow-learnings note ( Files Reviewed (1 file)
Previous Review Summary (commit 5814bbc)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5814bbc)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed a small, well-scoped visual fix to the session header pill (spacing, ring centering, loading-hide behavior) and found no correctness, security, or breaking-change issues in the changed lines. Files Reviewed (3 files)
Reviewed by claude-sonnet-5 · Input: 32 · Output: 4.8K · Cached: 739.5K Review guidance: REVIEW.md from base branch |
Summary
Three visual fixes to the session page header pill (
SessionContextMetrics, the element holding the context ring + percentage + session cost):gap-1.5→gap-2,px-2.5→px-3).h-7 w-7wrapper (≈24.5px at NativeWind's rem≈14px in this app). The SVG drew from the wrapper's top-left origin and overflowed 3.5px right/bottom, so its visible center sat ~1.75px low. The wrapper Views are deleted andContextUsageRingnow renders directly as a flex child; the pill's existingitems-centercenters its explicit 28px layout box exactly.loading?: booleanprop renders the pill withopacity-0(layout box reserved, non-pressable, hidden from accessibility) while the session page loads. Both call sites pass it: the route-levelsessionQuery.isPendingbranch in[session-id].tsxandsession-detail-content.tsx(loading={shouldShowLoading}).Why: the pill was optically off-center and cramped, and during loading it showed a meaningless track-only ring that popped into real content on load.
How: one mechanism in the component —
loadinggatespressableand appliesopacity-0+ a11y-hiding props on the existing non-pressableViewreturn. No third return branch, no call-site divergence.Simpler-shape decision (recorded from plan review): a pure
getHeaderPillRenderStatehelper + unit tests was considered and rejected — the loading state is one boolean gate on an RN component the node test harness cannot render, so the helper was API surface without product value. Consequently there are no test changes: the new behavior is visual-only, covered by the E2E evidence below. The pinned test "keeps a fixed non-interactive pill when context usage is unresolved" is deliberately untouched — it pins unresolved-but-loaded behavior, which is preserved; loading is a component prop, not agetHeaderPillContentinput.CLS contract (precise, not overclaimed): the hidden loading pill reserves its content-sized box (route path: ring-only ~45px; in-content path may include outgoing cost text). Guaranteed: header height and the anchor positions (x/y origins) of the back button and title are identical loading → loaded. Explicitly not claimed: pill-width or title-width equality — the pill is content-sized and the title text changes at load; that is accepted, documented behavior, not a regression. A guessed fixed-width reservation was considered and rejected as redesign.
Verification
pnpm format && pnpm typecheck && pnpm lint && pnpm check:unused && pnpm testfromapps/mobile/— all green (283 files / 2434 tests, no suite changes)kilo-auto/efficientsession content — no mocking): loaded pill shows ring + percentage + cost; tap opens the context sheet (interactivity regression check passed)kill -STOPnextjs suspend protocol, strictly scoped) — header title + skeleton visible, pill region pixel-uniform (0 drawn pixels), pill absent from the a11y treehierarchy, loading vs loaded): identical in all three states — back button[10,80][34,104], title origin(41,79), header container[0,0][402,124]. Single-line-title precondition met (session renamed to "T"; loaded title 25pt tall = 1 line). Width equality not asserted per the CLS contract.Objective pill measurements (3x screenshot pixel analysis): ring annulus exactly 28.0×28.0pt; ring y-center −0.17pt off pill center (sub-pixel — optically centered); ring→text visual gap 8.0pt (
gap-2); ring 12.3pt from pill left edge, text 11.3pt from right edge (px-3).Visual Changes
After-only evidence per the brief (the before state is the one described in the user request; reproducing it would need a second stack on a baseline worktree — disproportionate for a spacing change).
Reviewer Notes
loadinghidden-wins semantics: ifshouldShowLoadingis true whilecontextInfoalready exists, the pill hides — consistent with the loading definition..kilo_workflow/learnings/mobile-remote-cli-model-id-prefix.md):remote-cli.shneeds the CLI idkilo/kilo-auto/efficient, not the in-app id; fresh E2E accounts need seeded credits.