Repository navigation
feat(SET): custom shared element transitions - #10885
bartlomiejbloniarz wants to merge 39 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Priority: Change: Feature · Severity of issue fixed: Medium Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check |  |
Issue #10876 is limited to transform interpolation during iOS back swipes. The PR also adds the full SharedTransition.custom API, PROGRESS_ANIMATION selection, broad snapshot and property support,… |
Split the custom-transition API, examples, documentation, public exports, and unrelated matrix-rotation changes into separate pull requests, or link active issues that require those changes. Keep this pull request focused on the #10876 prog… |
-
Check name Status Explanation Title check The title clearly and concisely identifies the main change: custom shared element transitions for SET. Linked Issues check Issue #10876requires progress-driven iOS back swipes to interpolate transforms, start at the current view, and restore the source after cancellation.SharedTransitions.cppnow records the progress …Description check The pull request description clearly matches the changes. It explains the restored custom and progress shared-transition APIs, native implementation, tests, examples, documentation, and the linked iss… Full details: Linked Issues check
-
Explanation
Issue
#10876requires progress-driven iOS back swipes to interpolate transforms, start at the current view, and restore the source after cancellation.SharedTransitions.cppnow records the progress configuration, applies it to gesture-driven shared elements with the current transition progress, and restores sources for cancelled transitions.SharedTransition.tsinterpolates changed properties and decomposes and interpolates transform matrices. Jest tests cover progress 0, 0.5, and 1, including transform output. Runtime tests cover progress from 0 to 1.
Full details: Description check
-
Explanation
The pull request description clearly matches the changes. It explains the restored custom and progress shared-transition APIs, native implementation, tests, examples, documentation, and the linked issue fix.
-
Full details: Out of Scope Changes check
-
Explanation
Issue
#10876is limited to transform interpolation during iOS back swipes. The PR also adds the fullSharedTransition.customAPI,PROGRESS_ANIMATIONselection, broad snapshot and property support, many custom-transition examples, public exports, documentation, and a separate Y-axis matrix decomposition fix. These changes are not required by#10876and are not supporting tests or refactoring for that issue.Resolution
Split the custom-transition API, examples, documentation, public exports, and unrelated matrix-rotation changes into separate pull requests, or link active issues that require those changes. Keep this pull request focused on the
#10876progress-path fix and its tests.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d36a0a4e-555d-4246-8bec-9af28de9c266
📒 Files selected for processing: 41
apps/common-app/runtime-tests/reanimated/tests/layoutAnimations/sharedTransition.test.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/ChangeTheme.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/BackgroundColor.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/BasicCustom.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/BorderRadius.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/GlobalOrigin.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/Opacity.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/ProgressAnimation.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/Transform.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/TransformMatrix.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/Window.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/index.tsxapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/CustomTransition/styles.tsapps/common-app/src/apps/reanimated/examples/SharedElementTransitions/Profiles.tsxapps/common-app/src/apps/reanimated/examples/index.tsdocs/docs-reanimated/docs/shared-element-transitions/overview.mdxpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationType.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsManager.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsManager.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/PropsDiffer.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/PropsDiffer.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/SharedTransitionValuesFactory.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/SharedTransitionValuesFactory.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/SharedTransitions.cpppackages/react-native-reanimated/__tests__/SharedTransition.test.tspackages/react-native-reanimated/__typetests__/sharedTransitionTest.tst.tspackages/react-native-reanimated/changelog/shared-transition-custom-animations.feature.mdpackages/react-native-reanimated/src/UpdateLayoutAnimations.native.tspackages/react-native-reanimated/src/UpdateLayoutAnimations.tspackages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsxpackages/react-native-reanimated/src/animation/utilCommon.tspackages/react-native-reanimated/src/commonTypes.tspackages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsxpackages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.tsxpackages/react-native-reanimated/src/index.tspackages/react-native-reanimated/src/layoutReanimation/SharedTransition.tspackages/react-native-reanimated/src/mock.ts
💤 Files with no reviewable changes: 2
- packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/PropsDiffer.h
- packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/PropsDiffer.cpp
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
aad814b to
43a9dca
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Format this file before merging. · matrixUtils.tsx:1
packages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsx:1
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFormat this file before merging.
The static-check pipeline reports that this file fails the formatter check during
yarn lint:js. Run the repository formatter on this file and commit the result. The pipeline does not identify a more precise offending line.Source: Pipeline failures
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4e763106-68c5-4e57-9da2-7f2f3fbcfd92
📒 Files selected for processing: 3
packages/react-native-reanimated/__tests__/SharedTransition.test.tspackages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsxpackages/react-native-reanimated/src/layoutReanimation/SharedTransition.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
|
@coderabbitai The |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Preserve exact ±90° Y endpoints during interpolation. · matrixUtils.tsx:453-467
packages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsx:453-467
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve exact ±90° Y endpoints during interpolation.
When
sinRyis exactly1or-1,decomposeMatrixIntoMatricesAndAnglescan returnrx = πor-πfor a matrix whose actual X rotation is zero.interpolateMatrix(0, start, stop)andinterpolateMatrix(1, start, stop)then rebuild a matrix that differs from the corresponding endpoint. The shared-transition progress path uses this interpolation for transform matrices, so the moving element can start with a transform jump and finish in an incorrect transform state.Use the matrix entries that remain unambiguous when Y reaches gimbal lock:
Suggested fix
- rx = Math.atan2(sinRy * rotationMatrix[0][1], sinRy * rotationMatrix[0][2]); + rx = Math.atan2(rotationMatrix[2][1], rotationMatrix[1][1]);
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7c6b9ee0-5c97-4ae2-b404-a98689b1cb53
📒 Files selected for processing: 1
packages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
At the gimbal lock the X angle was read from the first row of the rotation, which gives ±π for a rotation around Y alone, so rebuilding the matrix flipped the Y and Z axes. The X and Z rotations combine there, and the second column gives their angle. Addresses #10885 (review) Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
|
@coderabbitai The ±90° Y case was broken: a rotation around Y alone decomposed to The suggested |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^matrixUtils\.(tsx|test\.tsx)$'
ast-grep outline packages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsx
ast-grep outline packages/react-native-reanimated/__tests__/matrixUtils.test.tsx
rg -n -C 22 'Math\.atan2|Math\.asin|gimbal' packages/react-native-reanimated/src/animation/transformationMatrix/matrixUtils.tsx
rg -n -C 25 '0\.4|0\.7|interpolateMatrix|decomposeMatrixIntoMatricesAndAngles' packages/react-native-reanimated/__tests__/matrixUtils.test.tsxLength of output: 6384
The implementation now uses ✏️ Learnings added
You are interacting with an AI system. |
1ed6f8a to
07d84a9
Compare
07d84a9 to
7f972cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d9710d72-d371-4d52-aff5-6249d82c1bd4
📒 Files selected for processing: 6
apps/common-app/runtime-tests/reanimated/tests/layoutAnimations/sharedTransition.test.tsxpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsManager.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/SharedTransitions.cpppackages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
|
@coderabbitai The moderate merge risk in your summary rests on the two findings you withdrew in #10885 (comment) and #10885 (comment). Fabric does not reuse view tags. Progress worklet errors propagate the same way as errors in the custom animation factory that |
|
The earlier clarifications establish that:
Revised assessment: low merge risk based on the available review evidence. This assessment does not establish that runtime tests pass. I’m requesting a summary update to remove both withdrawn findings as risk factors and revise the rating. 🧠 Learnings used
✅ Action performedSummary regeneration completed, but the summary was unchanged. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7b4797a6-94a8-4f24-81b8-b2cba5ff3da6
📒 Files selected for processing: 3
apps/common-app/runtime-tests/reanimated/tests/layoutAnimations/sharedTransition.test.tsxpackages/react-native-reanimated/__tests__/SharedTransition.test.tspackages/react-native-reanimated/src/layoutReanimation/SharedTransition.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| }; | ||
| }); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
I was wondering whether in docs we should also show how to type separate callbacks with the exported CustomSharedTransitionAnimation and ProgressSharedTransitionAnimation types?
Or to mention those types somewhere in API section (if we have such section in docs for SETs).
Just thinking it would make sense for cases where people don't put the callbacks inline?
|
|
||
| ```tsx | ||
| const transition = SharedTransition.custom((values) => { | ||
| 'worklet'; |
There was a problem hiding this comment.
Do you think it would be possible for us to add auto-workletization for these callbacks in a follow-up PR, with matching scoped to SharedTransition calls?
There was a problem hiding this comment.
Until then, I think in some remarks section we should explicitly state that custom and progressAnimation callbacks need the 'worklet' directive. As far as I remember we do it in all the other places in docs where the directive is needed
There was a problem hiding this comment.
I imagine we need to rework the docs anyway once all our joint efforts on finalizing all those PRs are done, not to have just the Overview section but to plan it a bit more thoroughly, so we could as well leave that remark about docs for later. Still curious about auto-workletization though
| ``` | ||
|
|
||
| On iOS, when the animation is triggered by a swipe gesture or the OS back button we instead run a progress-based transition. This way the animation responds to the user swiping their finger accordingly, and allows for transition cancellation. This behavior will be possible to customize in the future. | ||
| To define the animation yourself, use `SharedTransition.custom`. The worklet receives the values of the source view (`current*`), the values of the target view (`target*`) and the window size (`windowWidth`, `windowHeight`), and returns the animations for each property. |
There was a problem hiding this comment.
Not necessarily a change I would make, as I don't have a strong opinion, but just curious: have we considered here ever to have two separate target and current objects inside values to be able to have target.width instead of targetWidth?
| export enum SharedTransitionType { | ||
| /** | ||
| * Use the regular animation for screen transitions and the progress-based | ||
| * animation for transitions that follow the screen transition progress (iOS | ||
| * back navigation). | ||
| */ | ||
| ANIMATION = 'animation', | ||
| /** | ||
| * Use the progress-based animation for all transitions. Screen transitions | ||
| * drive the progress with the builder's animation. | ||
| */ | ||
| PROGRESS_ANIMATION = 'progressAnimation', | ||
| } |
There was a problem hiding this comment.
Maybe it is just me but I found those comments a bit hard to follow. If you agree, perhaps we could try to rephrase some of this.
For example could we clarify what controls progress in each case? I find “screen transitions drive the progress with the builder’s animation” a bit vague without giving a short example
Here is my spin on this, but it's just a draft of what kind of info I'd see here still 🤔
| export enum SharedTransitionType { | |
| /** | |
| * Use the regular animation for screen transitions and the progress-based | |
| * animation for transitions that follow the screen transition progress (iOS | |
| * back navigation). | |
| */ | |
| ANIMATION = 'animation', | |
| /** | |
| * Use the progress-based animation for all transitions. Screen transitions | |
| * drive the progress with the builder's animation. | |
| */ | |
| PROGRESS_ANIMATION = 'progressAnimation', | |
| } | |
| export enum SharedTransitionType { | |
| /** | |
| * Use the regular or custom animation, except during iOS back navigation. | |
| * During iOS back navigation, use the progress animation controlled by | |
| * the screen transition. | |
| */ | |
| ANIMATION = 'animation', | |
| /** | |
| * Always use the progress animation. | |
| * During iOS back navigation, the screen transition controls progress. | |
| * Otherwise, the builder settings, such as `.duration()` or `.springify()`, | |
| * control progress. | |
| */ | |
| PROGRESS_ANIMATION = 'progressAnimation', | |
| } |
…out animations proxy Register the progress animation from AnimatedComponent.native.tsx, run it from onTransitionProgress on the UI thread, pass its config with ProgressLayoutAnimationStart, register progress configs under the progress layout animation type, resolve transforms against the frame in SharedTransitionValuesFactory, and register the Custom Transition example. Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
- Every shared transition registers a progress animation; without a custom one
every changed property is interpolated (numbers, colors, percentages, arrays,
objects and the decomposed transform matrix).
- The default and custom animations start from the properties that differ,
pass the transform as [{ matrix }] and skip the globalOrigin and
transformMatrix aliases. A custom animation also starts the properties it
animates from the source view, and follows the builder's delay and reduce
motion.
- The progress animation registered for the native progress path processes
its colors, including the box shadow colors.
- PROGRESS_ANIMATION drives the progress animation with the builder's animation
for transitions that don't follow the screen transition progress.
- Add the static progressAnimation() and defaultTransitionType(), map a returned
transformMatrix to transform, and type every value the native side passes.
- Global origins equal the window-space origins, and per-corner and per-side
border values are passed only when one of the views sets them.
- Add a chainable SharedTransition mock.
Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
The native progress path interpolated only the frame and the border radius, so a transform of the source jumped to its full value at the start of an iOS back swipe. Every shared view now has a progress animation, so the native linear interpolation is removed and the progress animation is skipped for containers whose start is still pending or that are not progress transitions. Fixes #10876 Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
…vice Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
The type check with the legacy React Native types rejects numeric colors in a BoxShadowValue, and lint rejects the empty onStart in the reduce motion test: https://github.com/software-mansion/react-native-reanimated/actions/runs/38559289163/job/115836086761 Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
…ixUtils find-unused-code reports scaleMatrix and getRotationMatrix after the matrix interpolation moved into matrixUtils: https://github.com/software-mansion/react-native-reanimated/actions/runs/38561505177/job/115843438672 Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
At the gimbal lock the X angle was read from the first row of the rotation, which gives ±π for a rotation around Y alone, so rebuilding the matrix flipped the Y and Z axes. The X and Z rotations combine there, and the second column gives their angle. Addresses #10885 (review) Co-authored-by: Arkadiusz Kasprzyk <arkadiusz.kasprzyk@swmansion.com>
9a876d8 to
9b29581
Compare
pawicao
left a comment
pawicao
left a comment
There was a problem hiding this comment.
Three questions about transform types and interpolation.
| */ | ||
| export type CustomSharedTransitionAnimation = ( | ||
| values: SharedTransitionAnimationsValues | ||
| ) => StyleProps; |
There was a problem hiding this comment.
Could we restrict the return type for transform to the supported matrix form? It currently accepts transform: [{ scale: withTiming(2) }], but that starts without a matching initial value and produces NaN.
| decomposeMatrixIntoMatricesAndAngles( | ||
| (current as { matrix: AffineMatrixFlat }).matrix | ||
| ), | ||
| decomposeMatrixIntoMatricesAndAngles( |
There was a problem hiding this comment.
Could we handle zero-scale matrices before decomposition? With a target scale: 0, this path divides by zero and returns NaN matrix entries. I reproduced this in a Jest test of the default progress animation.
| jsi::Array SharedTransitionValuesFactory::buildMatrixArray(const ShadowView &view) { | ||
| // Resolving the operations against the frame turns percent translates into points. | ||
| const auto transform = | ||
| BaseViewProps::resolveTransform(view.layoutMetrics.frame.size, getViewProps(view).transform, TransformOrigin{}); |
There was a problem hiding this comment.
Does the matrix interpolation path preserve perspective? With a target perspective: 500, the default progress animation returns matrix entry 11 as 0 instead of -0.002, even at progress 1. Converting all transforms to matrices also sends regular perspective transforms through this path.
Summary
Brings back the Reanimated 3 API for custom shared element transitions on top of the current layout animations proxy.
SharedTransition.custom(factory)defines the animation of each property from thecurrent*andtarget*values of the two views and the window size. Custom transform animations accept a single matrix. Progress callbacks can compute individual transform operations directly.progressAnimation(worklet)computes the style for a given progress. On iOS it drives back navigation, including the swipe gesture. WithdefaultTransitionType(SharedTransitionType.PROGRESS_ANIMATION)it also drives the other transitions.globalOriginX,globalOriginYandtransformMatrix. A returnedtransformMatrixis applied as thetransform.This PR replaces #9040. Its
[SET] Custom animationscommit is followed by Arkadiusz Kasprzyk's 23 commits from kasperski95#1, rebased onmain. The commits after them complete the API, fix the progress path, and add tests, examples and docs.Fixes #10876
Test plan
fabric-exampleand go to each target screen and back. On iOS, also swipe back and cancel a swipe.yarn workspace fabric-example runtime-tests --library reanimated --platform ios --udid <udid>(and--platform android). The shared transition suite covers custom and progress transitions.yarn workspace react-native-reanimated jest SharedTransition.iOS — #10876 repro: Shift, Go, a held back swipe that is cancelled, then a completed back swipe
The Before recording is the one from #10876, made on
mainwith the same repro screen.ios-swipe-syncoff.mp4
10876-after.mp4
iOS — [SET] Custom Transition:
custom()with springs, then a back swipe along the arc ofprogressAnimation()custom-examples.mp4
Changelog
yarn changelog:addfor each changed package, or this PR does not changereact-native-reanimatedorreact-native-worklets.AI context
Investigation
Design
Scope
Verification