th3rdwave / react-native-safe-area-context

A flexible way to handle safe area insets in JS. Also works on Android and Web!
MIT License
2.08k stars 191 forks source link

insets.top switching to 0 during iPad orientation change #485

Open Gregoirevda opened 3 months ago

Gregoirevda commented 3 months ago

On iPad, insets.top changes during an orientation change from X to 0 and back to X, causing the app to re-render and recalculate layouts back and forth.

This only happens when going from landscape to portrait. I've opened a related issue on RN-navigation: https://github.com/react-navigation/react-navigation/issues/11915

EDIT: insets.top changes from landscape to portrait to 0 insets.bottom changes from portrait to landscape to 0

jacobp100 commented 3 months ago

Does it happen with just this library - or is it only with react navigation?

Gregoirevda commented 3 months ago

@jacobp100 I've tried and happens just with react-native-safe-area-context

import { SafeAreaProvider, useSafeAreaInsets } from 'react-native-safe-area-context';

export default function App() {
  return (
    <SafeAreaProvider>
      <Kid />
    </SafeAreaProvider>
  );
}

function Kid() {
  const insets = useSafeAreaInsets();
  console.log({ insets });

  return null;
}
// Opening the app on iPad in portrait mode
[2]  LOG  {"insets": {"bottom": 20, "left": 0, "right": 0, "top": 74}}

// Changing orientation to landscape (switches to 0 and back to 20)
[2]  LOG  {"insets": {"bottom": 0, "left": 0, "right": 0, "top": 74}}
[2]  LOG  {"insets": {"bottom": 20, "left": 0, "right": 0, "top": 74}}
// End orientation change

// Changing orientation back to portrait
[2]  LOG  {"insets": {"bottom": 0, "left": 0, "right": 0, "top": 74}}
[2]  LOG  {"insets": {"bottom": 0, "left": 0, "right": 0, "top": 0}}
[2]  LOG  {"insets": {"bottom": 0, "left": 0, "right": 0, "top": 74}}
[2]  LOG  {"insets": {"bottom": 20, "left": 0, "right": 0, "top": 74}}
// End orientation change

As all iOS apps with a blurred header put <ScrollView contentContainerStyle={{paddingTop: insets.top}} and all apps having a blurred bottom tab navigation have <ScrollView contentContainerStyle={{paddingBottom: insets.bottom}} This causes the ScrollView to lag a lot when changing orientation

jacobp100 commented 3 months ago

For the lag, try setting contentInset and scrollIndicatorInsets rather than padding - you'll be able to skip a layout update. You could also put a SafeAreaView with the insets inside the scrollview, as that component can shortcut a few processes, so can do the updates faster than your JS implementation can.

I'm not too sure why you get those insets - but presumably, iOS is giving them. If you're comfortable with Xcode, it would useful to know if those are coming from UIKit, and how far apart they are. I know react native can coalesce some events that are really close (like scroll events) - it's opt-in, but it might be a good workaround

Gregoirevda commented 3 months ago

I've replaced

- (void)safeAreaInsetsDidChange
{
  [self invalidateSafeAreaInsets];
}

by

- (void) viewSafeAreaInsetsDidChange
{
    [self invalidateSafeAreaInsets];
}

in RNCSafeAeraProvider.mm

and that does the trick of not switching to 0 during orientation change.

I've also debugged the app more, and it turns out I have a useEffect, depending on a useCallback, depending on useBottomTabBarOffset (react-navigation-bottom-tabs) depending on useSafeAreaInsets

// useBottomTabBarOffset in RN-bottom-tabs
const getPaddingBottom = (insets: EdgeInsets) =>
  Math.max(insets.bottom - Platform.select({ ios: 4, default: 0 }), 0);
Gregoirevda commented 3 months ago

lol viewSafeAreaInsetsDidChange is never called so indeed it solves the problem, but insets aren't sent anymore.

this solves the problem

  if (safeAreaInsets.top < 1.0 || safeAreaInsets.bottom < 1.0 || ( _initialInsetsSent &&
      UIEdgeInsetsEqualToEdgeInsetsWithThreshold(safeAreaInsets, _currentSafeAreaInsets, 1.0 / RCTScreenScale()) &&
      CGRectEqualToRect(frame, _currentFrame))) {
    return;
  }

Maybe this should be improved

  // This gets called before the view size is set by react-native so
  // make sure to wait so we don't set wrong insets to JS.
  if (CGSizeEqualToSize(self.frame.size, CGSizeZero)) {
    return;
  }
jacobp100 commented 3 months ago

You want to look in RNCSafeAreaProvider - that receives the insets from iOS, and passes them down to all the safe area views - see this issue for more info https://github.com/th3rdwave/react-native-safe-area-context/issues/92

I think the check proposed isn't safe, because the insets can be zero in some circumstances. It would be useful to get information about the timing between the two events. Are they almost instantly after each other?

Gregoirevda commented 3 months ago

@jacobp100 code I was referring to is indeed in RNCSafeAreaProvider. Yes, the events are coming instantly one after another during orientation transition

jacobp100 commented 3 months ago
  • (void) viewSafeAreaInsetsDidChange

Is that called? That's a UIViewContoller method - and this is a UIView

Gregoirevda commented 3 months ago

lol viewSafeAreaInsetsDidChange is never called so indeed it solves the problem, but insets aren't sent anymore.

UIViewContoller indeed

Gregoirevda commented 3 months ago

@jacobp100 What would be a good way to batch onInsetsChange?

jacobp100 commented 3 months ago

Look in the main source:-

https://github.com/facebook/react-native/blob/main/packages/react-native/React/Views/ScrollView/RCTScrollEvent.m#L81

https://github.com/facebook/react-native/blob/03c75c22fa6863f347f88b152a20340237f0afd6/packages/react-native/React/Views/ScrollView/RCTScrollView.m#L1058-L1077