Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Serve multiple ad types with a single AdLoaderAd widget #723

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Serve multiple ad types with a single AdLoaderAd widget #723

wants to merge 7 commits into from

Conversation

msbit
Copy link
Contributor

@msbit msbit commented Dec 12, 2022 •
edited

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.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • I signed the [CLA].
  • 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).
  • No, this is not a breaking change.

@msbit
Copy link
Contributor Author

msbit commented Jan 2, 2023

Happy New Year everyone!

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

✌🏻

@msbit
Copy link
Contributor Author

msbit commented Jan 20, 2023

I'm following up on this PR, and I've noticed that (unlike #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 msbit force-pushed the ad-loader-ad branch 2 times, most recently from a94f12b to d4328ec Compare January 28, 2023 04:34
@joelbrostrom
Copy link

I need this 🙏

@zoeyfan zoeyfan requested a review from jjliu15 February 15, 2023 19:32
@msbit msbit force-pushed the ad-loader-ad branch 2 times, most recently from a954ed5 to f5c75bd Compare March 5, 2023 10:30
@martinsellergren
Copy link

martinsellergren commented Mar 15, 2023 •
edited

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

@msbit msbit force-pushed the ad-loader-ad branch 4 times, most recently from a59cb76 to d7d6aca Compare March 23, 2023 07:42
@msbit msbit force-pushed the ad-loader-ad branch 2 times, most recently from d7d6aca to 555b681 Compare March 29, 2023 00:36
@msbit msbit force-pushed the ad-loader-ad branch 6 times, most recently from 02c4bd3 to d03e4fc Compare April 7, 2023 06:57
@lazytesting
Copy link

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

@oelburk
Copy link

oelburk commented Jan 3, 2024 •
edited

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

@msbit msbit force-pushed the ad-loader-ad branch 2 times, most recently from dfba93c to 9ad7cfb Compare April 21, 2024 03:14
@msbit
Android: Make FlutterNativeAdLoadedListener more generally usable
bb9f531 
In `FlutterNativeAdLoadedListener`, make the inner weak reference
be generic on the `OnNativeAdLoadedListener` interface instead of
the `FlutterNativeAd` concrete class. This allows using the same
listener class with any other type that conforms to
`OnNativeAdLoadedListener`.

Update `FlutterNativeAd` to conform to `OnNativeAdLoadedListener`
to allow delegating in the same existing fashion.
@msbit
iOS: Relax constraints on FLTAdInstanceManager methods
4b21118 
For the `onBanner*` and `onNative*` methods, relax the type constraint
from `FLTBannerAd` and `FLTNativeAd` respectively, to `id<FLTAd>` so any
instance that conforms to `FLTAd` can be used.
@msbit
Android: Allow passing a FlutterAdLoader factory
2db1c3b 
Create a new `GoogleMobileAdsPlugin` constructor which takes an additional
`Supplier<FlutterAdLoader>` argument. This is available solely for testing,
and allows the provision of a mock `FlutterAdLoader` for those `FlutterAd`s
that need it during their construction.
@msbit
Initial AdLoaderAd type
1eb2ecd 
Add the `AdLoaderAd` component, without the specific configuration items
for each of the handled ad types. The component implementation is known
as:

  * `AdLoaderAd`  (Flutter)
  * `FlutterAdLoaderAd` (Android)
  * `FLTAdLoaderAd` (iOS)
@msbit
Allow serving Banner ads in AdLoaderAd
ee10019 
Allow the `AdLoaderAd` instance to serve `Banner` ads, which are
instances of:

  * `AdManagerAdView` under Android, and
  * `GAMBannerView` under iOS
@msbit
Allow serving Custom ads in AdLoaderAd
ded6483 
Allow the `AdLoaderAd` instance to serve `Custom` ads, which are
instances of:

  * `NativeCustomFormatAd` under Android, and
  * `GADCustomNativeAd` under iOS
@msbit
Allow serving Native ads in AdLoaderAd
3705c0f 
Allow the `AdLoaderAd` instance to serve `Native` ads, which are
instances of:

  * `NativeAd` under Android, and
  * `GADNativeAd` under iOS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjliu15 jjliu15 Awaiting requested review from jjliu15

@bparrishMines bparrishMines Awaiting requested review from bparrishMines

@luo86 luo86 Awaiting requested review from luo86

@srichakradhar srichakradhar Awaiting requested review from srichakradhar

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@msbit @joelbrostrom @martinsellergren @lazytesting @oelburk