-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IOS-6797] Support in-feed Placement #17
Conversation
{ | ||
MAAdapterError *adapterError = [ALVungleMediationAdapter toMaxError: error isAdPresentError: YES]; | ||
[self.parentAdapter log: @"AdView ad failed to present with error: %@", adapterError]; | ||
[self.delegate didFailToDisplayAdViewAdWithError: adapterError]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will not notify any ad presenting error to the mediation, right? or Might we move self.delegate didFailToDisplayAdViewAdWithError: adapterError];
to bannerAdDidFail:withError:
delegate method? Thanks a lot. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callback method here has been changed from bannerAdDidFailToPresent
to bannerAdWillClose
.
We don't have bannerAdDidFailToPresent
anymore with VungleBannerView
class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, it's taken care at bannerAdDidFail
callback.
MAAdapterError *adapterError = [ALVungleMediationAdapter toMaxError: error isAdPresentError: YES]; | ||
[self.parentAdapter log: @"AdView ad failed to present with error: %@", adapterError]; | ||
[self.delegate didFailToDisplayAdViewAdWithError: adapterError]; | ||
[self.parentAdapter log: @"AdView ad will close %@", bannerView.placementId]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the toMaxError line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callback method here has been changed from bannerAdDidFailToPresent
to bannerAdWillClose
.
So, no error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, it's taken care at bannerAdDidFail
callback.
{ | ||
MAAdapterError *adapterError = [ALVungleMediationAdapter toMaxError: error isAdPresentError: YES]; | ||
[self.parentAdapter log: @"AdView ad failed to present with error: %@", adapterError]; | ||
[self.delegate didFailToDisplayAdViewAdWithError: adapterError]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u need to move this logic into bannerAdDidFail: callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have below in bannerAdDidFail
.
MAAdapterError *adapterError = [ALVungleMediationAdapter toMaxError: error isAdPresentError: NO];
[self.parentAdapter log: @"AdView failed to load with error: %@", adapterError];
[self.delegate didFailToLoadAdViewAdWithError: adapterError];
Do you know if we need isAdPresentError: NO
or isAdPresentError: YES
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its based on present error or load error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
NSNumber *customWidth = [parameters.localExtraParameters al_numberForKey: @"adaptive_banner_width"] ? : 0; | ||
NSNumber *customHight = [parameters.localExtraParameters al_numberForKey: @"adaptive_banner_height"] ? : 0; | ||
|
||
if (customWidth && customWidth) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we need to add more check with isNotAdaptiveBanner here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good point. I will add the check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is suppose to be if ( isAdaptive ).
there is no custome api for MAX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we are waiting for the answer from Product team on whether we need to support "custom size with work-around api" or not (and not supporting adaptive).
Based on it, I will update this logic.
|
||
self.isAdloadSuccess = YES; | ||
NSMutableDictionary *extraInfo = [NSMutableDictionary dictionaryWithCapacity: 3]; | ||
BOOL isExtraInfo = NO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think MAX will comment on this.
so i think u can just do what google dose.
https://github.com/AppLovin/AppLovin-MAX-SDK-iOS/blob/master/Google/GoogleAdapter/ALGoogleAdViewDelegate.m#L37
Check for ALSdk.versionCode >= 6150000 for if else. Thanks @ashinagawa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
NSNumber *customWidth = [parameters.localExtraParameters al_numberForKey: @"adaptive_banner_width"] ? : 0; | ||
NSNumber *customHight = [parameters.localExtraParameters al_numberForKey: @"adaptive_banner_height"] ? : 0; | ||
|
||
if (!isAdaptiveBanner && customWidth && customWidth) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can u move the line 521 and 522 inside here. and since u ave a default value as 0 which our sdk support u dont need to check customWidth && customWidth in if block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Thanks!
Updated.
* Updated adapter to support in-feed placements
This commit will update adapter code to support in-feed ad type.
This PR is using
VungleBannerView
implementation. (https://github.com/Vungle/vng-ios-sdk/pull/706)IOS-6797