apache / cordova-plugin-inappbrowser

Apache Cordova InAppBrowser Plugin
https://cordova.apache.org/
Apache License 2.0
1.11k stars 2.14k forks source link

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

Closed russcarver closed 3 years ago

russcarver commented 5 years ago

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.

russcarver commented 5 years ago
screen shot 2018-09-21 at 8 12 49 pm
russcarver commented 5 years ago
screen shot 2018-09-21 at 7 39 01 pm
vespino commented 5 years ago

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

russcarver commented 5 years ago

@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 commented 5 years ago

@russcarver can I use your fork?

russcarver commented 5 years ago

@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 commented 5 years ago

@russcarver then I was just in time :)

vespino commented 5 years ago

@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 commented 5 years ago

@russcarver think I have found it in this file: https://github.com/vespino/cordova-plugin-inappbrowser/blob/master/src/ios/CDVInAppBrowserNavigationController.m

russcarver commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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

mtlehtin commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

KaiCMueller commented 5 years ago

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

GreenSpecialist commented 5 years ago

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?

Kevin-Hamilton commented 4 years ago

This seemed to fix the issue for me:

https://github.com/Kevin-Hamilton/cordova-plugin-inappbrowser/commit/dd31f321cc5b0e6d2b7b5984da05e11d166fc372

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 commented 4 years ago

We solved this that way: https://github.com/GEDYSIntraWare/cordova-plugin-inappbrowser/commit/0c75a939327bd2598f6dc85644df60f9835e5ac3

Should I create a PR?

mosabab commented 4 years ago

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 commented 4 years ago

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

Looking forward for reviews.

NiklasMerz commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

kartiksolanki commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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?