[RNMobile] Add SafeAreaView to Help & Support view

[twstokes]

Related PRs

• Wrap NavigationContainer with SafeAreaView.

Description

Related issue. When the Help view is rotated from portrait to landscape and back to portrait, it should continue to respect the device's Safe Area boundaries.

How has this been tested?

Prerequisites: iOS device with a notch

1. In the portrait orientation, open the block editor in WPiOS by creating a new post.
2. Tap the three dots at the top right.
3. Tap "Help & Support".
4. Observe that the top of the view is under the notch.
5. Rotate the device into landscape.
6. Rotate the device back to portrait.
7. ℹ️ Expected: The top of the sheet view abides by the Safe Area boundaries.

Screenshots

See important note below.

Types of changes

Bug fix.

Potential unintended areas of impact

• Android's Help & Support view

⚠️ Important note

As demonstrated in the above screenshots, the view will not match its original UI after the rotation sequence. The root problem (which affects all React Native bottom sheet views) has been identified and will be addressed later in a subsequent PR. Though this solution isn't optimal, it's limited in scope to just the Help & Support view and carries less risk.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[twstokes]

Hiding whitespace changes may make this review a little simpler.

[dcalhoun]

From testing on an iPhone 13 mini simulator, I am seeing the following. The initial render shows an expected gap above the Help screen. After rotating to landscape and then back to portrait, there is no longer a gap. The Help background then renders all the way to the top the screen behind the notch.

The content is appropriately contained, it is just the background that changes after the rotations. I'm not sure what may be causing this currently.

ios-help-safeareaview.mov

[twstokes]

From testing on an iPhone 13 mini simulator, I am seeing the following. The initial render shows an expected gap above the Help screen. After rotating to landscape and then back to portrait, there is no longer a gap. The Help background then renders all the way to the top the screen behind the notch.

The content is appropriately contained, it is just the background that changes after the rotations. I'm not sure what may be causing this currently.

ios-help-safeareaview.mov

Thanks @dcalhoun - let me see if I can resolve that. I missed it in my testing.

[twstokes]

Just an update:

I believe I've tracked this down to this method returning incorrect values at the time it fires. When the view is initially loaded in a portrait orientation, result has this value:

{"safeAreaInsets": {"bottom": 34, "left": 0, "right": 0, "top": 47}}

but when rotating back to portrait from landscape, it looks like this:

{"safeAreaInsets": {"bottom": 34, "left": 0, "right": 0, "top": 0}}

Possibly there's a race involved, because if I manually call

SafeArea.getSafeAreaInsetsForRootView().then( ( result ) => { }

at the time of an orientation change, its result value will be correct.

I've also observed that changing the app's orientation (portrait -> landscape -> portrait) before rendering the Help & Support view has the same effect.

[dcalhoun]

I believe I've tracked this down to this method returning incorrect values at the time it fires.

Possibly there's a race involved, because if I manually call

SafeArea.getSafeAreaInsetsForRootView().then( ( result ) => { }

at the time of an orientation change, its result value will be correct.

Good sleuthing! I was not aware we were using a third-party package for this logic. I presumed it was driven by React Native's own SafeAreaView. I wonder if there is a bug in our code or one in the third-party package, as this incredibly generic issue (🤦🏻 😄) implies.

In hopes of finding a way swap the third-party package with SafeAreaView, I was able to find the point we added react-native-safe-area. However, without researching further, wordpress-mobile/gutenberg-mobile#448 (review) appears to pretty strongly imply swapping is not a good approach. Not sure if we should heed that advice or research that option further. Maybe the correct solution is to not wrap the Help section in SafeAreaView and take a different approach?

I've also observed that changing the app's orientation (portrait -> landscape -> portrait) before rendering the Help & Support view has the same effect.

That is a really good find. I interpret this to mean it is not related to the Help section implementation at all. Would you agree?

[twstokes]

as this incredibly generic issue (🤦🏻 😄) implies.

Haha, I saw that one as well. I'm not sure either which side it's on.

However, without researching further, wordpress-mobile/gutenberg-mobile#448 (review) appears to pretty strongly imply swapping is not a good approach.

Thanks for surfacing this!

I see we're using React Navigation in some areas, and they appear to recommend react-native-safe-area-context, but I don't know enough about it yet to determine viability.

I interpret this to mean it is not related to the Help section implementation at all. Would you agree?

Totally agree. I think it's larger than this scope and affects every <BottomSheet>, so we can probably close this and move to a new issue / PR.

[dcalhoun]

I see we're using React Navigation in some areas, and they appear to recommend react-native-safe-area-context, but I don't know enough about it yet to determine viability.

From glancing at react-native-safe-area-context, it at least appears to be more actively maintained than react-native-safe-area.

I think it's larger than this scope and affects every <BottomSheet>, so we can probably close this and move to a new issue / PR.

Makes sense to me.

[twstokes]

Hey @dcalhoun - after mentioning this issue to @hypest, he recommended that we consider pushing forward with this solution as a stop-gap until we can slate resources to fix it properly (since it affects all bottom sheets). Though this fix isn't optimal it should ensure that users can tap the "Close" button.

How do you feel about going in that direction? Thanks!

[dcalhoun]

Hey @dcalhoun - after mentioning this issue to @hypest, he recommended that we consider pushing forward with this solution as a stop-gap until we can slate resources to fix it properly (since it affects all bottom sheets). Though this fix isn't optimal it should ensure that users can tap the "Close" button.

How do you feel about going in that direction? Thanks!

That makes sense to me. I do view this as an improvement to the current experience. 🚀

Additionally, I was actually thinking about this issue just last night. I noticed a similar behavior occurs in the host WPiOS app, where the sheet temporarily renders its background behind the status bar.

Maybe that interaction is by design, but I found it odd personally. I am unsure if we are utilizing iOS Sheets or some custom bottom sheet in WPiOS. The below behavior does not match the behavior of Sheets I see in other apps.

ios-sheet-coverage.mp4

[twstokes]

where the sheet temporarily renders its background behind the status bar.

Maybe that interaction is by design, but I found it odd personally.

Yeah that's interesting!

Thanks for your input @dcalhoun! We'll charge forward on getting this change out, and I'll also be sure to make a new issue with the findings above.

[dcalhoun]

Merging for @twstokes as he does not have the repository permissions as of writing. The code looks good and functionality works as expected. The failing e2e tests are already present in trunk.