Use 'SafeAreaView' through the app #3067
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
borisyankov
force-pushed
the
safe-area-view
branch
5 times, most recently
from
e6a75a3 to
1d0e33e
Compare
borisyankov
force-pushed
the
safe-area-view
branch
from
1d0e33e to
0c4d027
Compare
borisyankov
added
the
blocked on other work
borisyankov
force-pushed
the
safe-area-view
branch
from
0c4d027 to
9143e6d
Compare
|
Rebased and updated. |
borisyankov
force-pushed
the
safe-area-view
branch
2 times, most recently
from
51c9934 to
cac146d
Compare
borisyankov
force-pushed
the
safe-area-view
branch
2 times, most recently
from
f182c24 to
4ab0806
Compare
borisyankov
force-pushed
the
safe-area-view
branch
from
4ab0806 to
242eb6f
Compare
|
P1 because it fixes #3066 . |
gnprice
removed
the
blocked on other work
|
We're now on react-navigation v2 (see #3573), so this is unblocked! |
gnprice
force-pushed
the
safe-area-view
branch
from
242eb6f to
f038792
Compare
|
Just rebased this branch. (Across over a year's changes!) |
Makes sure our main tabs are aligned properly with the edges of the screen on iOS. More about how safe areas work on iOS: https://developer.apple.com/documentation/uikit/uiview/positioning_content_relative_to_the_safe_area About the React Native-specific component: https://facebook.github.io/react-native/docs/safeareaview
Instead of using `react-native-safe-area` and getting the inset values from the `session` state, use `SafeAreaView` witch is a component coming with React Native.
Fixes zulip#3066 Previous approach almost worked correctly, but there was one case it didn't - when the keyboard did pop on an iPhone, the bottom space was sill there, though it shouldn't, as the keyboard is located over the `Home Indicator`.
All 'screen'-type components need a `SafeAreaView` to make sure the offsets from the edges are correct. This makes sure we do that fot the `ChatScreen` component.
We no longer need `react-native-safe-area` so stop using it to track the changes to the safe area insets (we are using `SafeAreaView`)
We no longer track `safeAreaInsets` data so remove i from the reducer.
No longer used and needed. Remove completely from the codebase.
Now, completely unused, remove the 'react-native-safe-area' package.
chrisbobbe
force-pushed
the
safe-area-view
branch
from
f038792 to
96981eb
Compare
|
Just rebased across another year's changes; I came across this while looking at #4267. There are a few issues with this branch that will need to be resolved. I tested with an iPhone XR simulator.
|
|
Also, it's worth noting that React Navigation has its own docs that go through how to handle safe areas—in a way that doesn't (at least doesn't directly) use the |
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Sep 23, 2020
Even though we've been passing the `hidden` prop for the lightbox's ZulipStatusBar since the beginning (3f8ad4a), evidently it sometimes still appears. I don't have a clear answer for why, but I suspect it might have to do with a particular subtlety in react-navigation. From their docs [1]: """ If you're using a tab or drawer navigator, it's a bit more complex because all of the screens in the navigator might be rendered at once and kept rendered - that means that the last `StatusBar` config you set will be used (likely on the final tab of your tab navigator, not what the user is seeing). """ We do use a tab navigator (`MainTabs`), so this isn't implausible on its face. When the status bar appears, it's been causing zulip#4267: the close button appears behind the status bar. The `ZulipStatusBar` component has been conscripted into doing part of the work of React Native's [2], or React Navigation's [3], `SafeAreaView` component, which we haven't started using yet. (zulip#3067 is open for using it all over the app.) In particular, as long as the `hidden` prop is true, a `View` with the height of the top inset of the safe area (wrapping the status bar) prevents the rest of the screen's content from "unsafely" rendering in that area. When this screen's `ZulipStatusBar` has its `hidden` prop passed as `true`, however, it doesn't defend the safe-area view at the top, whether or not a status bar is actually showing. That's because the wrapping `View` gets its height set to zero in the `hidden` case. So, without answering why a status bar might actually be showing when we tell it not to, remove the wrapping `View`'s height difference between `hidden` being true and false, conceding that `ZulipStatusBar` should be the defender of the top safe area, and in particular that it should still do so when it's been marked as `hidden`. At least until we move on the sweeping changes of zulip#3067. Also, we've got a sliding animation for the header, and the distance it needs to travel has increased by the height of the safe area. So, account for that by adding `safeAreaInsets.top` to `NAVBAR_SIZE` in the appropriate place. (Without that addition, the header just retreats into the top inset instead of leaving the screen entirely.) [1] https://reactnavigation.org/docs/4.x/status-bar/#tabs-and-drawer [2] https://reactnative.dev/docs/0.62/safeareaview [3] https://reactnavigation.org/docs/4.x/handling-iphonex/ Fixes: zulip#4267
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Sep 23, 2020
Even though we've been passing the `hidden` prop for the lightbox's ZulipStatusBar since the beginning (3f8ad4a), evidently it sometimes still appears. I don't have a clear answer for why, but I suspect it might have to do with a particular subtlety in react-navigation. From their docs [1]: """ If you're using a tab or drawer navigator, it's a bit more complex because all of the screens in the navigator might be rendered at once and kept rendered - that means that the last `StatusBar` config you set will be used (likely on the final tab of your tab navigator, not what the user is seeing). """ We do use a tab navigator (`MainTabs`), so this isn't implausible on its face. When the status bar appears, it's been causing zulip#4267: the close button appears behind the status bar. The `ZulipStatusBar` component has been conscripted into doing part of the work of React Native's [2], or React Navigation's [3], `SafeAreaView` component, which we haven't started using yet. (zulip#3067 is open for using it all over the app.) In particular, as long as the `hidden` prop is true, a `View` with the height of the top inset of the safe area (wrapping the status bar) prevents the rest of the screen's content from "unsafely" rendering in that area. When this screen's `ZulipStatusBar` has its `hidden` prop passed as `true`, however, it doesn't defend the safe-area view at the top, whether or not a status bar is actually showing. That's because the wrapping `View` gets its height set to zero in the `hidden` case. So, without answering why a status bar might actually be showing when we tell it not to, remove the wrapping `View`'s height difference between `hidden` being true and false, conceding that `ZulipStatusBar` should be the defender of the top safe area, and in particular that it should still do so when it's been marked as `hidden`. At least until we make the sweeping changes of zulip#3067. Also, we've got a sliding animation for the header, and the distance it needs to travel has increased by the height of the safe area. So, account for that by adding `safeAreaInsets.top` to `NAVBAR_SIZE` in the appropriate place. (Without that addition, the header just retreats into the top inset instead of leaving the screen entirely.) There are no other places where we pass `hidden` as `true` for `ZulipStatusBar`, so changing its behavior in that situation shouldn't have any nasty side effects. [1] https://reactnavigation.org/docs/4.x/status-bar/#tabs-and-drawer [2] https://reactnative.dev/docs/0.62/safeareaview [3] https://reactnavigation.org/docs/4.x/handling-iphonex/ Fixes: zulip#4267
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #2702 (as it changes internally how it uses
SafeAreaView)