Animation fix - support dynamic fillValue #141
Animation fix - support dynamic fillValue #141jkhusanov wants to merge 4 commits intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the segmented-arc animation pipeline so fillValue can be changed dynamically (including decreases) while keeping the cap/segments visually in sync, and adds a regression test to confirm the animation restarts on rerender.
Changes:
- Updates
Segmentto keep an animated path mounted for all segments and to update the path when the animated value moves below a segment’s start. - Updates
SegmentedArcto mark animations complete via thestart()callback and to re-run the animation effect when the filled angle changes. - Adds a test asserting
Animated.timingis re-invoked whenfillValuechanges.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/components/Segment.js |
Keeps animated segment paths mounted and updates path drawing logic for dynamic values. |
src/SegmentedArc.js |
Reworks the animation effect to re-run on filled-angle changes and clears the “running” flag on completion. |
src/__tests__/SegmentedArc.spec.js |
Adds a test verifying animation re-triggers when fillValue updates. |
Comments suppressed due to low confidence (1)
src/components/Segment.js:36
- Removing the previous “complete” guard means that once
v.valueexceedsarc.end, this listener will continue callingsetNativePropson every animation frame for the remainder of the overall animation (and for every segment that’s already complete). This can become a noticeable performance cost with multiple segments. Consider reintroducing a small ref-based guard (e.g., track whether this segment is currently complete and skip updates whilev.value >= arc.end, resetting whenv.valuedrops back belowarc.endfor reverse animations).
if (v.value >= arc.end) {
arcRef.current.setNativeProps({
d: drawArc(arc.centerX, arc.centerY, radius, arc.start, arc.end)
});
} else {
arcRef.current.setNativeProps({
d: drawArc(arc.centerX, arc.centerY, radius, arc.start, v.value)
});
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@jkhusanov I've opened a new pull request, #143, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jkhusanov I've opened a new pull request, #144, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jkhusanov I've opened a new pull request, #145, to work on those changes. Once the pull request is ready, I'll request review from you. |
* 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>
It addresses the issue #103, the solution is inspired by #104, and further improved to fix animation back when fillValue is set to a lower value than the original, as reported as a comment in that PR.
Randomly changing fillValue on press:
Screen.Recording.2026-02-17.at.5.36.08.PM.mov