search

ratson / cordova-plugin-admob-free

New development has been moved to "admob-plus-cordova", https://github.com/admob-plus/admob-plus/tree/master/packages/cordova
https://github.com/admob-plus/admob-plus
MIT License
499 stars 214 forks source link

Fix for iPhone X status bar improperly refreshed #218

Closed rafaellop closed 6 years ago

rafaellop commented 6 years ago

Leaving this as true for all iOS versions above 7 has no effect except on iPhone X where the settings makes the status bar refreshed and finally blacked out.

tennist commented 5 years ago

Please undo this commit. Changing the code as such breaks the app on all iOS devices running a verion of iOS greater than 11.0 except for the iPhone X. There needs to be another way to fix this for the iPhone X. Somehow it needs to detect whether or not this the phone is an iPhone X or not.

tennist commented 5 years ago

@rafaellop this commit breaks the iOS status bar handle on all other iOS devices other than the iPhone X that are running iOS 11 or higher. Lets work together to find a better way of detecting if the device is an iPhone X.

rafaellop commented 5 years ago

I disagree. I have tested and published few apps with that fix and they work properly on other iphones. The issue might be not the code but the statusbar plugin as far as I noticed. Please try to remove the statusbar plugin and rebuild you code. I have tested this on iphone 6/7 simulator and on physical iphone 7 device and have no issues at all.

On the other hand, if the code is as it was before my commit, iphones prior to X works properly even with the statusbar plugin added, but iphone x statusbar is impossible to make works OK. So, the best solution is the modification + removing cordova statusbar plugin until they fix it.

This reply also refers to the issue https://github.com/ratson/cordova-plugin-admob-free/issues/224

tennist commented 5 years ago

Many of us rely on the statusbar plugin to change its color, appearance, overlay, etc. to be consistent across android and ios. Removing this plugin requires us to hard code all of this into our apps using multiple languages. For instance, I use it to set my statusbar color and to not overlay my web views. If I remove the statusbar plugin and allow the use of your code above, the statusbar overlays the navigation bar in my app on the iPhone 8 and iphone 8+. The problem is that these two plugins are fighting each other to resize the window and the admob plugin is the last one to do a resize since it happens after the ad is loaded from admob. It undoes the positioning that the statusbar plugin did.

The solution to check for this statusbar frame size in versions of iOS7 and greater was an elegant solution. However, with the new iPhone X, it could potentially cause problems for certain apps. It would be best to come up with a solution that works for all, especially for those of us whose apps utilize the statusbar plugin.

I do know some ways to detect if the device is an iPhone X and will try to compile a solution shortly.

tennist commented 5 years ago

After playing around with these items a bit on the iPhone X simulator, I cannot get any of my apps to show issues with the original code. This explains why I have not had any complaints from my users to date. I think this is just an issue with your particular apps. Please post some screenshots of your issue so that we can see what problems you are having with the code set back to original.

rafaellop commented 5 years ago

@ratson Please revert the old code if you find that reasonable. I can live with my fork until ecordova statusbar is fixed. Eventually the abovementioned hack that would detect iphonex might be useful.

tennist commented 5 years ago

@rafaellop & @ratson I have been thinking about this more and it doesn't make sense to hack this plugin so that it will work with another plugin. Really, the iOS7 hack in this plugin should be removed and the option to not overlay the status bar in the statusbar plugin should be dperecated. Apple wants people to develop their apps so that the statusbar overlays the app and this is why they made the change in iOS7. It is really easy to add padding to the region beneath the statusbar and get the same effect. At this point I have removed the statusbar plugin from my apps and allow the statusbar to overlay the webview.

So I take back my original suggestion for reverting the code. However, I do think a change is warranted here. If everyone gets rid of the status bar option to overlay the webview and develops their app correctly, the iOS7 hack will cause problems for any device with iOS < 11.0. I suggest we remove the hack altogether. The following changes are needed:

  1. comment lines 672 and 674 (this way they are still there if anyone wants to enable them on their own)
  2. add line below line 674 that reads float top = 0.0;

People can then refer to the following article for properly handling the status bar in iOS https://blog.phonegap.com/displaying-a-phonegap-app-correctly-on-the-iphone-x-c4a85664c493

rafaellop commented 5 years ago

Please modify the code and do a pull request. @ratson will decide to merge it or not.

tennist commented 5 years ago

@rafaellop & @ratson i just created a pull request to get rid of this hack altogether. It also gets rid of the superview black background that was set in the latest releases. I think we can do something better here than a black background as discussed in issue #186.

tennist commented 5 years ago

@rafaellop & @ratson i just created a revised pull request to gets rid of this hack altogether. In addition to other items.