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

Add space for non-overlapping bottom banners on iPhone X #186

Closed MBuchalik closed 6 years ago

MBuchalik commented 6 years ago

This PR adds basic support for iPhone X. Please note the following:

How did I test it?

Unfortunately, I don't have a physical iPhone X for testing. All tests were made in the iOS simulator with the test ad ID provided by Google.

Below, you can find a few screenshots. I hope this PR will be useful for you. :smiley:

iphone-x-portrait iphone-x-landscape iphone-6s-portrait

ratson commented 6 years ago

@MBuchalik I don't have an iOS device to test too. Regarding to the background color, I will just merge it util someone complain about it. Thank you for the PR.

MBuchalik commented 6 years ago

@ratson Thanks for merging the PR! Do you think that you will find some time to publish this version to npm in the next days? (I am using PGB for some projects and here you can't manually run npm install :smile:)

ratson commented 6 years ago

@MBuchalik Sure, just released a new version with your fix.

anacierdem commented 6 years ago

I prefer overlapped ads to handle this case. This allows filling the bottom part with content that is compatible with the app in the webview instead of default black.

ratson commented 6 years ago

@anacierdem Would you share what need to be done as a user? It would be great if you could send a PR with the instructions and comment out the line for changing background color.

tennist commented 5 years ago

@ratson & @anacierdem I second commenting the line that sets the background to black. This can be handled on an individual app basis outside of the plugin and not change background colors on people that have designed to the default white or use a different background color.

MBuchalik commented 5 years ago

@tennist To me, it seems like it is not possible to do so without a separate plugin. Or do you have an idea how you could select a background color without another plugin? (Please note that setting the WebView's background color doesn't help here since the ads live in a superview of it.)

anacierdem commented 5 years ago

@ratson I have already proposed my solution at #121

tennist commented 5 years ago

The solution in this post for the bottom banner issue is the best, however, when you flip from portrait to landscape it doesn’t inject the ad into the space created. It just has an empty space. Then when you flip back to portrait mode it has the ad at the bottom. I still do not like switching the background of the superview to black since my apps have map pages that allow the superview to show through and it ruins the color scheme. I agree that black looks best at the bottom however. Perhaps we can add padding to the bottom of the ad and set the ad background to black. Or create a variable for the banners that allows people to choose what the superview background color gets set to. Also, as mentioned in solution #218, I think we should get rid of the iOS 7 hack altogether. This is a hack to make the status bar plugin work and apple prefers to have the statusbar overlay the app. I will play with these items more when I get back to my computer on Monday.

tennist commented 5 years ago

@MBuchalik & @ratson & @anacierdem My latest pull request removes the black background as it caused me and others some complications in our apps. I have come up with a better solution, however, I need some help with how to code it. I am not very experienced in objective c. I agree that the easiest solution is to just overlay the webview, however, for those that prefer not to do that we should have a solution.

What needs to be done is to create an additional view similar to bannerView. We can call it safeAreaView. We will give this view default black background and hide it until needed. Then in resize views, we can turn the view on, size it to the height of the safe area, and position it appropriately. I can handle the positioning and sizing the view within the resizeViews function, but I have no idea how to go about creating the new view.

If we are real ambitious we could set it up so the user can set a background color in config.xml.

tennist commented 5 years ago

@MBuchalik I just submitted a pull request that I believe solves my issue and keep your app looking the way you want it to. We can work together to build upon my solution if you feel like it, however, my apps do not need any further enhancement.

Also, I am open to help others that might need to implement this solution into the other banner ad placement locations (top, top-overlay, or bottom-overlay).