Animation fix - support dynamic fillValue (#141)

* Support dynamic fill value

* Fix segments animation when going back

* Fix animation race condition for dynamic fillValue updates (#144)

* Initial plan

* Fix animation race condition by cancelling in-flight animations

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Add test for animation cancellation during updates

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Properly handle animation reference cleanup after cancellation

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Improve animation cleanup logic to prevent race conditions

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Refine animation cleanup to prevent race conditions

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Address final code review feedback

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

* Clear animation reference in cleanup and improve test helper reusability

Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jkhusanov <25942541+jkhusanov@users.noreply.github.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

Author: jkhusanov

src/SegmentedArc.js

@@ -49,7 +49,7 @@ export const SegmentedArc = ({
   }
 
   const [arcAnimatedValue] = useState(new Animated.Value(0));
-  const animationRunning = useRef(false);
+  const currentAnimation = useRef(null);
   useShowSegmentedArcWarnings({ segments: segmentsProps });
 
   const { dataErrors, segments, fillValue, filledArcWidth, emptyArcWidth, spaceBetweenSegments, arcDegree, radius } =
@@ -138,22 +138,38 @@ export const SegmentedArc = ({
 
   useEffect(() => {
     if (!lastFilledSegment) return;
-    if (animationRunning.current) return;
     if (!isAnimated) return;
-    animationRunning.current = true;
-    Animated.timing(arcAnimatedValue, {
+
+    // Cancel any in-flight animation to prevent dropped updates
+    if (currentAnimation.current) {
+      currentAnimation.current.stop();
+    }
+
+    const animation = Animated.timing(arcAnimatedValue, {
       toValue: lastFilledSegment.filled,
       duration: animationDuration,
       delay: animationDelay,
       useNativeDriver: false,
       easing: Easing.out(Easing.ease)
-    }).start();
+    });
 
-    const listenerId = arcAnimatedValue.addListener(e => {
-      if (e.value === lastFilledSegment.filled) animationRunning.current = false;
+    currentAnimation.current = animation;
+
+    animation.start(({ finished }) => {
+      // Only clear if this is still the current animation AND it finished successfully (not stopped)
+      if (currentAnimation.current === animation && finished) {
+        currentAnimation.current = null;
+      }
     });
-    return () => arcAnimatedValue.removeListener(listenerId);
-  }, []);
+
+    // Cleanup: stop this specific animation if it's still running
+    return () => {
+      if (currentAnimation.current === animation) {
+        animation.stop();
+        currentAnimation.current = null;
+      }
+    };
+  }, [lastFilledSegment.filled, animationDuration, animationDelay, isAnimated]);
 
   if (arcs.length === 0) {
     return null;

src/__tests__/SegmentedArc.spec.js

@@ -42,11 +42,16 @@ describe('SegmentedArc', () => {
     return render(<SegmentedArc {...properties} />);
   };
 
+  const createCompletedAnimationMock = () => ({
+    start: jest.fn(cb => cb && cb({ finished: true })),
+    stop: jest.fn()
+  });
+
   beforeEach(() => {
     Animated.timing = jest.fn();
     Easing.out = jest.fn();
     Easing.ease = jest.fn();
-    Animated.timing.mockReturnValue({ start: jest.fn() });
+    Animated.timing.mockReturnValue({ start: jest.fn(), stop: jest.fn() });
     jest.spyOn(console, 'warn').mockImplementation();
     props = { segments, fillValue: 50 };
   });
@@ -304,6 +309,63 @@ describe('SegmentedArc', () => {
     expect(Easing.out).not.toHaveBeenCalledWith(Easing.ease);
   });
 
+  it('re-runs animation when fillValue changes dynamically', () => {
+    Animated.timing.mockReturnValue(createCompletedAnimationMock());
+
+    wrapper = render(<SegmentedArc {...props} fillValue={25} />);
+    expect(Animated.timing).toHaveBeenCalledTimes(1);
+
+    const firstCallToValue = Animated.timing.mock.calls[0][1].toValue;
+
+    Animated.timing.mockClear();
+    Animated.timing.mockReturnValue(createCompletedAnimationMock());
+
+    wrapper.rerender(<SegmentedArc {...props} fillValue={75} />);
+    expect(Animated.timing).toHaveBeenCalledTimes(1);
+
+    const secondCallToValue = Animated.timing.mock.calls[0][1].toValue;
+    expect(secondCallToValue).not.toBe(firstCallToValue);
+    expect(secondCallToValue).toBeGreaterThan(firstCallToValue);
+  });
+
+  it('cancels in-flight animation when fillValue changes before animation completes', () => {
+    let firstAnimationCallback;
+    const mockStop = jest.fn();
+    const mockStart = jest.fn(cb => {
+      firstAnimationCallback = cb;
+    });
+    Animated.timing.mockReturnValue({ start: mockStart, stop: mockStop });
+
+    wrapper = render(<SegmentedArc {...props} fillValue={25} />);
+    expect(Animated.timing).toHaveBeenCalledTimes(1);
+    expect(mockStart).toHaveBeenCalledTimes(1);
+
+    // Simulate fillValue changing before animation completes
+    Animated.timing.mockClear();
+    const newMockStop = jest.fn();
+    const newMockStart = jest.fn();
+    Animated.timing.mockReturnValue({ start: newMockStart, stop: newMockStop });
+
+    wrapper.rerender(<SegmentedArc {...props} fillValue={75} />);
+
+    // Verify that the previous animation was stopped
+    expect(mockStop).toHaveBeenCalled();
+    // Verify that a new animation was started
+    expect(Animated.timing).toHaveBeenCalledTimes(1);
+    expect(newMockStart).toHaveBeenCalledTimes(1);
+
+    // Simulate the stopped animation's callback executing with finished: false
+    // This should NOT clear currentAnimation (because finished is false)
+    if (firstAnimationCallback) {
+      firstAnimationCallback({ finished: false });
+    }
+
+    // Verify that the new animation is still active by unmounting and checking
+    // that its stop method is called
+    wrapper.unmount();
+    expect(newMockStop).toHaveBeenCalled();
+  });
+
   it('sets the last segment for lastFilledSegment prop when fillValue is equal or greater than 100%', () => {
     props.fillValue = 100;
     wrapper = getWrapper(props);

src/components/Segment.js

@@ -12,20 +12,23 @@ export const Segment = ({ arc }) => {
   const { filledArcWidth, radius, isAnimated, emptyArcWidth, arcAnimatedValue } = segmentedArcContext;
 
   const arcRef = useRef();
-  const animationComplete = useRef(false);
 
   useEffect(() => {
     if (!isAnimated) return;
     const listener = arcAnimatedValue.addListener(v => {
       if (!arcRef.current) return;
-      if (animationComplete.current) return;
-      if (v.value <= arc.start) return;
+
+      if (v.value <= arc.start) {
+        arcRef.current.setNativeProps({
+          d: drawArc(arc.centerX, arc.centerY, radius, arc.start, arc.start)
+        });
+        return;
+      }
 
       if (v.value >= arc.end) {
         arcRef.current.setNativeProps({
           d: drawArc(arc.centerX, arc.centerY, radius, arc.start, arc.end)
         });
-        animationComplete.current = true;
       } else {
         arcRef.current.setNativeProps({
           d: drawArc(arc.centerX, arc.centerY, radius, arc.start, v.value)
@@ -34,7 +37,7 @@ export const Segment = ({ arc }) => {
     });
 
     return () => arcAnimatedValue.removeListener(listener);
-  }, []);
+  }, [arc.start, arc.end, arc.centerX, arc.centerY, isAnimated, arcAnimatedValue, radius]);
 
   return (
     <G>
@@ -45,9 +48,7 @@ export const Segment = ({ arc }) => {
         d={drawArc(arc.centerX, arc.centerY, radius, arc.start, arc.end)}
       />
 
-      {isAnimated && arc.filled > arc.start && (
-        <AnimatedPath ref={arcRef} fill="none" stroke={arc.filledColor} strokeWidth={filledArcWidth} />
-      )}
+      {isAnimated && <AnimatedPath ref={arcRef} fill="none" stroke={arc.filledColor} strokeWidth={filledArcWidth} />}
 
       {!isAnimated && arc.filled > arc.start && (
         <Path