fix(mobile): keep reduced-motion views visible and aligned#23234

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include Android’s Animator scale in the reduced-motion boundary.

useReducedMotion() only reads TRANSITION_ANIMATION_SCALE. When Android sets only ANIMATOR_DURATION_SCALE=0, reducedMotion remains false and the composer still assigns FadeIn.duration(160) while the label expands. This configuration disables Android Animator-based animations, so the existing guard can still leave the label’s entering fade enabled and preserve the transparent-label failure. Treat either animation scale as reduced motion before assigning entering.

@coderabbitai I checked this against the installed Reanimated implementation and tested the exact configuration on the Android emulator at 2fb259c. With animator_duration_scale=0, transition_animation_scale=1, and window_animation_scale=1, I restarted the app and exercised short draft → expanded long draft → empty. The expanded model name remained visible. Updated captures using fictional demo data are now in the PR description. In this configuration useReducedMotion() is false, so the existing FadeIn still runs; this is also the same entering path as before the fix.

The distinction is that this FadeIn uses Reanimated’s worklet withTiming path, whose ReduceMotion.System setting comes from NativeProxy.getIsReducedMotion() / TRANSITION_ANIMATION_SCALE. Android’s separate Animator duration scale does not disable that path. Reanimated’s ANIMATOR_DURATION_SCALE handling in css/DurationScale.kt belongs to its CSS animation path, not this entering FadeIn. The bug reproduced when Reanimated’s own reduced-motion setting was true, and the new guard bypasses the entering animation in that case.

I’m treating the proposed additional Animator-scale boundary as a false positive for this fix. Please reconsider/withdraw this finding based on the native verification and the animation path above.

@PixPMusic, you are correct. I withdraw this finding.

I incorrectly applied Android Animator duration-scale behavior to Reanimated’s worklet-based FadeIn. The separate CSS animation handling does not establish a defect in this entering animation.

Your implementation trace and native test address the configuration I flagged. The existing guard bypasses the entering animation when Reanimated reports reduced motion. No additional Animator-scale boundary is needed for this fix.

Thanks for the clarification.

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟡 Medium patches/react-native-reanimated@4.7.1.patch:135

A Fabric update that changes a non-layout prop such as backgroundColor after the entering animation completes but before its queued frame is applied is lost. This branch only updates animation.finalView; addOngoingAnimations later replaces its props with the stale updateMap_ newProps, and completion cleanup reconciles only layout metrics and opacity. Rebase the retained frame's props on finalView while preserving its animated values so the update reaches the native view.

No longer relevant as of 4525f5c1d25b1000b27c18acb1b770e1d2396788

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Bound retries for permanently unmounted tags.

When correctedTags[i] == -1, addOngoingAnimations() retains the entry and calls schedulePullOnNextFrame() on every pull. That callback schedules another render. If the tag never mounts while its animation remains active, the same branch can request UI work every frame. Add a per-entry or bounded retry guard. Clear the guard when the update is applied, cancelled, removed, or the surface is torn down.

Cancellation and teardown already clear updateMap_, so this is not a post-cancellation leak.

@coderabbitai I traced this against the installed Reanimated 4.7.1 and RN 0.88 implementations. The conditional loop is possible, but I found no supported lifecycle that leaves an eligible tag permanently absent:

• preserveMountedTags calls resolveView; -1 means there is no native view object. A detached or offscreen view is not sufficient.
• Entering starts from a concrete Fabric Insert that remains in the returned transaction. Flattening produces removal/deletion, whose paths erase or cancel the animation and queued update.
• Rootless prerender suspends mounting, so it does not create this entering insert. Stopping a surface removes it from the shadow-tree registry; the scheduled callback can no longer visit it and renew the loop.

An arbitrary retry count has no established native-mount scheduling bound. It could strand the one completed frame while its real Create/Insert batch is delayed—the liveness failure this patch repairs. Android cancellation/idle verification recorded zero subsequent layout-animation pulls, as well as zero rendered frames; I am not treating rendered-frame counts alone as proof about callbacks.

I am leaving the retry intact. Please identify a supported sequence that leaves the concrete insert permanently absent while keeping its animation eligible, or withdraw this finding. A permanently--1 mock by itself would not establish that lifecycle.

@PixPMusic I withdraw this finding. I identified a conditional retry loop, but I did not establish a supported sequence that leaves the native view permanently absent while its animation remains eligible. A permanently--1 mock would not establish that sequence.

Your lifecycle explanation addresses that gap. An arbitrary retry limit could prevent a retained final frame from being applied after a delayed mount. The reported zero layout-animation pulls also address callback activity, not just rendered frames.

No retry guard is warranted by the evidence presented.

✅ Review thread resolved.

_{You are interacting with an AI system.}

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve synchronous props when rebasing the queued frame.

On iOS, a later React prop update can replace node->current with a target that lacks an earlier synchronous prop. The retarget branch then replaces updateValues.newProps with finalView plus enteringProps, which can drop that synchronous prop before the queued frame mounts.

Store all synchronous props separately from enteringProps, and merge them after rebasing the entering frame. Keep the new React target as the base so stale props are not replayed. This is limited to the queued, unmounted entering-animation path; Android accumulates these props in the light node.

@coderabbitai The set difference in the proposed sequence is real algebraically, but the claimed iOS ordering is not established by the native call paths.

In the non-backend path used here, ReanimatedModuleProxy::performOperations flushes layout-animation operations and executeLayoutAnimationsRequests() before applying synchronous updates. On iOS, RCTMountingManager::scheduleTransaction initiates mounting synchronously on the main queue; a reentrant transaction is deferred until the outer transaction finishes. The outer animation pull consumes the queued frame before the synchronous write, so the sequence “synchronous write → later Fabric retarget → consumption of that still-queued frame” needs an additional reachable ordering.

Bookkeeping can run for an absent native component, so absence alone is not my reason for disputing this. The question is how the iOS frame remains queued across those operations. The Android/iOS native fixture verified latest React colors/borders, custom final animation values, React opacity and cancellation/remount, but did not claim to reproduce this synchronous-write ordering.

Accumulating all synchronous values and overlaying them after every new React target can also preserve an obsolete value over a later accepted React update. I am not adding that blanket overlay without a concrete native failure and ownership rule. Please provide a supported iOS call sequence that leaves the completed frame queued through the synchronous write and later retarget, or withdraw this finding.

@PixPMusic I withdraw this finding. My analysis identified a possible prop-set difference, but did not establish a reachable iOS call sequence that exposes it.

The missing step is evidence that the completed frame remains queued through the synchronous write and a later Fabric retarget. An absent native component does not establish that ordering. The patch’s explicit unmounted-view retention branch is Android-only.

The proposed blanket overlay also lacked a rule for when a later React update supersedes a synchronous value. It is not justified by the evidence presented. No code change is needed for this finding.

✅ Review thread resolved.

_{You are interacting with an AI system.}