Skip to content
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

Issue with status bar on iPhone X when toolbar is at the top (v3.0.0) #301

Closed
russcarver opened this issue Sep 22, 2018 · 30 comments · Fixed by #656
Closed

Issue with status bar on iPhone X when toolbar is at the top (v3.0.0) #301

russcarver opened this issue Sep 22, 2018 · 30 comments · Fixed by #656

Comments

@russcarver
Copy link

russcarver commented Sep 22, 2018

There is an issue with the status bar and the iPhone X as described at https://stackoverflow.com/questions/47399938/issue-with-status-bar-using-cordova-inappbrowser-and-ios-11-and-iphone-x

When the inappbrowser toolbar is at the top of the screen, the status bar on the iPhone X is half white and half gray.

Changing the following line in CDVInAppBrowser.m solves the problem and makes the whole status bar gray. Change line:
statusBarFrame.size.height = STATUSBAR_HEIGHT;
to
statusBarFrame.size.height = 0;

This does not seem to negatively affect other iPhones or iPads.

@project-bot project-bot bot added this to new/unassigned/reopened issues in Apache Cordova: project-bot automation testing Sep 22, 2018
@russcarver
Copy link
Author

screen shot 2018-09-21 at 8 12 49 pm

@russcarver
Copy link
Author

screen shot 2018-09-21 at 7 39 01 pm

@project-bot project-bot bot moved this from new/unassigned/reopened issues to edited issue in Apache Cordova: project-bot automation testing Sep 22, 2018
@janpio janpio added the bug label Sep 22, 2018
@janpio janpio moved this from edited issue to new/unassigned/reopened issues in Apache Cordova: project-bot automation testing Sep 24, 2018
@vespino
Copy link

vespino commented Oct 22, 2018

@russcarver is this something I can fix when using Phonegap Build? Or is this an update waiting to happen?

@russcarver
Copy link
Author

@vespino I'm waiting for them to fix this. In the mean time, I've just forked it and made the changes shown above.

@vespino
Copy link

vespino commented Oct 23, 2018

@russcarver can I use your fork?

@russcarver
Copy link
Author

@vespino I don't have it anymore as I decided to go a different route, but if you fork it yourself and make the change outlined above, it will work fine. You can install plugins from local directories using the "cordova plugin add" command.

@vespino
Copy link

vespino commented Oct 23, 2018

@russcarver then I was just in time :)

@vespino
Copy link

vespino commented Oct 23, 2018

@russcarver do you mind sharing the different route btw?

And do you mind pointing me to where the changes have to be made? I can't seem to find "statusBarFrame.size.height = STATUSBAR_HEIGHT;" in this file: https://github.com/vespino/cordova-plugin-inappbrowser/blob/master/src/ios/CDVInAppBrowser.m

@vespino
Copy link

vespino commented Oct 23, 2018

@russcarver
Copy link
Author

Mine was in the other file, but it could be outdated by now. You can also just change the #define of the constant since it looks used in only that one place.

@vespino
Copy link

vespino commented Oct 23, 2018

@russcarver seems like the change I made indeed solves the statusbar issue, but raises another one. When opening a link in the inappbrowser and closing it, the app freezes on iOS. You would think the guys at phonegap would have addressed this issue more than a year after introduction of the iPhone X.

@russcarver
Copy link
Author

@vespino Sorry to hear that. That didn't happen to me in my tests, but I didn't test it out on iOS 12. Any details about your environment shared here may help the code owner debug.

@vespino
Copy link

vespino commented Oct 25, 2018

@russcarver I'm using this fork of themeable browser now: https://github.com/toontoet/cordova-plugin-themeablebrowser.git

@mtlehtin
Copy link

mtlehtin commented Jan 2, 2019

It seems to fix also the issue if you just remove the setting of the height

Remove this line from CDVInAppBrowserNavigationController.m:
statusBarFrame.size.height = STATUSBAR_HEIGHT;

Btw Statusbar height should be 44pt. I don't understand that why the statusbar height is hardcoded anyway.
#define STATUSBAR_HEIGHT 20.0

