larsacus / LARSAdController

Lightweight ad mediation for iOS to properly manage multiple ad networks dynamically including iAd and Google ads.
http://theonlylars.com/blog/2013/01/10/stupid-easy-ads-with-larsadcontroller-3-dot-0/
MIT License
269 stars 60 forks source link

adBanner out of screen when coming back from another viewController in landscape #19

Closed nicocccc closed 11 years ago

nicocccc commented 11 years ago

Hi,

Thanks for all the good work. I had the following bug with iPad and landscape orientation.

My app consist of a main viewController with an adBanner and a settings viewController with no ad banner. iPad is in landscape orientation. I put addAdContainerToView in main viewController's viewWillAppear as you said. On first launched, the adbanner is loaded and everything is ok.

Then when the settings viewController is launched, I hide the adBanner. When the settings viewController is dismissed, I unhide the adBanner. My main viewController viewWillAppear is fired and the bug starts there. In LARSADController, the pb is in containerFrameForInterfaceOrientation: orientation is UIDeviceOrientationLandscape but the size returned by self.parentView.frame is portrait, and so the adBanner goes off screen.

If I shake a little the iPad, layoutBannerViewsForCurrentOrientation is fired again and the ad banner comes back.

So I added the following quick fix in containerFrameForInterfaceOrientation to switch the values if necessary: if (yOrigin > width) { CGFloat newWidth = yOrigin; yOrigin = width; width = newWidth; }

and now, everythings works fine :)

larsacus commented 11 years ago

Hmm...

Try integrating this branch and let me know if the issue still persists. I've pretty much stopped development on the master branch in anticipation of the branch I sent you in the link releasing soon. It's much more flexible and much easier to maintain, not to mention supports ARC.

Let me know if the issue persists on that branch if you get it integrated, but keep in mind that it is still in development, so I wouldn't recommend shipping that branch's code to the app store just yet.

Updated instructions are in the readme.

nicocccc commented 11 years ago

ok. So I tried veeeeery quickly your new branch and I had some problems. I'm not sure if I did everything right, so I'll look further a bit later.

But here are the problems I've noticed so far:

Here is a screenshot: http://bit.ly/Z7yBhX

I'll look a bit deeper and open an issue on the new branch.

larsacus commented 11 years ago

Sounds like you are not adding an ad banner somewhere. In any view controller you would like to display an ad, you need to be calling addAdContainerToView:inViewController: (or whatever that call is) in viewWillAppear: or viewDidAppear: (whichever makes more sense for your application). Also make sure that viewWillAppear: is actually getting called when that view controller displays the second time. Sometimes view controller life cycle methods don't get called due to improper view controller setup or something else. Put a log statement or a breakpoint on the addAdContainermethod in order to confirm that it's being called.

In regards to those bugs, I fixed those bugs last night. I was seeing them as well. The latest code should not exhibit those issues. Let me know if any of this helps!

nicocccc commented 11 years ago

Everything is ok now.

Your latest commit fixed the "iAd and adMob at the same time" bug. And I moved addAdContainerToView:inViewController: to viewDidAppear: and it fixed the first "banner out of screen in landscape" bug.

Thanks a lot.

larsacus commented 11 years ago

I appreciate the time to look at this. I'll let you know when the new branch is completed!

nicocccc commented 11 years ago

Thank you :) One last question : admob banners displayed are not the same between the new branch and the old one. Why?

larsacus commented 11 years ago

Not the same as in how?

nicocccc commented 11 years ago

They don't have the same size. With the old version, they were only 1024x66 an 768x66 (same size as iAd). With new branch, they are 1024x90 and 768x90 (conforms to admob smart banners).

larsacus commented 11 years ago

I am in fact using the admob smart banners with the new branch. So you're saying you're expecting the Admob ads to fill the whole area, just like the iAds do? I really think it just depends on the ad inventory that comes back and what size is returned. Although I'm not sure.

nicocccc commented 11 years ago

My problem is more in the height than in the width of the admob banners. I'll have to tweak a little the UI to conform with a 90px height banner. But I'm fine with that. The benefits of smart banners worth it.

larsacus commented 11 years ago

What would make that easier to deal with?

nicocccc commented 11 years ago

I don't know yet... But nothing on your part. I'll have to change my main viewController. Thanks for asking.

larsacus commented 11 years ago

File a new issue if you find I need to expose another value. Because the way I have it now, the container that all of the ads sit in is a static height, but the ads inside all could have different heights, therefore there could be a small gap between the top of an ad and the top of the container, due to the difference in all of the ads.