feat: coalesced event

[Gregoirevda]

Summary

onInsetsChange can sometimes be sent very quickly after another. Using the same technique as the ScrollView, we keep the view property onInsetsChange, but it's triggered from a custom RCTEvent with a coalescingKey.

If events are sent 5 times very quickly, only 1 event will be sent with the latest insets and frame.

Fixes #579 #485 #172

Note:
I've tested this in a real app and onInsetsChange calls goes from 3 to 1, but in the example app it doesn't change (still 3 calls when rotating) and that's because it doesn't change anything in batching which is good.
So, if during a flush to JS 5 onInsetsChange are sent only the last one will survive, but if the app isn't busy and it can send the 5 events independently it will.

EventEmitter will collect the event in a queue and then flush them. Coalesce changes a flush with multiple events from
[onInsetsChange x1 y1, onInsetsChange x2 y2] to
[onInsetsChange x2 y2]

PS:
I've only implemented the old architecte, if this can be reviewed I can then only apply changes to this part and finish the fabric arch.
Requires RCT_NEW_ARCH_ENABLED=0 pod install to test.

[jacobp100]

Thank you for this!

Interesting it didn't fully resolve the issue. But at least it's not worse

[Gregoirevda]

Added an example with only a useSafeAreaInsets() (not pushed) to double check and result are (iPad)
Before

-> to landscape
 LOG  onInsetsChange {"bottom": 20, "left": 0, "right": 0, "top": 74}

-> to portrait
LOG   onInsetsChange {"bottom": 0, "left": 0, "right": 0, "top": 74}
 LOG  onInsetsChange {"bottom": 0, "left": 0, "right": 0, "top": 0}
 LOG  onInsetsChange {"bottom": 20, "left": 0, "right": 0, "top": 74}

After

-> to landscape
 LOG  onInsetsChange {"bottom": 20, "left": 0, "right": 0, "top": 74}

-> to portrait
LOG   onInsetsChange {"bottom": 0, "left": 0, "right": 0, "top": 74}
 LOG  onInsetsChange {"bottom": 20, "left": 0, "right": 0, "top": 74}

Even though there's still a double event, all data make sense now. I guess {"bottom": 0, "left": 0, "right": 0, "top": 0} is send quicker than the 2 others.
On our project we're going from 3 events to 1.

I don't want to add unnecessary complexity to the fabric variant, but I debugged and same results, so will apply there too

[jacobp100]

Ok I think I know what's going on here. The batching is still useful to have, but I think the bug we're still having goes like this

• The orientation changes, which updates the size of the root view
• iOS performs a synchronous layout, which triggers an asynchronous RN layout pass
• Because RN's layout is async, the views momentarily have the wrong size for the screen
• iOS synchronously applies the safe area to these views based on how they're intersecting with the edges of the screen
• An event is fired with incorrect safe area insets
• RN finishes its layout
• iOS synchronously applies the safe area to these views again, and this time it's correct
• A second event is fired with correct insets

I believe the fix here is when the safe area insets change, set a dirty flag rather than immediately firing an event. Then add an event listener for layout change events, and then if the dirty flag is set, fire the event

You want to use the uiManagerDidPerformLayout selector, and link it up like in here

Don't feel you have to take this on, we'll still merge this PR because it is important. But if you do take it on, I'm happy to provide pointers

[Gregoirevda]

I'm happy to continue this, but I cannot work on it now. If this can already be shipped it would be great.

I need to add setMaintainVisibleContentPosition? Didn't get it :)

[jacobp100]

No - if you add [_eventDispatcher.bridge.uiManager.observerCoordinator addObserver:self]; and - (void) uiManagerDidPerformLayout, the latter function will get called every time RN updates the layout

[Gregoirevda]

@jacobp100 I've set a flag in invalidateSafeAreaInsets and added the observer in init and only emit the event in uiManagerDidPerformLayout and this won't work:

• Initial layout:
  uiManagerDidPerformLayout isn't called after the component mounted. I've fixed this by emitting in the invalidateSafeAreaInsets when _initialInsetsSent hasn't been set.

• portrait to landscape event order:

uiManagerWillPerform
uiManagerDidPerform
invalidateSafeAreaInsets

=> no events sent

• landscape to portait event order

invalidateSafeAreaInsets
uiManagerWillPerform
uiManagerDidPerform
uiManagerWillPerform
uiManagerDidPerform
..... many times
invalidateSafeAreaInsets (stops here)

=> I can toggle the flag once the event is emitted in uiManagerDidPerform, but the last event wont be sent

[janicduplessis]

Thanks for working on this!

Are we good merging this as is? We can look at the other issue later, and might also want to implement in for new arch.

Can you just run the formatting script again?

[Gregoirevda]

@janicduplessis updated with clang format