Skip to content
This repository has been archived by the owner on May 19, 2021. It is now read-only.

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

Merged
merged 1 commit into from Mar 23, 2018

Conversation

MBuchalik
Copy link
Collaborator

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

  • Only bottom banners are affected that don't overlap the WebView.
  • I don't have an Obj C background. So: Please look at the changes carefully.
  • The idea is based on appfeel/admob-google-cordova@0bdea38
  • I added a black background for the webView's superview. Otherwise, the bottom part would be white which - in my opinion - doesn't look that great.

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. 馃槂

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

@ratson
Copy link
Owner

ratson commented Mar 23, 2018

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

@ratson ratson merged commit a4a302b into ratson:master Mar 23, 2018
@MBuchalik
Copy link
Collaborator Author

MBuchalik commented Mar 24, 2018

@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 馃槃)

@ratson
Copy link
Owner

ratson commented Mar 24, 2018

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

@anacierdem
Copy link

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
Copy link
Owner

ratson commented Mar 24, 2018

@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
Copy link
Collaborator

tennist commented May 31, 2018

@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
Copy link
Collaborator Author

@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
Copy link

@ratson
I have already proposed my solution at #121

@tennist
Copy link
Collaborator

tennist commented Jun 2, 2018

The solution in this post for the bottom banner issue is the best, however, when you flip from portrait to landscape it doesn鈥檛 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
Copy link
Collaborator

tennist commented Jun 8, 2018

@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
Copy link
Collaborator

tennist commented Jun 8, 2018

@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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants