fix(mobile): shell-own login keyboard pad, un-nest status fades - #4850
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThis round only adds/updates Files Reviewed (3 files)
Note: Previous Review Summaries (3 snapshots, latest commit 5597326)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5597326)Status: No Issues Found | Recommendation: Merge Executive SummaryThis round only adds two new Files Reviewed (2 files)
Note: Previous review (commit af1e544)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the follow-up change that fully removes the Reanimated fade animations (previously just un-nested) from the login status branches and the earlier login-shell keyboard-pad ownership change; no high-confidence security, correctness, or logic issues found in the changed code. Files Reviewed (3 files)
Previous review (commit a506987)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the login-shell keyboard-compensation ownership change and the animation un-nesting in Files Reviewed (2 files)
Note: Reviewed by claude-sonnet-5 · Input: 24 · Output: 4.9K · Cached: 473.2K Review guidance: REVIEW.md from base branch |
Summary
What — two login-screen keyboard defects and one intermittent-render mitigation in
apps/mobile, confined tologin-screen.tsxandlogin/email-otp-form.tsx:KeyboardAvoidingView behavior="padding"and the OTP form's localkeyboardHeight + 16spacer stacked, pushing the form 194–376pt above the keyboard and clipping the logo under the Dynamic Island. Fix: the OTP form's spacer machinery is deleted outright; the iOS KAV stays as the single iOS compensation.EDGE_TO_EDGE_ENFORCED), so the root KAV was inert; on a 720×1280 phone the email input and Send code were fully covered by the IME. Fix: the login shell consumeskeyboardDidShow/keyboardDidHideitself (via the shared, already-tested reducer) and appliespaddingBottom = endCoordinates.height + safe-area bottom inseton a wrapper View outside the ScrollView. This lands in the same commit as the spacer deletion so Android never has a zero-compensation window.layout={LinearTransition}from the per-branch fades) did not hold: E2E round e1 measured 2/2 deterministic remount parking at washed-out ~40–50% alpha (pixel-measured, stable 3+ minutes, interactive underneath). The per-branchentering/exitingfades were the remaining interrupted-animation surface, so this PR now removes them entirely — the status branches render as plain Views and status swaps are instant. With no animation on these branches there is no animation state to park. Cost: the branch fade (200/150ms, decorative) is gone; the plan's "transitions stay visibly animated" goal is deliberately dropped — a login screen that can park invisible is worse than an instant swap.Why — users on small Android phones could not see the field they were typing into nor the Send code button; iOS OTP users lost the logo and had the form pushed up to half a screen too high.
How — one working keyboard compensation per platform, owned by the login shell; not two stacked, and no third mechanism. The shell shares kilo-chat's pure reducer (
resolveAppAwareKeyboardPadding+resolveKeyboardPaddingEventsForPlatform); kilo-chat's reducer andAppAwareKeyboardPaddingVieware byte-untouched. The pad formula isendCoordinates.height + useSafeAreaInsets().bottom=WindowInsets.ime().bottomexactly: RN 0.86 reportsendCoordinates.heightminus the nav bar, andscreenYis not the IME top underadjustResize(vendoredReactRootView.java:962,974-977; math-locked on pixel9: 704px + 63px = 767px = the dumpsys IME frame). The pad applies only while the keyboard is up, so the resting layout is unchanged.Also commits four E2E learnings from the repro runs to
.kilo_workflow/learnings/.Verification
Reproduced on the unmodified baseline (quantified; screenshots/dumps retained in run artifacts):
Post-fix device verification (three verifier rounds on the final code; evidence in run artifacts):
iOS keyboard matrix (e1, PASS):
Android matrix (e2, all PASS — bounds are uiautomator vs dumpsys IME frame):
Transparency recurrence loops (e2 + e3, repaired build, PASS):
Documented deviations (accepted, not failures):
Checks in
apps/mobile:pnpm format,pnpm typecheck,pnpm lint(0/0),pnpm check:unused,pnpm test(277 files, 2346 tests) — all green.Visual Changes
Reviewer Notes
app-aware-keyboard-padding-state.tsand kilo-chat'sAppAwareKeyboardPaddingVieware intentionally untouched: the view pads by rawendCoordinates.heighton both platforms, which under-clears by the nav inset on Android and would re-create the double-count on iOS. Sharing only the pure reducer is the deliberate amount of reuse.apps/mobile/e2e/changes, no new test files (the change deletes UI wiring and consumes an already-tested pure reducer).