Commit fdb3e56
authored
fix: reanimated 4.3.0 support and bugfixes (#3532)
## 🎯 Goal
## Summary
This PR upgrades `examples/SampleApp` to React Native `0.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:
- message overlay / context menu flicker during layout correction
- the context menu completely falling apart on bottom sheet open
- bottom sheet hard swipe close flicker
A big part of this work was understanding the impact of
`react-native-reanimated@4.3.0`.
`4.3.0` turned 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 on `4.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 `reanimated` static feature
flags were part of the culprit. Namely, `4.3.0` enables some of the
optional feature flags to be `true` by default. This unfortunately
changes things in many ways:
- `FORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONS` was the strongest flag
level signal, as it directly messes with renders
- enabling it on the older good baseline (`4.2.3`) was enough to
reproduce the problematic behaviour
- `4.3.0` still had regressions even with
`FORCE_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
investigation
PS: The thing is breaks is opening the bottom sheet to full screen,
while it has specifically a child `FlatList` that will change its
content container size when opened to the highest snap point. This is
noticeable immediately in the `emoji` picker (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 the `FlatList`, 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 the `reanimated`
upstream 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
- Upgraded `examples/SampleApp` to React Native `0.81.6`
- Updated the aligned native/runtime dependencies needed for the RN 0.81
baseline
- Fixed the SampleApp Metro config so the app can boot correctly on the
upgraded stack
### Message overlay / context menu
- Refactored `MessageOverlayHostLayer` so overlay pieces no longer split
their vertical position across absolute `top` and animated `translateY`
- Each overlay layer now animates a single corrected Y value and applies
it through absolute positioning
- This was the real kicker on `4.3.0`, as having multiple animated
styles and expecting them to play nice with each other is out of
question
This 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.0` specifically
either.
### Bottom sheet
- Kept the sheet on the transform based motion path, but with a huge
overhaul (almost a rewrite), to facilitate animating through a logical
state machine that does not imply the need to `cancelAnimation`
- Fixed the hard swipe-to-close flicker by separating gesture
finalization from the close timing animation by one frame (even though
the shared values were kept intact, the animation would very briefly
restart on hard close and moving this orchestration to the JS stack
helps tremendously as we no longer depend layout commit topology on the
`reanimated` stack to make sure that we are doing stuff right)
- Kept keyboard offset composed into the final sheet transform instead
of letting keyboard handling fight the sheet motion path
- Fixed the bottom sheet opening for only 2-3 frames and then freezing,
often not accepting gestures either
- This was fixed by automation with the `cancelAnimation` fixes, but the
deal is that now the order of operations within `reanimated` has changed
and also having unsafe behaviour like we did really messes with the new
`SharedValue` architecture they've included in `4.3.0`
This 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/`reanimated` timing and commit behaviour.
The two concrete fixes were:
- stop splitting overlay Y position across multiple visual channels
- stop handing bottom-sheet gesture motion directly into close timing in
the same frame
The changes have been extensively tested on both `4.3.0` and
pre-`4.3.0`. This is of course with
`FORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONS` disabled, 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
<!-- Provide a description of the implementation -->
## 🎨 UI Changes
<!-- Add relevant screenshots -->
<details>
<summary>iOS</summary>
<table>
<thead>
<tr>
<td>Before</td>
<td>After</td>
</tr>
</thead>
<tbody>
<tr>
<td>
<!--<img src="" /> -->
</td>
<td>
<!--<img src="" /> -->
</td>
</tr>
</tbody>
</table>
</details>
<details>
<summary>Android</summary>
<table>
<thead>
<tr>
<td>Before</td>
<td>After</td>
</tr>
</thead>
<tbody>
<tr>
<td>
<!--<img src="" /> -->
</td>
<td>
<!--<img src="" /> -->
</td>
</tr>
</tbody>
</table>
</details>
## 🧪 Testing
<!-- Explain how this change can be tested (or why it can't be tested)
-->
## ☑️ Checklist
- [ ] I have signed the [Stream
CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform)
(required)
- [ ] PR targets the `develop` branch
- [ ] Documentation is updated
- [ ] New code is tested in main example apps, including all possible
scenarios
- [ ] SampleApp iOS and Android
- [ ] Expo iOS and Android1 parent a49d3a8 commit fdb3e56
File tree
13 files changed
+1393
-1402
lines changed- examples/SampleApp
- android
- gradle/wrapper
- ios
- SampleApp.xcodeproj
- package/src
- components
- MessageInput/__tests__/__snapshots__
- UIComponents
- contexts/overlayContext
- __tests__
13 files changed
+1393
-1402
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | | - | |
9 | | - | |
| 8 | + | |
| 9 | + | |
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
41 | 46 | | |
42 | 47 | | |
43 | 48 | | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
0 commit comments