Skip to content

Commit

Permalink
lightbox: Fix lack of access to close button on some iOS devices.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
chrisbobbe committed Sep 23, 2020
1 parent eed6d03 commit 5ba27b2
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/common/ZulipStatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ZulipStatusBar extends PureComponent<Props> {
render() {
const { theme, hidden, safeAreaInsets, orientation } = this.props;
const backgroundColor = this.props.backgroundColor ?? this.props.narrowTitleBackgroundColor;
const style = { height: hidden ? 0 : safeAreaInsets.top, backgroundColor };
const style = { height: safeAreaInsets.top, backgroundColor };
const statusBarColor = getStatusBarColor(backgroundColor, theme);
return (
orientation === 'PORTRAIT' && (
Expand Down
10 changes: 6 additions & 4 deletions src/lightbox/Lightbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import { View, Dimensions, Easing } from 'react-native';
import PhotoView from 'react-native-photo-view';
import { connectActionSheet } from '@expo/react-native-action-sheet';

import type { Auth, Dispatch, Message } from '../types';
import type { Auth, Dispatch, Message, Dimensions as ZulipDimensions } from '../types';
import { connect } from '../react-redux';
import type { ShowActionSheetWithOptions } from '../message/messageActionSheet';
import { getAuth } from '../selectors';
import { getAuth, getSession } from '../selectors';
import { getResource } from '../utils/url';
import { SlideAnimationView } from '../common';
import LightboxHeader from './LightboxHeader';
Expand Down Expand Up @@ -38,6 +38,7 @@ const styles = createStyleSheet({

type SelectorProps = $ReadOnly<{|
auth: Auth,
safeAreaInsets: ZulipDimensions,
|}>;

type Props = $ReadOnly<{|
Expand Down Expand Up @@ -96,7 +97,7 @@ class Lightbox extends PureComponent<Props, State> {
});

render() {
const { src, message, auth } = this.props;
const { src, message, auth, safeAreaInsets } = this.props;
const footerMessage =
message.type === 'stream' ? `Shared in #${message.display_recipient}` : 'Shared with you';
const resource = getResource(src, auth);
Expand All @@ -114,7 +115,7 @@ class Lightbox extends PureComponent<Props, State> {
<SlideAnimationView
property="translateY"
style={[styles.overlay, styles.header, { width }]}
from={-NAVBAR_SIZE}
from={-(NAVBAR_SIZE + safeAreaInsets.top)}
to={0}
{...this.getAnimationProps()}
>
Expand Down Expand Up @@ -142,5 +143,6 @@ class Lightbox extends PureComponent<Props, State> {
export default connectActionSheet(
connect(state => ({
auth: getAuth(state),
safeAreaInsets: getSession(state).safeAreaInsets,
}))(Lightbox),
);

0 comments on commit 5ba27b2

Please sign in to comment.