chore(mobile): drive React Doctor to zero across mobile ui/habits (#243)

[thomasluizon]

Burns down all 100 React Doctor findings in the mobile components/{ui,habits,habit-list*} file-set (bucket B4-mob-ui) to zero — the mobile mirror of #501's web-routes pass. Every finding is either fixed behavior-preservingly or suppressed with a WHY linking #243 (genuine FPs / deliberate choices only). Gates green: mobile typecheck, expo lint, vitest (1072/1072), and the RD --scope changed diff gate (0 new).

Fixed properly (~44, behavior-preserving)

• no-ref-current-in-render (error) — dropped the redundant lastSyncedValueRef in buffered-sheet-input (the focus-guarded setDraft is already idempotent, so the render-time ref write was dead).
• rn-prefer-pressable (9 files) — TouchableOpacity → Pressable across the pickers, habit-calendar, checklists, chips and the offline state, replicating each activeOpacity as pressed-opacity feedback (default 0.2 where none was set).
• rn-prefer-pressable list rows — app-select now uses a memo'd row + useCallback renderItem (clears rn-no-inline-flatlist-renderitem / rn-list-callback-per-row / rn-no-inline-object-in-list-item).
• Mechanical — js-set-map-lookups (4×, Set lookups), rn-no-single-element-style-array, no-usememo-simple-expression, rerender-memo-with-default-value (module const), rn-no-legacy-shadow-styles → boxShadow, no-array-index-as-key where a stable id exists (habit-calendar weekday key), rn-list-data-mapped + skeleton renderItem hoist in habit-list.
• prefer-use-effect-event — fresh-start-animation onComplete moved to useEffectEvent so the one-shot entrance effect no longer re-subscribes.

Suppressed with WHY (Refs #243)

• rn-prefer-reanimated (16) — every one is deliberate RN Animated with useNativeDriver: true (transform/opacity already on the UI thread), so the rule's JS-thread-stutter premise doesn't apply. Reanimated 4.x migration is deferred because worklets 0.10.0 is ABI-pinned to the SDK 57 set and the swap needs on-device animation QA this burn-down can't do. Files: anchored-menu, app-date-picker, app-select, app-time-picker, app-toast, astra-avatar, confirm-dialog, fresh-start-animation, progress-bar, push-prompt, scroll-to-top-button, settings-row, skeleton, status-dot (SVG stroke, non-native-driver), trial-banner, trial-expired-modal.
• exhaustive-deps (18) — extracted habitsQuery / appConfig / profile aliases that are already listed in deps (the analyzer wants the qualified member path; the alias tracks it, so no staleness). Mostly habit-list.
• Structural / FP — no-many-boolean-props (5, private single-use aggregators), no-giant-component (2, already decomposed shells), no-event-handler (3, mount/exit-animation orchestration), no-prop-callback-in-effect / no-pass-*-to-parent (registration handles + documented parent-mirror callbacks), rn-no-scrollview-mapped-list (5, bounded wheels/carousels/pickers), rn-no-dimensions-get (3, one-shot/menu-dismiss reads), only-export-components (3, co-located helpers, Fast-Refresh-only), no-polymorphic-children (deliberate label-or-node API), rerender-state-only-in-handlers + rerender-defer-reads-hook (read in render / no non-hook path), no-array-index-as-key (value objects with no id, non-reordering lists), effect-needs-cleanup (cleanup via the returned clearTimer), advanced-event-handler-refs, rn-prefer-expo-image (static bundled asset).

Tests

Updated app-time-picker and offline-unavailable-state tests to query Pressable instead of TouchableOpacity (implementation-detail assertions after the migration).

Refs #243 (React Doctor burn-down: mobile ui/habits)

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
orbit-ui-mobile-web	Ignored		Jul 14, 2026 12:54am

[claude]

Code Review: PR #504

Scope: PR #504, thomasluizon/orbit-ui-mobile, chore/rd-mobile-ui-habits → main
Recommendation: APPROVE

Summary

Mechanical React Doctor burn-down over apps/mobile/components/{ui,habits,habit-list*} (43 components + 2 tests), mirroring the already-shipped web pass. Full diff (47 files) plus subagent reports reviewed; the riskiest hunks (the buffered-sheet-input.tsx ref removal, the frequencyHandlers de-memoization, the EMPTY_HABIT_ROW_ACTIONS module constant, and the TouchableOpacity→Pressable pressed-state replication) were traced by hand against the pre-diff logic; all are behavior-preserving. No Critical/High findings survive.

Findings

Critical

None.

High

None.

Medium

None. (Two design-reviewer observations — a mobile shadow-token gap in settings-row.tsx:253 and pressed-opacity magic-number sprawl carried forward from the original activeOpacity values — are pre-existing debt reformatted by this diff, not new drift introduced by it, and both were self-flagged non-blocking. Kept out per the signal gate.)

Low / Info

None posted (signal gate: Low/Info aren't PR-review findings).

Subagents

Agent	Verdict
parity-checker	PAIRED — mobile-only apps/mobile changes with no corresponding apps/web edits in this PR, but a live remote branch origin/chore/rd-web-habits-ui (commit b006c843, "chore(web): drive React Doctor to zero across web habits/ui components (#243)") is the paired sibling PR for the equivalent web bucket — same pattern as the already-merged #500/#501/#502 bucket pairs. Established paired-PR convention for the #243 rollout, not a parity violation.
i18n-syncer	N/A — no new user-facing strings; every t() call reuses pre-existing keys.
contract-aligner	N/A — no packages/shared/src/types/* or orbit-api change; also not verifiable in this CI job (orbit-api not checked out).
security-reviewer	N/A — no orbit-api change.
design-reviewer	PASS (with 2 non-blocking notes, see above) — no hardcoded brand colors, no new AI-slop tells, no banned patterns introduced.

Validation

Skipped per CI adaptation — Build / Unit Tests / SonarCloud run as separate required checks on this PR.

Deferred — N/A dimensions & files not verdicted

• Contract drift, security, backend hard rules: N/A — diff never touches packages/shared/src/types/*, endpoints.ts, or orbit-api.
• FEATURES.md parity: N/A — no user-facing feature, screen, gating, or tool-count change; pure internal refactor.
• Every changed file (all 47) got a verdict during the walk; nothing else deferred.

What's good

• Every suppression carries a genuine WHY + an issue URL (#243), satisfying the comment-policy gate — not narration.
• The one behavior-relevant deletion (lastSyncedValueRef in buffered-sheet-input.tsx) is correctly reasoned: the outer value !== prevValue guard already makes the removed ref redundant; focus/blur/keystroke paths show no regression.
• TouchableOpacity→Pressable migrations consistently replicate each component's original activeOpacity as pressed-state opacity, including correctly not re-guarding pressed against disabled where RN's own disabled prop already suppresses pressed.
• Tests were updated in lockstep with the Pressable migration rather than left broken.

Recommendation

Merge. No action required before merge. Two follow-up tickets worth opening (non-blocking, not filed as review findings): (1) a mobile shadow-token layer (sh-1/sh-2/sh-3) so settings-row.tsx's boxShadow isn't a bare literal, and (2) a shared pressed-opacity constant set to stop each component re-encoding its own historical activeOpacity value inline.

[claude]

Code Review: PR #504

Scope: PR #504 in thomasluizon/orbit-ui-mobile ("chore(mobile): drive React Doctor to zero across mobile ui/habits (#243)")
Recommendation: APPROVE

Summary

A mechanical, behavior-preserving React Doctor lint burn-down over apps/mobile/components/{ui,habits,habit-list*} — the mobile mirror of the already-merged web passes (#503, #505) for the same file surface. Changes are TouchableOpacity→Pressable swaps that faithfully replicate each prior activeOpacity as a pressed-state opacity style, .includes()→Set.has() lookups, memoized FlatList renderItem extraction, a useEffectEvent fix for a stale-closure risk, one RN shadow-props→boxShadow token swap, and ~40 react-doctor-disable-next-line suppressions each carrying a WHY note linking issue #243. No product behavior, data flow, or contract surface changes.

Note: this PR carries a prior APPROVE review from this same reviewer (submitted 2026-07-14T00:01:07Z). The only commits since then are two Merge branch 'main' into chore/rd-mobile-ui-habits merges bringing in already-independently-reviewed/merged sibling PRs (#503, #505); the PR's own file diff (apps/mobile/components/{ui,habits} + habit-list*) is unchanged. This review re-confirms that finding.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• [Info] apps/mobile/components/ui/settings-row.tsx:253 — boxShadow: '0px 1px 3px rgba(0, 0, 0, 0.35)' is a raw hex/rgba literal rather than a DESIGN.md named shadow token, but it's a 1:1 mechanical translation of the pre-existing shadowColor/shadowOpacity/shadowRadius/shadowOffset/elevation RN props this PR replaced (not a new or altered value), and matches the established platform-adapter pattern already used elsewhere in apps/mobile (RN shadow props can't consume CSS custom properties). Pre-existing token drift, out of scope for this behavior-preserving PR.
• [Info] Pressed-opacity values introduced by the Pressable migration vary per component (0.2 / 0.7 / 0.75 / 0.8 / 0.85), mirroring the activeOpacity values that already varied across these components pre-PR — no new inconsistency introduced. A follow-up ticket to consolidate to one kit-standard pressed-opacity token would be reasonable but isn't this PR's job.

Subagents

Agent	Verdict
parity-checker	PAIRED — all changed mobile files have current, non-drifted web mirrors already fixed by prior merged PRs #503/#505; the RN-specific lint rules driving this diff (rn-prefer-pressable, rn-no-scrollview-mapped-list, etc.) have no web equivalent, so the platform-only tooling fix is expected, not a parity violation. No behavioral drift found.
i18n-syncer	N/A — no user-facing strings added/changed/removed.
contract-aligner	N/A — no packages/shared/src/types/*, endpoints.ts, or orbit-api DTO changes.
security-reviewer	N/A — orbit-api not touched.
design-reviewer	PASS — no layout/spacing/radius/color-token regressions; base styles.* objects untouched, only conditional pressed ? {...} : null entries appended, matching the existing Pressable idiom already used elsewhere. No new UI compositions.

Validation

Check	Result
Lint	N/A in this review context (CI-driven); PR body states expo lint and RD --scope changed (0 new) are green.
Type check	N/A — PR body states mobile typecheck is green.
Tests	N/A — PR body states vitest 1072/1072 passing; spot-checked the two updated test files, which correctly assert node.type === Pressable by reference (not string tag), matching how react-test-renderer resolves composite vs. host components.
Build (api)	N/A — orbit-api not touched.

Deferred — N/A dimensions & files not verdicted

• Dimension 13 (Backend hard rules) — N/A, orbit-api not touched.
• Dimension 14 (FEATURES.md parity) — N/A, zero user-facing feature-surface change.
• Every changed file in the PR's file list was read and verdicted via the full diff; none skipped.
• Phase 6 (/validate) skipped per CI-wrapper convention — Build/Unit Tests/SonarCloud run as separate required checks on this PR.
• The sibling orbit-api repo is not checked out in this job; no cross-repo dimension was needed for this mobile-only, non-contract PR.

What's good

• Every suppression carries a specific, genuine WHY tied to #243 rather than a blanket disable — e.g. exhaustive-deps suppressions correctly identify the flagged dependency as an already-listed alias of a qualified member path.
• The Pressable migration correctly gates pressed-opacity behind !isDisabled/!isFirst/!isLast where the original had a disabled prop, and preserves each original activeOpacity (including RN's implicit 0.2 default) rather than normalizing it away.
• buffered-sheet-input.tsx — the removed lastSyncedValueRef is a genuine dead-ref deletion, not just lint-silencing.
• fresh-start-animation.tsx — the useEffectEvent extraction for handleComplete is the textbook-correct fix, correctly dropping onComplete from the effect's deps.
• Test updates correctly switch from string-tag to reference matching for Pressable, the only reliable way to locate a composite RN component in react-test-renderer.

Recommendation

Merge as-is. No Critical or High findings survived review; the two Info notes are pre-existing/out-of-scope observations, not action items.

[claude]

Code Review: PR #504 (thomasluizon/orbit-ui-mobile)

Scope: PR #504, branch chore/rd-mobile-ui-habits → main, 48 files changed (47 apps/mobile/** source/test files)
Recommendation: APPROVE

Summary

This is a mechanical, behavior-preserving React Doctor lint burn-down across apps/mobile/components/{ui,habits,habit-list*} — TouchableOpacity→Pressable conversions (replicating activeOpacity as pressed-state opacity styles), memoization fixes, Set-based lookups replacing .includes(), key-prop stabilization, and RN-specific lint-suppression comments (react-doctor-disable-next-line, all carrying a WHY + issue-#243 URL). The full diff was read, and the riskiest hunks were spot-verified in detail (the lastSyncedValueRef removal in buffered-sheet-input.tsx, the useEffectEvent refactor in fresh-start-animation.tsx, the frequencyHandlers memo removal, AppSelectOptionRow memoization, and the settings-row.tsx shadow-prop migration) — all preserve behavior correctly.

Findings

Critical: None
High: None
Medium: None (two candidates raised by design-reviewer were investigated and refuted — see below)
Low / Info: None posted (per Signal gate)

Two items the design-reviewer subagent raised were checked against the pre-diff code and dropped as false positives:

• The pressed ? {opacity:0.2} : null vs 0.7 variance in app-date-picker.tsx / app-time-picker.tsx exactly replicates each button's pre-existing activeOpacity (or RN's implicit 0.2 default where none was set) — confirmed against the diff's - lines. Not a regression; it's the PR doing exactly what its body promises.
• The raw boxShadow: '0px 1px 3px rgba(0,0,0,0.35)' in settings-row.tsx:253 is a syntax-preserving migration of the same pre-existing hardcoded values (shadowColor: '#000000', shadowOpacity: 0.35, etc.) that predate this diff — it isn't a new hardcode, and it isn't a brand-color/token violation (DESIGN.md's raw-token ban targets --slate-*/violet rgba, not generic black shadows). Pre-existing tech debt, not Critical, not in scope per rubric ("focus on changed code, not pre-existing issues").

Subagents

Agent	Verdict
parity-checker	PAIRED — every touched file's web mirror was already updated in companion PR #501 (chore(web): drive React Doctor to zero across web app routes (#243), confirmed MERGED 2026-07-13). This PR is inherently RN-primitive-only (Pressable, RN Animated, FlatList) with zero behavioral delta, matching the established split-PR precedent from #501/#502/#503.
i18n-syncer	N/A — no new or changed user-facing strings in this diff
contract-aligner	N/A — no packages/shared/src/types/*, endpoints.ts, or orbit-api changes
security-reviewer	N/A — no orbit-api code touched
design-reviewer	PASS — no hardcoded brand colors, no AI-slop tells, no new gradients/radii/fonts introduced; the two raised concerns were pre-existing and faithfully preserved (see Findings)

Validation

Check	Result
Lint	N/A — Build/Unit Tests/SonarCloud run as separate required CI checks for this PR; PR body states expo lint clean + RD --scope changed diff gate 0 new
Type check	N/A — same; PR body states mobile typecheck green
Tests	N/A — same; PR body states vitest 1072/1072 passing, and the two test files touched (app-time-picker.test.tsx, offline-unavailable-state.test.tsx) were verified to correctly import Pressable for their updated assertions
Build (api)	N/A — orbit-api untouched, and not checked out in this job

Deferred — N/A dimensions & files not verdicted

• i18n: N/A — surface not touched (no new/changed user-facing strings).
• Contract drift + backward-compat: N/A — surface not touched (no shared types/endpoints/orbit-api).
• Security (API side): N/A — surface not touched (no orbit-api); orbit-api sibling repo not checked out in this job, not verifiable in CI.
• Backend hard rules: N/A — surface not touched (no orbit-api).
• FEATURES.md parity: N/A — pure lint/behavior-preserving refactor, no user-facing feature surface added/changed/removed.

All 47 changed source/test files were read in full diff context and given a verdict; nothing changed was silently skipped.

What's good

• Every suppression carries a genuine WHY + https://github.com/.../issues/243 URL, satisfying the comment-policy dimension by construction.
• The one behavior-risky change (buffered-sheet-input.tsx's lastSyncedValueRef removal) was verified correct: the ref was tracking a value functionally identical to draft, and setDraft(value) already no-ops when value === draft, so dropping the redundant render-time ref write (itself an ESLint no-ref-current-in-render violation) changes nothing observable.
• useEffectEvent usage in fresh-start-animation.tsx matches the already-established pattern in apps/web (React 19.2.3 has it stable).
• Parity is satisfied through the documented two-PR split (#501 web, #504 mobile) rather than same-PR mirroring — consistent with prior merged precedent (#502/#503) for this exact React-Doctor burn-down effort.

Recommendation

Approve and merge. No Critical/High findings survived verification; the diff is a faithful, well-documented mechanical burn-down with its risky hunks individually checked for correctness.

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud