Skip to content

Unable to close image preview dialog on iphone #4267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cameracker opened this issue Sep 23, 2020 · 5 comments · Fixed by #4442
Closed

Unable to close image preview dialog on iphone #4267

cameracker opened this issue Sep 23, 2020 · 5 comments · Fixed by #4442

Comments

@cameracker
Copy link

On the iphone, the close button for the image preview renders behind the status bar for ios. As a result, users are unable to close the image preview dialog without killing the mobile app entirely.

@chrisbobbe
Copy link
Contributor

Hi, thanks for the report! Which iPhone and which iOS version are you on?

@cameracker
Copy link
Author

Hey, sorry. I was meaning to go back and add a screen cap and some info and got side tracked 😆 Thank you for the follow up.

I have an iPhone XR, iOS 13.7

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue 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
Copy link
Contributor

I've just sent #4268 for this.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue 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
@chrisbobbe
Copy link
Contributor

Oh—as a workaround until a fix lands, you should be able to swipe right from the extreme left edge of the screen, as a "back" gesture, to get out of the lightbox, so you don't have to quit and relaunch the app. 🙂

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 28, 2021
Including left and right safe areas, for landscape mode.

Fixes: zulip#4267
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 28, 2021
Including left and right safe areas, for landscape mode.

Fixes: zulip#4267
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jan 29, 2021
Including left and right safe areas, for landscape mode.

Fixes: zulip#4267
Gautam-Arora24 pushed a commit to Gautam-Arora24/zulip-mobile that referenced this issue Feb 2, 2021
Including left and right safe areas, for landscape mode.

Fixes: zulip#4267
Gautam-Arora24 pushed a commit to Gautam-Arora24/zulip-mobile that referenced this issue Feb 3, 2021
Including left and right safe areas, for landscape mode.

Fixes: zulip#4267
@gnprice
Copy link
Member

gnprice commented Jul 13, 2021

(For cross-reference: this was a particularly bad case of the general issue #3066.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants