search

zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.28k stars 643 forks source link

Safe areas are not handled correctly on iPhone X #3066

Open borisyankov opened 5 years ago

borisyankov commented 5 years ago

We are currently handling, but not 100% correctly, the 'screen border offsets' known as 'safe areas' on iOS. One such bug: iPhone X with a popped keyboard has some extra space below the compose box

gnprice commented 5 years ago

Blocked on the React Navigation v2 upgrade #2702 .

gnprice commented 5 years ago

@jackrzhang provided (on #3389) some handy screenshots that very clearly demonstrate one of the symptoms of this issue:

On any iPhone X model, the upper edge of the screen covers part of the application UI in landscape orientation:

[...]

I don't have a physical iPhone X device to use for testing, so it's also possible that this reproduces only on a simulator.

gnprice commented 5 years ago

I think #3370 is probably also a symptom, indirectly:

[From a video by @armaanahluwalia to demonstrate a different issue:]

A blank white band between the compose box and the keyboard.

Didn't repro for me on Android, and @jackrzhang reported it repro'd for him on a simulated iPhone XR but not on an iPhone 7. So that's consistent with it happening only on iOS devices that have a display cutout.

I suspect it's caused by the logic we have for attempting to avoid the cutout, i.e. by a bug in that logic.

gnprice commented 5 years ago

Additional info from the dupe issue #3064 (filed just before this one), quoting @borisyankov:

When iPhone X was released, new functionality to iOS was added to indicate safe area - to help with apps handling optional notches, rounded corners or the ...

A good article explaining all this here: https://medium.com/rosberryapps/ios-safe-area-ca10e919526f

We started using a component react-native-safe-area to handle this.

It isn't working perfectly and we need to fix it. Testing on iOS X or Simulator with it makes few layout issues obvious.

Better yet, use the now built-in component: https://facebook.github.io/react-native/docs/safeareaview

gnprice commented 5 years ago

We're now on react-navigation v2 (see #3573), so this is unblocked!

chrisbobbe commented 3 years ago

4315 would have us using a more maintained thing for getting the safe-area insets.

chrisbobbe commented 3 years ago

OK, now that #4315 is merged, we're using react-native-safe-area-context instead of react-native-safe-area to provide the safe-area insets. We've stopped storing the values in Redux; the new module uses React Context.

As discussed around here, the new module is not without flaws, though.

It offers a useSafeAreaInsets hook and a withSafeAreaInsets HOC, which will provide the insets in a live-updating way, so that the values are even correct when switching between screen orientations. There are some hiccups in that live-updating, though; see https://github.com/zulip/zulip-mobile/pull/4315#issuecomment-755836500 for some observations.

The preferred solution to try will be to try to use SafeAreaView, a React component provided by the module, instead of useSafeAreaInsets/withSafeAreaInsets. It's specifically recommended because it doesn't have any flickers when changing screen orientations.

If that doesn't work and we have to use useSafeAreaInsets/withSafeAreaInsets, we'll need to do some debugging about those flickers.

chrisbobbe commented 3 years ago

I plan to make a focused PR to fix this issue after #4468 is in.