fix: reanimated 4.3.0 support and bugfixes#3532
Conversation
| hasCommittedVisibilityRef.current = true; | ||
| wasVisibleRef.current = visible; | ||
|
|
||
| if (!visible || wasVisible) { |
There was a problem hiding this comment.
Something this silly helps with at least 2-3 edge cases here. Interesting !
| return solvedBottomTop - bottomH.value.y; | ||
| }); | ||
|
|
||
| useAnimatedReaction( |
There was a problem hiding this comment.
Note for myself: These need to be extracted in a helper or something that will improve reusability.
SDK Size
|
| position: 'absolute', | ||
| top: topH.value.y, | ||
| top: topVisualY.value, | ||
| opacity: 1, |
There was a problem hiding this comment.
this trick is being done because we can only layout the top/bottom items absolutely once we know their layout, meaning that there's a good chance on slower devices that they'll get teleported before that and basically have no positional props (and this would cause the teleported components to be rendered for 1 frame somewhere on the screen, typically top-left as the absolute fill system doesn't know what to do with them)
There was a problem hiding this comment.
so, we essentially do this:
- render & teleport
- layout "invisibly"
- wait for
onLayoutto trigger - update the
reanimatedmutables - only then change the items to "visible"
|
|
||
| const sheetAnimatedStyle = useAnimatedStyle(() => ({ | ||
| transform: [{ translateY: translateY.value }], | ||
| paddingBottom: translateY.value, |
There was a problem hiding this comment.
with this rewrite, we no longer need to apply paddingBottom for the content to flow properly - but rather can rely purely on translations, which is a nice little perf boost
2 participants
🎯 Goal
Summary
This PR upgrades
examples/SampleAppto React Native0.81.6, fixes the Metro setup needed for that upgrade, and addresses the two main animation regressions that showed up while validating the SDK against newer React Native / Reanimated behavior:A big part of this work was understanding the impact of
react-native-reanimated@4.3.0.4.3.0turned out to be a meaningful runtime change for this codebase, not just a routine version bump. In practice it changes the behavior of some animation paths that were previously stable on4.2.3, especially around gesture driven transitions, overlay (modal specifically) post layout settle corrections and UI that depends on tight synchronization between layout and animated values.During (yet another weekend) debugging, the
reanimatedstatic feature flags were part of the culprit. Namely,4.3.0enables some of the optional feature flags to betrueby default. This unfortunately changes things in many ways:FORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONSwas the strongest flag level signal, as it directly messes with renders4.2.3) was enough to reproduce the problematic behaviour4.3.0still had regressions even withFORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONS=false, so the upgrade issue is broader than a single flag, but this flag was a major clue during the investigationPS: The thing is breaks is opening the bottom sheet to full screen, while it has specifically a child
FlatListthat will change its content container size when opened to the highest snap point. This is noticeable immediately in theemojipicker (since it has a lot of items), but it will be the case for any of the sheets with enough content. Essentially, something in the commit flow of shared values (when the feature flag is enabled) changes the way other components behave when mounting new styles (i.e components with new animated styles) and all of this happening while other animations are going on. In the case of theFlatList, it's simply virtualization kicking in and destroying other styles entirely while there is an animation going on and items are being rendered. It boggles my mind that would affect the content below it and I was not able to find the culprit (yet), but for now we'll recommend to integrators that they do not use this feature flag specifically until either I open an issue /PR in thereanimatedupstream or I find a good way to refactor our code so that this does not happen anymore. Needless to say, these issues are ridiculously difficult to debug and even more difficult to reproduce efficiently.Changes:
SampleApp upgrade
examples/SampleAppto React Native0.81.6Message overlay / context menu
MessageOverlayHostLayerso overlay pieces no longer split their vertical position across absolutetopand animatedtranslateY4.3.0, as having multiple animated styles and expecting them to play nice with each other is out of questionThis fixes the visible one frame message/context-menu flicker that could happen during open time layout corrections, especially on newer Reanimated versions. This was not an issue with
4.3.0specifically either.Bottom sheet
cancelAnimationreanimatedstack to make sure that we are doing stuff right)cancelAnimationfixes, but the deal is that now the order of operations withinreanimatedhas changed and also having unsafe behaviour like we did really messes with the newSharedValuearchitecture they've included in4.3.0This fixes the brief top-snap flash that could happen when closing the sheet with a fast downward gesture.
Most of the work here was not about changing product behaviour intentionally, but about making the SDK’s animation model more robust against newer React Native/
reanimatedtiming and commit behaviour.The two concrete fixes were:
The changes have been extensively tested on both
4.3.0and pre-4.3.0. This is of course withFORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONSdisabled, as I have no clue why it breaks stuff so much. More thorough investigation into that will definitely be done in the future.🛠 Implementation details
🎨 UI Changes
iOS
Android
🧪 Testing
☑️ Checklist
developbranch