From 18757d9ff1d34e42ab5e1931b0f5758de0bcf6bf Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 17 Aug 2026 00:06:46 -0400 Subject: [PATCH] fix: tear down loop animations the current props no longer request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A looping animation was only ever removed when a property was still in animatedProperties AND its value changed. Props that drop a property from `animate` entirely, or that keep animating it but stop asking for `loop`, hit neither gate, so the infinite animation kept driving the view — and on iOS the saved snapshot replayed it on every didMoveToWindow. Both platforms now filter running loops against the current props on each update, and iOS also filters the snapshot before replaying it on re-attach. Adds example/app/issues/54 covering both cases. --- android/src/main/java/com/ease/EaseView.kt | 93 ++++++++++ example/app/issues/54/_layout.tsx | 23 +++ example/app/issues/54/index.tsx | 195 +++++++++++++++++++++ example/app/issues/54/other.tsx | 37 ++++ example/src/demos/index.ts | 5 + ios/EaseView.mm | 145 +++++++++++++++ 6 files changed, 498 insertions(+) create mode 100644 example/app/issues/54/_layout.tsx create mode 100644 example/app/issues/54/index.tsx create mode 100644 example/app/issues/54/other.tsx diff --git a/android/src/main/java/com/ease/EaseView.kt b/android/src/main/java/com/ease/EaseView.kt index 190c88f..6bc6835 100644 --- a/android/src/main/java/com/ease/EaseView.kt +++ b/android/src/main/java/com/ease/EaseView.kt @@ -124,6 +124,76 @@ class EaseView(context: Context) : ReactViewGroup(context) { } } + // Drop every running loop the current props no longer ask for. + // + // The per-property paths in applyAnimateValues() only cancel an animator + // when the property is still in animatedProperties AND its value changed. + // Props that drop a property from `animate` entirely, or that keep + // animating it but stop asking for `loop`, hit neither gate — the infinite + // ValueAnimator keeps driving the view forever, even after the view is + // reused for different content. Treat the current props as the source of + // truth instead. + private fun removeStaleLoopAnimations( + opacity: Float, + translateX: Float, + translateY: Float, + scaleX: Float, + scaleY: Float, + rotate: Float, + rotateX: Float, + rotateY: Float, + borderRadius: Float, + borderWidth: Float, + elevation: Float + ) { + if (runningAnimators.isEmpty()) return + + val mask = animatedProperties + for ((animatorKey, property) in LOOPABLE_PROPERTIES) { + val animator = runningAnimators[animatorKey] as? ValueAnimator ?: continue + if (animator.repeatCount != ValueAnimator.INFINITE) continue + + val (propertyMask, configName) = property + val stillAnimated = mask and propertyMask != 0 + if (stillAnimated && getTransitionConfig(configName).loop.let { it == "repeat" || it == "reverse" }) { + continue + } + + animator.cancel() + runningAnimators.remove(animatorKey) + + // cancel() strands the property wherever the sweep happened to be, + // so write the resting state the current props ask for. Every + // animate* prop carries its identity value once JS clears the mask + // bit, so the incoming values are correct either way. Colors are + // skipped — an unset color has no identity value and the style now + // owns it. + val target: Float + val prev: Float? + val apply: (Float) -> Unit + when (animatorKey) { + "alpha" -> { target = opacity; prev = prevOpacity; apply = { this.alpha = it } } + "translationX" -> { target = translateX; prev = prevTranslateX; apply = { this.translationX = it } } + "translationY" -> { target = translateY; prev = prevTranslateY; apply = { this.translationY = it } } + "scaleX" -> { target = scaleX; prev = prevScaleX; apply = { this.scaleX = it } } + "scaleY" -> { target = scaleY; prev = prevScaleY; apply = { this.scaleY = it } } + "rotation" -> { target = rotate; prev = prevRotate; apply = { this.rotation = it } } + "rotationX" -> { target = rotateX; prev = prevRotateX; apply = { this.rotationX = it } } + "rotationY" -> { target = rotateY; prev = prevRotateY; apply = { this.rotationY = it } } + "animateBorderRadius" -> { target = borderRadius; prev = prevBorderRadius; apply = { setAnimateBorderRadius(it) } } + "animateBorderWidth" -> { target = borderWidth; prev = prevBorderWidth; apply = { setAnimateBorderWidth(it) } } + "elevation" -> { target = elevation; prev = prevElevation; apply = { this.elevation = it } } + else -> continue + } + + // Skip only when a path below will animate this property anyway: + // those read the live view value as their "from", so writing the + // target first would flatten the animation into a no-op. + if (stillAnimated && prev != null && prev != target) continue + apply(target) + } + } + companion object { // Bitmask flags — must match JS constants const val MASK_OPACITY = 1 shl 0 @@ -140,6 +210,23 @@ class EaseView(context: Context) : ReactViewGroup(context) { const val MASK_BORDER_COLOR = 1 shl 11 // Masks 12-15 are shadow properties (iOS only) const val MASK_ELEVATION = 1 shl 16 + + // runningAnimators key → (mask bit, getTransitionConfig name) + private val LOOPABLE_PROPERTIES = listOf( + "alpha" to (MASK_OPACITY to "opacity"), + "translationX" to (MASK_TRANSLATE_X to "translateX"), + "translationY" to (MASK_TRANSLATE_Y to "translateY"), + "scaleX" to (MASK_SCALE_X to "scaleX"), + "scaleY" to (MASK_SCALE_Y to "scaleY"), + "rotation" to (MASK_ROTATE to "rotate"), + "rotationX" to (MASK_ROTATE_X to "rotateX"), + "rotationY" to (MASK_ROTATE_Y to "rotateY"), + "animateBorderRadius" to (MASK_BORDER_RADIUS to "borderRadius"), + "backgroundColor" to (MASK_BACKGROUND_COLOR to "backgroundColor"), + "animateBorderWidth" to (MASK_BORDER_WIDTH to "borderWidth"), + "borderColor" to (MASK_BORDER_COLOR to "borderColor"), + "elevation" to (MASK_ELEVATION to "elevation"), + ) } // --- Transform origin (0–1 fractions) --- @@ -473,6 +560,12 @@ class EaseView(context: Context) : ReactViewGroup(context) { onTransitionEnd?.invoke(true) } else { // Subsequent updates: animate changed properties (skip non-animated) + // Runs after the animationBatchId bump above so the cancellations + // can't be mistaken for this batch's animations completing. + removeStaleLoopAnimations( + opacity, translateX, translateY, scaleX, scaleY, + rotate, rotateX, rotateY, borderRadius, borderWidth, elevation + ) var anyPropertyChanged = false if (prevOpacity != null && mask and MASK_OPACITY != 0 && prevOpacity != opacity) { diff --git a/example/app/issues/54/_layout.tsx b/example/app/issues/54/_layout.tsx new file mode 100644 index 0000000..f492cd2 --- /dev/null +++ b/example/app/issues/54/_layout.tsx @@ -0,0 +1,23 @@ +import { Tabs, Stack } from 'expo-router'; + +export default function StaleLoopLayout() { + return ( + <> + + + + + + + ); +} diff --git a/example/app/issues/54/index.tsx b/example/app/issues/54/index.tsx new file mode 100644 index 0000000..a651ba7 --- /dev/null +++ b/example/app/issues/54/index.tsx @@ -0,0 +1,195 @@ +import { useState } from 'react'; +import { Pressable, StyleSheet, Text, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; +import { EaseView } from 'react-native-ease'; + +// Issue #54 — a looping animation survives on an EaseView whose current props +// no longer ask for it. +// https://github.com/appandflow/react-native-ease/issues/54 +// +// Both cards below keep the SAME EaseView instance across the load — no key, +// no remount — which is what a list row does when it swaps a skeleton for its +// loaded content. The teardown paths in updateProps are gated on "property is +// still in animatedProperties AND its value changed", so neither card hits +// them and the shimmer keeps sweeping over the content. +// +// Steps to reproduce: +// 1. Both cards shimmer — a band sweeps left→right on a 1.5 s linear loop. +// 2. Press "Load content". Both cards swap to their loaded state. +// 3. Expected: both bands stop. +// Without the fix: both keep sweeping forever. +// 4. Switch to the Other tab and back. Without the fix the loops are replayed +// by didMoveToWindow → reapplyLoopAnimations even after being cancelled. +// 5. "Back to skeleton" remounts both views to start the shimmer over. + +const SWEEP = 64; + +export default function StaleLoopTab() { + const insets = useSafeAreaInsets(); + const [loaded, setLoaded] = useState(false); + // Loops are only created on first mount, so resetting has to remount the + // views. Loading must NOT — the bug needs the same EaseView instance. + const [mountKey, setMountKey] = useState(0); + + return ( + + Issue #54 — stale shimmer loop + + Press Load content. Both bands must stop sweeping. Then switch to the + Other tab and back — they must still be stopped. + + + + + + + + + + + + + + + + setLoaded(true)} + > + Load content + + { + setLoaded(false); + setMountKey((k) => k + 1); + }} + > + Back to skeleton + + + + ); +} + +function Card({ + title, + detail, + children, +}: { + title: string; + detail: string; + children: React.ReactNode; +}) { + return ( + + {title} + {detail} + {children} + + ); +} + +const styles = StyleSheet.create({ + root: { + flex: 1, + backgroundColor: '#1a1a2e', + paddingHorizontal: 20, + }, + heading: { + fontSize: 22, + fontWeight: '700', + color: '#fff', + marginBottom: 8, + }, + body: { + fontSize: 14, + color: '#aaaacc', + marginBottom: 28, + lineHeight: 20, + }, + card: { + marginBottom: 28, + }, + cardTitle: { + fontSize: 15, + fontWeight: '600', + color: '#e0e0ff', + marginBottom: 4, + }, + cardDetail: { + fontSize: 11, + fontFamily: 'monospace', + color: '#6666aa', + marginBottom: 10, + }, + track: { + height: 64, + borderRadius: 12, + backgroundColor: '#16213e', + overflow: 'hidden', + }, + band: { + width: 56, + height: '100%', + alignSelf: 'center', + backgroundColor: '#4a90d9', + }, + buttons: { + flexDirection: 'row', + gap: 12, + justifyContent: 'center', + }, + button: { + paddingHorizontal: 20, + paddingVertical: 12, + borderRadius: 10, + backgroundColor: '#16213e', + }, + buttonDisabled: { + opacity: 0.4, + }, + buttonText: { + color: '#e0e0ff', + fontSize: 15, + fontWeight: '600', + }, +}); diff --git a/example/app/issues/54/other.tsx b/example/app/issues/54/other.tsx new file mode 100644 index 0000000..32aa642 --- /dev/null +++ b/example/app/issues/54/other.tsx @@ -0,0 +1,37 @@ +import { StyleSheet, Text, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; + +// Switching to this tab detaches the shimmer views from the window, which is +// what makes didMoveToWindow → reapplyLoopAnimations run when you come back. +export default function OtherTab() { + const insets = useSafeAreaInsets(); + + return ( + + Other tab + + Go back to the Shimmer tab. Any card that was loaded must still be + still. + + + ); +} + +const styles = StyleSheet.create({ + root: { + flex: 1, + backgroundColor: '#1a1a2e', + paddingHorizontal: 20, + }, + heading: { + fontSize: 22, + fontWeight: '700', + color: '#fff', + marginBottom: 8, + }, + body: { + fontSize: 14, + color: '#aaaacc', + lineHeight: 20, + }, +}); diff --git a/example/src/demos/index.ts b/example/src/demos/index.ts index 6ec1cf2..d46a16a 100644 --- a/example/src/demos/index.ts +++ b/example/src/demos/index.ts @@ -153,6 +153,11 @@ export const demos: Record = { title: 'Issue #45 — Android modal background', section: 'Issues', }, + 'issue-54': { + route: '/issues/54', + title: 'Issue #54 — Stale shimmer loop', + section: 'Issues', + }, 'issue-loop-cancel': { route: '/issues/loop-cancel', title: 'Audit — Cancelled loop resurrects', diff --git a/ios/EaseView.mm b/ios/EaseView.mm index 2acab0b..04358ba 100644 --- a/ios/EaseView.mm +++ b/ios/EaseView.mm @@ -155,6 +155,66 @@ static EaseTransitionConfig transitionConfigFromStruct(const T &src) { return transitionConfigFromStruct(t.defaultConfig); } +static bool isLoopingTransition(const EaseTransitionConfig &config) { + return config.loop == "repeat" || config.loop == "reverse"; +} + +// Map a saved loop animation key back to the property it drives. Returns the +// mask bit and writes the transition-config property name, or 0 if the key is +// not one a loop can be saved under. +static int easePropertyForLoopKey(NSString *key, std::string &outName) { + if ([key isEqualToString:kAnimKeyOpacity]) { + outName = "opacity"; + return kMaskOpacity; + } else if ([key isEqualToString:kAnimKeyTransformTransX]) { + outName = "translateX"; + return kMaskTranslateX; + } else if ([key isEqualToString:kAnimKeyTransformTransY]) { + outName = "translateY"; + return kMaskTranslateY; + } else if ([key isEqualToString:kAnimKeyTransformScaleX]) { + outName = "scaleX"; + return kMaskScaleX; + } else if ([key isEqualToString:kAnimKeyTransformScaleY]) { + outName = "scaleY"; + return kMaskScaleY; + } else if ([key isEqualToString:kAnimKeyTransformRotateZ]) { + outName = "rotate"; + return kMaskRotate; + } else if ([key isEqualToString:kAnimKeyTransformRotateX]) { + outName = "rotateX"; + return kMaskRotateX; + } else if ([key isEqualToString:kAnimKeyTransformRotateY]) { + outName = "rotateY"; + return kMaskRotateY; + } else if ([key isEqualToString:kAnimKeyCornerRadius]) { + outName = "borderRadius"; + return kMaskBorderRadius; + } else if ([key isEqualToString:kAnimKeyBackgroundColor]) { + outName = "backgroundColor"; + return kMaskBackgroundColor; + } else if ([key isEqualToString:kAnimKeyBorderWidth]) { + outName = "borderWidth"; + return kMaskBorderWidth; + } else if ([key isEqualToString:kAnimKeyBorderColor]) { + outName = "borderColor"; + return kMaskBorderColor; + } else if ([key isEqualToString:kAnimKeyShadowOpacity]) { + outName = "shadowOpacity"; + return kMaskShadowOpacity; + } else if ([key isEqualToString:kAnimKeyShadowRadius]) { + outName = "shadowRadius"; + return kMaskShadowRadius; + } else if ([key isEqualToString:kAnimKeyShadowColor]) { + outName = "shadowColor"; + return kMaskShadowColor; + } else if ([key isEqualToString:kAnimKeyShadowOffset]) { + outName = "shadowOffset"; + return kMaskShadowOffset; + } + return 0; +} + // Find lowest property name with a set mask bit among transform properties static std::string lowestTransformPropertyName(int mask) { if (mask & kMaskTranslateX) @@ -374,6 +434,82 @@ - (void)removeEaseAnimationForKey:(NSString *)key { [_loopAnimations removeObjectForKey:key]; } +// Drop every saved loop the current props no longer ask for. +// +// The per-property paths in updateProps: only tear an animation down when the +// property is still in animatedProperties AND its value changed. Props that +// drop a property from `animate` entirely, or that keep animating it but stop +// asking for `loop`, therefore hit neither gate — the infinite CAAnimation +// stays on the layer forever, and _loopAnimations replays it on every +// didMoveToWindow. Treat the current props as the source of truth instead. +- (void)removeStaleLoopAnimationsForProps:(const EaseViewProps &)props { + if (_loopAnimations.count == 0) { + return; + } + + int mask = props.animatedProperties; + BOOL needsTransformReset = NO; + + // updateProps: already runs inside one, but didMoveToWindow does not, and + // the model writes below must not start implicit animations. + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + + for (NSString *key in [_loopAnimations.allKeys copy]) { + std::string propertyName; + int propertyMask = easePropertyForLoopKey(key, propertyName); + if (propertyMask == 0) { + continue; + } + + BOOL stillAnimated = (mask & propertyMask) != 0; + if (stillAnimated && + isLoopingTransition(transitionConfigForProperty(propertyName, props))) { + continue; + } + + [self removeEaseAnimationForKey:key]; + + // Removing the animation drops the presentation layer back to the model, + // which still holds the loop's target — a shimmer frozen mid-sweep. Reset + // only when the property left animatedProperties: while it is still + // animated the model already holds the value the current props ask for, + // and writing here would cause a visible jump. Every animate* prop + // defaults to its identity value once JS clears the mask bit, so the + // current props are also the correct resting state. + if (stillAnimated) { + continue; + } + if (propertyMask & kMaskAnyTransform) { + // Recompose the whole matrix rather than writing a sub-key path: a + // transform carrying m34 perspective can't be reliably decomposed, and + // this keeps any sub-property that IS still animated at its own value. + needsTransformReset = YES; + } else if (propertyMask == kMaskOpacity) { + self.layer.opacity = props.animateOpacity; + } else if (propertyMask == kMaskBorderRadius) { + self.layer.cornerRadius = props.animateBorderRadius; + } else if (propertyMask == kMaskBorderWidth) { + self.layer.borderWidth = props.animateBorderWidth; + } else if (propertyMask == kMaskShadowOpacity) { + self.layer.shadowOpacity = props.animateShadowOpacity; + } else if (propertyMask == kMaskShadowRadius) { + self.layer.shadowRadius = props.animateShadowRadius; + } else if (propertyMask == kMaskShadowOffset) { + self.layer.shadowOffset = + CGSizeMake(props.animateShadowOffsetX, props.animateShadowOffsetY); + } + // Colors (background, border, shadow) are deliberately not reset: an + // unset SharedColor has no identity value, so writing one here would + // clobber a color the style is now responsible for. + } + + if (needsTransformReset) { + self.layer.transform = [self targetTransformFromProps:props]; + } + [CATransaction commit]; +} + - (void)reapplyLoopAnimations { if (_loopAnimations.count == 0) { return; @@ -887,6 +1023,9 @@ - (void)updateProps:(const Props::Shared &)props } else { // Subsequent updates: animate changed properties [self beginAnimationBatch]; + // After beginAnimationBatch so the removals below can't be mistaken for + // this batch's animations completing. + [self removeStaleLoopAnimationsForProps:newViewProps]; BOOL anyPropertyChanged = NO; if ((mask & kMaskOpacity) && @@ -1242,6 +1381,12 @@ - (void)didMoveToWindow { // When the view re-attaches (e.g. after a react-navigation tab switch), // re-apply any loop animations that were running. if (self.window != nil && !_isFirstMount) { + // The snapshot may predate a props update that stopped asking for the + // loop, so filter it against the current props before replaying. + if (_props) { + [self removeStaleLoopAnimationsForProps:*std::static_pointer_cast< + const EaseViewProps>(_props)]; + } [self reapplyLoopAnimations]; } }