@mtlehtin
Copy link

mtlehtin commented Jan 3, 2019

Besides this fix just pulls the transparent grayish backdrop/overlay to top of the viewport and not the inappbrowser-view.

[EDIT]:
What is the grayish overlay anyway? Why is it needed? Shouldn't it just show the statusbar with defined color and not overlay with it that half transparent gray overlay?

@mtlehtin
Copy link

mtlehtin commented Jan 8, 2019

In addition to setting statusBarFrame.size.height = 0; i also ended up doing something like following that does not entirely rely on the hardcoded STATUSBAR_HEIGHT. Following code adds top and bottom safeareas for the view (Since we have no control for 3rd party app that we are using inside IAB and it is easier to handle in the InAppBrowser).

CDVWKInAppBrowser.m:

- (BOOL)hasTopNotch {
    if (@available(iOS 11.0, *)) {
        return [[[UIApplication sharedApplication] delegate] window].safeAreaInsets.top > 20.0;
    }

    return  NO;
}

- (void)viewWillAppear:(BOOL)animated
{
    if (IsAtLeastiOSVersion(@"7.0") && !viewRenderedAtLeastOnce) {
        viewRenderedAtLeastOnce = TRUE;
        CGRect viewBounds = [self.webView bounds];
        
        if ([self hasTopNotch]) {
            BOOL toolbarVisible = !self.toolbar.hidden;
            BOOL toolbarIsAtBottom = ![_browserOptions.toolbarposition isEqualToString:kInAppBrowserToolbarBarPositionTop];

            float topSafeArea = [[[UIApplication sharedApplication] delegate] window].safeAreaInsets.top;
            float bottomSafeArea = [[[UIApplication sharedApplication] delegate] window].safeAreaInsets.bottom;

            if (toolbarVisible && toolbarIsAtBottom) {
                bottomSafeArea = 0.0;
            }

            viewBounds.origin.y = topSafeArea;
            viewBounds.size.height = viewBounds.size.height - (topSafeArea + bottomSafeArea);
        } else {
            viewBounds.origin.y = STATUSBAR_HEIGHT;
            viewBounds.size.height = viewBounds.size.height - STATUSBAR_HEIGHT;
        }
        self.webView.frame = viewBounds;
        [[UIApplication sharedApplication] setStatusBarStyle:[self preferredStatusBarStyle]];
    }
    [self rePositionViews];
    
    [super viewWillAppear:animated];
}

The logic for hasTopNotch is from here: https://stackoverflow.com/questions/46192280/detect-if-the-device-is-iphone-x.

In addition to that, I removed they gray-background: self.view.backgroundColor = [UIColor grayColor]; and replaced it with configurable option for background color and UIStatusBarStyleLightContent or UIStatusBarStyleDefault. This way the view is seamless since the background matches statusbar and bottom safearea background colors.

@NiklasMerz
Copy link
Member

@mtlehtin You could probably create a pull request with this fix.

@KaiCMueller
Copy link

@mtlehtin I would also appreciate a pull request for your solution

@GreenSpecialist
Copy link

inappbrowser-wkwebview is deprecated and it is merged with inappbrowser. There is no CDVWKInAppBrowser.m file anymore and if you implement @mtlehtin change in the CDVInAppBrowser.m it doesnt fix the issue... Any help?

usernuno added a commit to OutSystems/cordova-plugin-inappbrowser that referenced this issue Sep 6, 2019
Applied fix from apache#301 (comment).

This did not fix the background color of the top
portion of the screen. Added additional "fixes"
for that.

References https://outsystemsrd.atlassian.net/browse/RNMT-3220
@Kevin-Hamilton
Copy link

Kevin-Hamilton commented Sep 12, 2019

This seemed to fix the issue for me:

Kevin-Hamilton@dd31f32

Credit to "Rouven Retzlaff (BERIS)" who had a fix for CDVWKInAppBrowser.m, but I needed it for CDVUIInAppBrowser.m so I ported it over.

@NiklasMerz
Copy link
Member

We solved this that way: GEDYSIntraWare@0c75a93

Should I create a PR?

@mosabab
Copy link
Contributor

mosabab commented Oct 5, 2019

Hello @NiklasMerz

If you test your solution and work perfect on iphone x and previous version of iphone, you can create a PR for this solution, because this issue still not solved for iphone x family devices.

Regards

@NiklasMerz
Copy link
Member

Thanks @mosababubakr for testing. PR created. Thanks @mtlehtin for your code, I adapted if a little bit.

Looking forward for reviews.

@NiklasMerz
Copy link
Member

@mosababubakr Yes I already did that, but unfortunately I cannot add reviewers to PRs. But please be patient, Cordova maintainers do their work in free time. Dave has done a lot for this plugin recently.

@mosabab
Copy link
Contributor

mosabab commented Oct 6, 2019

@NiklasMerz yes, you are correct, and we are appreciate that, thank you

Do you test this solution in landscape mode? (Does this solution also fix notch in landscape mode)?

Regards

@NiklasMerz
Copy link
Member

I did a quick test again with the test app and it looks good to me. Please test this yourself with your app. You may need to adjust your HTML and CSS to make it look good. There should be plenty Google results for that.

@sardapv
Copy link

sardapv commented Feb 19, 2020

This doesn't fix this is in landscape mode. even though I have viewport-fit=cover with padding of safe-area-inset.. it doesn't work when u change orientation of inappbrowser. however if opened in landscaped mode works fine then rotate to portrait..doesn't! and vice versa. I am not sure how to set height and width of statusBarFrame detecting orientation change..there is transparent area (where OP has grey area) my notification bar of webpage sticks below notch but page content is displayed through transaparent area while scrolling! really bad User experience :( . I don't have toolbar, its a full screen InAppbrowser

expcapitaldev pushed a commit to expcapitaldev/cordova-plugin-themeablebrowser that referenced this issue Apr 16, 2020
expcapitaldev pushed a commit to expcapitaldev/cordova-plugin-themeablebrowser that referenced this issue Apr 16, 2020
@project-bot project-bot bot moved this from new/unassigned/reopened issues to closed issues in Apache Cordova: project-bot automation testing Jun 4, 2020
@kartiksolanki
Copy link

@mtlehtin : can you please elaborate on how did you configured option for background color and UIStatusBarStyleLightContent or UIStatusBarStyleDefault? i was able to fix the height issue with what you have suggested but i am not able to change the gray background color?

@mtlehtin
Copy link

Hi, sorry not for answering for these. I've not been involved in the project anymore.

But as for the background-color these are the code changes that i made in our internal fork:

CDVInAppBrowserOptions.h: Expose the options:

@property (nonatomic, assign) BOOL statusbarlightcontent;
@property (nonatomic, copy) NSString* backgroundcolor;

CDVWKInAppBrowser.m:

- (void)createViews
{
  // ...

  // Set background color if user sets it in options
  if (_browserOptions.backgroundcolor != nil) {
    self.view.backgroundColor = [self colorFromHexString:_browserOptions.backgroundcolor];
  } else {
    self.view.backgroundColor = [UIColor whiteColor];
  }

  [self.view addSubview:self.toolbar];
  [self.view addSubview:self.addressLabel];
  [self.view addSubview:self.spinner];
}

- (UIStatusBarStyle)preferredStatusBarStyle
{
    return _browserOptions.statusbarlightcontent ? UIStatusBarStyleLightContent : UIStatusBarStyleDefault;
}

Usage in the app:

window.cordova.InAppBrowser.open(url, target, 'backgroundcolor=#000000,statusbarlightcontent=yes|no')

@kartiksolanki
Copy link

@mtlehtin : Thanks for the reply. I did those changes and well the status bar does appear but what happens is I have my toolbar on bottom, and the status bar with set color appears in the bottom on top of bottom toolbar instead of status bar appearing on top at its default place. Any idea what is wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects