Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flush operations for events on iOS Fabric #4110

Merged
merged 2 commits into from Feb 26, 2023
Merged

Conversation

kmagiera
Copy link
Member

Summary

This PR fixes issue with UI not updating when style changes are connected to events on iOS Fabric. This problem has been introduced in #3970 where we migrated from using rAF and started using setImmediate instead. As a result we had to manually flush set-immediates queue in certain conditions. Specifically this was already happening after animations frame was run (i.e. in requestAnimationFrame callback) but should also be done directly after handling events much like we already do on Android here and on iOS-paper here.

The reason this stopped working on iOS and not on Android was that Android-fabric implementation still uses old architecture flow for handling events while iOS uses new C++ based react::EventListener API (that apparently wasn't working on Android last time we checked). This PR adds flushing operations queue in that new C++ based flow.

Note that this problem didn't occur before Shareable Rewrite (#3722) because before all mapper updates would result in us scheduling new animation frames. So the updates were happening eventually, were just delayed by one frame.

Test plan

Run Fabric example on iOS, test Article progress example.

@kmagiera kmagiera added this pull request to the merge queue Feb 26, 2023
Merged via the queue into main with commit 3025002 Feb 26, 2023
@kmagiera kmagiera deleted the fix-events-on-ios-fabric branch February 26, 2023 23:37
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

This PR fixes issue with UI not updating when style changes are
connected to events on iOS Fabric. This problem has been introduced in
software-mansion#3970 where we migrated from using `rAF` and started using
`setImmediate` instead. As a result we had to manually flush
set-immediates queue in certain conditions. Specifically this was
already happening after animations frame was run (i.e. in
`requestAnimationFrame` callback) but should also be done directly after
handling events much like we already do on Android
[here](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283)
and on iOS-paper
[here](https://github.com/software-mansion/react-native-reanimated/blob/d9a55c556fc32fcb5db59acc92cbedd6452af9dc/ios/REANodesManager.mm#L334).

The reason this stopped working on iOS and not on Android was that
Android-fabric implementation still uses old architecture flow for
handling events while iOS uses new C++ based `react::EventListener` API
(that apparently wasn't working on Android last time we checked). This
PR adds flushing operations queue in that new C++ based flow.

Note that this problem didn't occur before Shareable Rewrite (software-mansion#3722)
because before all mapper updates would result in us scheduling new
animation frames. So the updates were happening eventually, were just
delayed by one frame.

## Test plan

Run Fabric example on iOS, test Article progress example.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants