Skip to content

fix(mobile): loading and skeleton polish - #4772

Merged
iscekic merged 10 commits into
mainfrom
fix/mobile-loading-polish
Jul 27, 2026
Merged

fix(mobile): loading and skeleton polish#4772
iscekic merged 10 commits into
mainfrom
fix/mobile-loading-polish

Conversation

@iscekic

@iscekic iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Mobile loading polish, three items:

  1. Shared Skeleton shimmer — a soft horizontal gradient sweep (expo-linear-gradient) is layered over the existing opacity pulse. The sweep is driven by one refcounted module-level Reanimated clock shared by every instance, so the sweep costs a single UI-thread animation no matter how many skeletons are on screen (each instance also keeps its per-instance pulse, now at a faster 1700 ms cadence). Reduce Motion renders the same static muted block as before. API unchanged: Skeleton({ className }).
  2. Session transcript pagination: no visible jump when an older page arrives — the list header's transient loading skeleton (~80pt) collapsed in the same update that prepended the older page. FlashList's maintainVisibleContentPosition compensates prepended items but not the header height change, which produced a measured ~43pt upward jump of on-screen content at every page boundary (60fps frame analysis on the baseline). The placeholder is now suppressed at the render-model layer (an already-visible omitted-items banner stays stable through the load), and the fetch starts two viewports before the boundary so the prepend usually lands before the user reaches it. The identical pattern in the kilo-chat message list is fixed the same way, with the dead isFetchingOlder prop removed.
  3. Notifications settings loader matches the final layout — category placeholders are now structurally identical to the settled rows (same container classes, content-sized skeleton lines, native-switch-sized 31×51 block), and the master Switch / "Enable notifications" CTA are gated on a new pure deriveGateSettled predicate (permission settled AND, when granted, both push-token queries fetched-or-errored). A fully enabled user no longer sees a transient Enable card or a Switch flashing a wrong value while the token queries settle.

Test plan

Automated: full mobile suite green (226 files / 1881 tests), including the new deriveGateSettled truth table and updated pagination-header render-model tests. oxfmt, tsgo, oxlint, knip, git diff --check clean; expo-doctor introduces no new findings (the out-of-date-packages advisory pre-exists on main).

On-device (iPhone 17 Pro simulator, dev client built from this branch):

Criterion Result
Shimmer sweep moves across skeletons (frame analysis) PASS — highlight at ≥3 distinct x-positions; no frame freeze
Reduce Motion → static skeletons; setting restored PASS
Pagination: controlled scroll through both boundaries, anchor stability PASS — no loading placeholder ever visible; anchor y stable (max Δ 0.0pt) in all scored pauses at both boundaries
Pagination: transcript reaches message 1 of 130 PASS
Pagination: fast fling with page in flight Not reproducible at local latency (3 attempts; page always landed before the fling completed; no spinner or jump in any captured frame)
Notifications: skeleton vs settled row geometry PASS — row boundary deltas 0.00–0.67pt (±2pt budget); section headers Δ0
Notifications: registered user sees no Enable-CTA flash PASS — CTA absent in every cold-load frame
Notifications: CTA present for unregistered user; enable flow registers PASS — CTA gone and master ON after registration
Notifications: category toggle optimistic + pending behavior PASS

E2E notes: the loading window on the local stack is ~1 frame, so shimmer/skeleton captures used the deterministic suspend protocol (backend process briefly STOPped while the app was already bootstrapped, then resumed; screen settled to real data afterwards). A1.3's scroll-drag instrument is structurally inapplicable — no loading screen in the app is scrollable (every loading branch renders a static placeholder view that fits the viewport; verified across all Skeleton call sites); smoothness is covered by the no-freeze frame analysis, and interactivity by gestures/navigation processing immediately during the shimmer window plus the single-shared-sweep-clock design.

iscekic added 4 commits July 26, 2026 01:39
Replace the per-instance opacity pulse with a soft horizontal gradient
highlight driven by one refcounted module-level Reanimated clock shared
across all Skeleton instances. Reduced motion keeps the static muted
block; the component API is unchanged.
… load

The transient loading skeleton in the list header collapsed in the same
update that prepended an older page; FlashList's content-position
maintenance compensates prepended items but not the header height
change, producing a visible jump at the page boundary.

