googleads-mobile-flutter icon indicating copy to clipboard operation
googleads-mobile-flutter copied to clipboard

Serve multiple ad types with a single AdLoaderAd widget

Open msbit opened this issue 2 years ago • 6 comments
trafficstars

Description

The motivation behind this PR is to allow more elaborate configurations such as the one described here, where a single AdLoader can serve either a NativeAd or an AdManagerAdView.

To this end, this PR adds a new widget, AdLoaderAd, which can configured to serve any of the following platform ad types, all provided by AdLoader/GADAdLoader:

  • NativeAd/GADNativeAd
  • AdManagerAdView/GAMBannerView
  • NativeCustomFormatAd/GADCustomNativeAd

For the most part the widget and its supporting classes operated independently of the existing widgets. A notable exception is that for the NativeAd/GADNativeAd and GAMBannerView components, the existing supporting classes behind the relevant widgets are used. To allow this, internal APIs are modified as part of:

  • Android: Make FlutterNativeAdLoadedListener more generally usable
  • iOS: Relax constraints on FLTAdInstanceManager methods

to allow more general types to be passed to the existing classes. These changes do not cause any incompatibility with existing internal code.

To facilitate testing of the specific ad type under Android, a testing-only constructor has been added as part of:

  • Android: Allow passing a FlutterAdLoader factory

This is applicable to the FlutterAdLoaderAd type but could also be used by other types that expect a FlutterAdLoader during building and construction.

Related Issues

N/A

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • [x] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • [x] All existing and new tests are passing.
  • [x] I updated/added relevant documentation (doc comments with ///).
  • [x] The analyzer (flutter analyze) does not report any problems on my PR.
  • [x] I read and followed the [Flutter Style Guide].
  • [x] I signed the [CLA].
  • [x] I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • [ ] Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • [x] No, this is not a breaking change.

msbit avatar Dec 12 '22 23:12 msbit

Happy New Year everyone!

Is there anything else you would like to see before considering this PR?

✌🏻

msbit avatar Jan 02 '23 07:01 msbit

I'm following up on this PR, and I've noticed that (unlike https://github.com/googleads/googleads-mobile-flutter/pull/749) this wasn't assigned any reviewers automatically, so checking in with all those listed in CODEOWNERS for packages/google_mobile_ads/*:

@blasten @bparrishMines @jjliu15 @luo86 @srichakradhar

Is there anything further you'd like or need from me when considering this PR?

msbit avatar Jan 20 '23 23:01 msbit

I need this 🙏

joelbrostrom avatar Jan 30 '23 15:01 joelbrostrom

I'm trying to use this fork but having a problem with varying ad sizes. If setting a fixed size for all ads it works well. But if we have different sizes for e.g banners(https://developers.google.com/admob/flutter/banner/inline-adaptive) / native ads - how to set correct size of the Container of the AdWidget? Thanks

martinsellergren avatar Mar 15 '23 13:03 martinsellergren

@srichakradhar and @jjliu15 we have been testing this fork for a longer period and it helps us increasing the monitisation of these ad positions. Is there anything which holds back merging this PR? We're happy to assist with anything which is needed for that.

lazytesting avatar Sep 20 '23 06:09 lazytesting

Hey, any updates on the status of this PR? @srichakradhar @jjliu15 @luo86 @bparrishMines @LTPhantom

oelburk avatar Jan 03 '24 13:01 oelburk