capacitor-community / admob

Community plugin for using Google AdMob
MIT License
205 stars 66 forks source link

Wrong Height gets returned from Top BannerAdPluginEvents.SizeChanged #304

Open folsze opened 6 months ago

folsze commented 6 months ago

TLDR:

NOTE: the height returned by BannerAdPluginEvents.SizeChanged is not actually the height of the ad only, but it also includes any ion-safe-area-bottom, i.e. the margin of the ad to the edge of the screen, if it has any. This is not stated in the documentation of BannerAdPluginEvents.SizeChanged but it is the case.

Correct behaviour when banner at the bottom:

When the ad banner is at the bottom, then the height of the banner returned by BannerAdPluginEvents.SizeChanged is the height of the ad plus var(--ion-safe-area-bottom) (or whatever way you calculate this safe area, if not using ionic)

Wrong behaviour when banner at the top:

When the ad banner is at the top, then the height of the banner returned by BannerAdPluginEvents.SizeChanged is NOT the height of the ad plus var(--ion-safe-area-top) (although it should be!). It is slightly smaller than that. This causes layout issues.

Long description:

7A0E9B85-9448-447B-9AD0-C13F7D2E4323_1_102_o as you can see on the picture, I was taking a screenshot on macos, and on the bottom right corner of the ad you can see some dimensions, specifially 374 x 63

obviously those are the correct dimensions of the admob ad: 393 x 60

however when you look at the background of my image, when you look into the XCode console, the returned dimensions of BannerAdPluginEvents.SizeChanged turn out to be 393 x 95

somehow a wrong height is returned. Why?

first I thought his might be due to ion-safe-area-top, but it is 47px so this does not quite add up either

are you guys aware of any bugs currently with the height implementation? How could it be that 95px is returned when looking at the image the height is clearly something around 60px ?

This messes up my layout.

Especially on other modes/devices (potrait/landscape or ipads) this error in height is there to, but on a different scale. Somehow the height returned from BannerAdPluginEvents.SizeChanged is never correct

folsze commented 6 months ago

do note that the layout looks fine as is on the image, but that is only because I hardcoded some values.

folsze commented 6 months ago

Another example that shows this strange behaviour:

image

You can see the log: ⚡️ TO JS {"height":95,"width":390}

However you can also see the log, that happens after I print the info of the "GADBannerView" in the console:

warning: Module "/Users/felixolszewski/Library/Developer/Xcode/iOS DeviceSupport/iPhone14,2 17.1.2 (21B101)/Symbols/usr/lib/system/libsystem_kernel.dylib" uses triple "arm64e-apple-ios17.1.0", which is not compatible with the target triple "arm64-apple-ios13.0.0". Enabling per-module Swift scratch context. Printing description of $13: <WKCompositingView: 0x137d24270; frame = (0 0; 390 733); clipsToBounds = YES; layer = <WKCompositingLayer: 0x2838013a0> layerID = 37 "child clipping"> layerID = 37 "child clipping" Printing description of $14: <GADBannerView: 0x139206a90; frame = (0 47; 390 61); clipsToBounds = YES; tag = 2743243288699; backgroundColor = UIExtendedGrayColorSpace 0 0; layer = <CALayer: 0x28387eaa0>>

The dimensions are: (0 47; 390 61)

so the height is 61px and not 95px. Why does the BannerAdPluginEvents.SizeChanged return the wrong value?

(NOTE: the 47 stands of the top margin, it is the --ion-safe-area-top: 47px in this case

folsze commented 6 months ago

I tried with banner placement at the bottom and it works. I am now not adding the ion-safe-area anymore to the margin for the router outlet. For the top banner I had to do this. But then the toolbar had padding-top: var--(ion-safe-area-top). So there would be too much margin.

TLDR: When using an ion-header with a toolbar, with ads at the top of the screen, then this plugin does not work. I did not 100% figure it out yet, but I think the height of the ad if it is a banner on top is simply not returned correctly for devices which have some kind of --ion-safe-area-top (which they have not only when using ionic, I am talking about this area in general).

For banner on the bottom and --ion-safe-area-bottom the plugin calculates the height of the ad correctly, it calculates: banner-height + ion-safe-area-bottom

which is not actually the height of the ad, but the height of the ad-area measured from the bottom of the screen. If you guys accept such PRs, then I could add this to the documentation of BannerAdPluginEvents.SizeChanged.

nshejteri commented 5 months ago

I have the same problem when adaptive banner is located at the top (only iOS). Ad height is returned with included safeAreaBottom which should not be in case when ad is at the top of the screen.

https://github.com/capacitor-community/admob/blob/aeb4e17ab7a28f29a6c9f1f4a70494c0d6bf44cd/ios/Plugin/Banner/BannerExecutor.swift#L159

https://github.com/capacitor-community/admob/blob/aeb4e17ab7a28f29a6c9f1f4a70494c0d6bf44cd/ios/Plugin/Banner/BannerExecutor.swift#L160

mouadhbb commented 5 months ago

Hello @nshejteri @folsze https://github.com/capacitor-community/admob/pull/249#issuecomment-1900954136

https://github.com/capacitor-community/admob/pull/308