Suppress the placeholder at the render-model layer (loading maps to the
omitted banner when one is visible, else hidden) and start the older-page
fetch two viewports before the boundary so the prepend lands before the
user reaches it. Mirror the fix in the kilo-chat message list and remove
the now-unused isFetchingOlder prop end to end.
Match category and master-row placeholders to the settled row geometry,
and gate the master Switch and Enable-notifications CTA on a new pure
deriveGateSettled predicate so a fully enabled user never sees a
transient Enable card or a Switch with a transient wrong value while
the push-token queries settle.
iscekic added 3 commits July 26, 2026 12:34
Layer an opacity breathe (0.4->1.0, 850ms half-cycle, 1700ms full
cycle; pre-branch pulse was 1000/2000ms) on the shimmer sweep, and
keep the reduced-motion path fully static.
@Kilo-Org Kilo-Org deleted a comment from kilo-code-bot Bot Jul 26, 2026
@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Standin review of head 8d303b6cnot a Kilobot review. The automated reviewer posts nothing for this PR: the Kilo Code Review check runs green without posting findings, and @kilocode-bot reports it cannot post review comments. This review was therefore produced by a dedicated standin review agent over the full PR diff (git diff origin/main...HEAD, 14 files, +303/−77), reviewing against the PR description, acceptance criteria, and applicable AGENTS.md rules. It is posted so a review is on the record.

Standin reviewer checks (all green on 8d303b6c): pnpm typecheck, pnpm lint, pnpm check:unused, pnpm test (226 files / 1881 tests), pnpm format:check. pnpx expo-doctor was outside its permissions and is being handled under finding 3.

Findings (all Low; no critical/high/medium)

1. Low — Notifications master row's leading content is not gated, so the "wrong value" flash survives in the row body
apps/mobile/src/components/notifications-screen.tsx:431-444
deriveGateSettled gates the Switch and the Enable CTA, but the master row's dimming (opacity-50), the Bell/BellOff icon, and the subtitle "Permission or device registration is off." are all driven by notificationsEnabled, which is false while the token queries settle. On a cold mount, a fully enabled user still sees a dimmed BellOff row declaring notifications off — directly contradicting the settled state — while the trailing slot shows a skeleton. Same transient-wrong-state class this PR item set out to eliminate; fixed only for the trailing slot and CTA.
Required outcome: while the gate is unsettled (permission granted, tokens in flight), the master row's icon, subtitle, and opacity must not assert the "off" state; reflect the settled value or a neutral loading presentation.

2. Low — Pagination load is now completely silent to screen readers
apps/mobile/src/components/agents/session-pagination-header-render-model.ts:47-53, apps/mobile/src/components/kilo-chat/message-list.tsx:204-205
The removed header skeleton was the only element with accessibilityRole="progressbar" announcing older-page loading in the agents transcript; the kilo-chat header skeleton served the same purpose. Both are gone with no replacement: a VoiceOver/TalkBack user now gets no indication that a fetch started or that earlier messages arrived (the omitted/retryable/terminal banners keep accessibilityLiveRegion="polite", but the success path announces nothing).
Required outcome: provide some non-visual signal for older-page load progress or arrival (e.g., a polite live-region announcement on page completion), or record an explicit accepted rationale for the regression.

3. Low — expo-linear-gradient specifier deviates from the SDK 55 bundled pin
apps/mobile/package.json:62
The PR adds expo-linear-gradient: ~55.0.16; Expo SDK 55's bundledNativeModules.json (expo@55.0.27) pins ~55.0.15. The resolved 55.0.16 satisfies ~55.0.15, so this is functionally compatible, but the specifier suggests the package was not installed via the AGENTS.md-mandated npx expo install, and pnpx expo-doctor (required after dependency changes) was outside the reviewer's permissions.
Required outcome: align the specifier with the SDK bundled pin (or demonstrate expo-doctor passes clean with the current one).

4. Low — Unrelated lockfile churn in pnpm-lock.yaml
Two snapshot lines re-point vitest's @types/node context from 24.12.4 to 25.5.2. Both versions pre-exist and remain in the lockfile, so this is harmless dedupe drift, but it is unrelated to the PR's only dependency change and adds noise to the diff.
Required outcome: none blocking; ideally revert the unrelated hunks so the lockfile diff contains only the expo-linear-gradient addition.

5. Low — PR description overstates the merged skeleton behavior
apps/mobile/src/components/ui/skeleton.tsx:78-109
The description says the opacity pulse "becomes" a gradient sweep and that a screen of skeletons "costs a single UI-thread animation". The merged code retains a per-instance withRepeat opacity pulse in addition to the shared module-level shimmer clock — N+1 UI-thread animations, both effects active. Not a code defect (no worse than pre-PR baseline plus one shared animation), but the description of record is inaccurate.
Required outcome: correct the PR description to describe pulse-plus-sweep and the actual animation accounting.


Scope completed by the standin reviewer: full diff and commit boundaries; surrounding reads of every touched file; auth-gating reachability of the notifications screen; Reanimated 4.2.1 makeMutable export; TanStack Query isFetched semantics (settle idiom sound); olderMessagesOmittedItemCount monotonicity (no-jump invariant holds); refcount clock mount/unmount/StrictMode/reduce-motion races (balanced); no lingering references to the removed session-pagination-header-loading testID or isFetchingOlder prop.

Residual testing risks it noted: shimmer-clock behavior under fast-refresh and FlashList cell recycling is reasoned, not unit-tested (no pre-existing Skeleton coverage); Android rendering of the iOS-sized 31×51 switch skeleton and gradient corner clipping not device-verified.

Triage and repairs are in progress; outcomes will be recorded here per finding.

@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Standin-review loop complete on the new head 6ae99faac. Triage and outcomes for the five findings posted above:

  1. Master row leading content not gated — valid, repaired (4ec7b43ee). The row's icon, dimming, and subtitle are now driven by a new pure deriveMasterGateLeadingPresentation (neutral/on/off): while the gate is unsettled the row shows a neutral 18×18 icon skeleton and a content-sized subtitle skeleton instead of asserting "off"; settled and permission-error presentations are unchanged. Truth-table unit tests added.
  2. Pagination silent to screen readers — valid, repaired (d0aec421a). Both the agents session list and the kilo-chat list now call AccessibilityInfo.announceForAccessibility("Earlier messages loaded") when an older page actually prepends items, gated by the pure shouldAnnounceOlderMessagesArrival (no announcement on initial paint, appends, or empty prepends). No visual placeholder re-added. Truth-table unit tests added.
  3. expo-linear-gradient specifier — valid, repaired (6ae99faac). Specifier is now ~55.0.15, matching expo@55.0.27's bundledNativeModules.json; resolved version stays 55.0.16. pnpx expo-doctor reports 18/19 with only the out-of-date-packages advisory that pre-exists on main; expo-linear-gradient is not listed.
  4. Lockfile churn — valid, repaired (6ae99faac). The unrelated vitest @types/node snapshot hunks are reverted; the net lockfile diff contains only the expo-linear-gradient addition.
  5. PR description overstated skeleton behavior — valid, repaired. The description now states the sweep is layered over the retained per-instance pulse and that the single-animation claim applies to the shared sweep clock.

A fresh standin reviewer then re-reviewed the full diff at 6ae99faac (all four repairs verified against their findings; pnpm typecheck, pnpm test — 228 files / 1893 tests, pnpm lint, pnpm check:unused, pnpm format:check all green on its pass too). It returned no valid actionable findings. Its one new Low remark — the neutral subtitle skeleton is one line while the settled subtitle may wrap to two on narrow devices, so the neutral→settled transition can shift the row by ≤16pt once — is accepted as a residual, no code change, per the resolution path the reviewer itself offered: the pre-repair code had the same subtitle string flip at settle (with the same wrap shift plus the wrong-state flash), so this is strictly better than baseline; the shift lands in the same frame where the whole category section transitions from skeletons to content; and every deterministic fix (fixed two-line slot for all users, or truncated copy) is a permanent settled-layout regression to fix a transient sub-16pt shift on narrow devices.

Because findings 1–2 affect behavior, a targeted on-device re-verification of the notifications loading gate (and a pagination smoke pass) is queued behind the shared device-slot semaphore; results will be posted here. CI is running on 6ae99faac.

@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) On-device re-verification of the repair head 6ae99faac is complete (iPhone 17 Pro Max simulator, iOS 26.5, validated cached dev client — the expo-linear-gradient change was specifier-only, no native rebuild required; deterministic suspend protocol used for loading frames):

  • N1 — registered user, no wrong-state flash: PASS. Every pre-settle frame shows the neutral master row (18×18 icon skeleton, subtitle skeleton, 31×51 trailing skeleton, no dimming, no BellOff, no "Permission or device registration is off.", no Enable CTA). After settle: "Push notifications are on for this device.", master Switch ON, no CTA.
  • N2 — settled-off presentation unchanged: PASS. Unregistered user settles to BellOff + off subtitle + dimming + Switch OFF + Enable CTA, exactly as before the repair.
  • N3 — master-row geometry: PASS. Neutral = settled-on = settled-off = 53pt (Δ 0pt); icon and trailing slots stable. The accepted residual (subtitle wrap on narrower devices) does not manifest at this width.
  • N4 — category section smoke: PASS. Rows render; a category toggle flips optimistically and settles.
  • P1/P2 — pagination smoke: skipped with rationale. The repair's only pagination change is the non-visual AccessibilityInfo announcement (unit-covered predicate; not observable by Maestro). The visual no-jump behavior is byte-identical to the code the earlier on-device pass verified through both boundaries, and multi-page history (page size 50) could not be re-produced on the local stack within budget without a product-changing fixture harness. Recorded here so the skip is explicit.

Zero product or test-environment failures. All CI checks are green on 6ae99faac, and the fresh standin review of this head returned no valid actionable findings. The PR is ready for human review.

@iscekic
iscekic enabled auto-merge (squash) July 26, 2026 15:54
@iscekic
iscekic disabled auto-merge July 26, 2026 15:58
@Kilo-Org Kilo-Org deleted a comment from kilo-code-bot Bot Jul 26, 2026
@iscekic
iscekic merged commit 3fa1312 into main Jul 27, 2026
68 checks passed
@iscekic
iscekic deleted the fix/mobile-loading-polish branch July 27, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants