-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add targeting info to Banner ads #97
base: master
Are you sure you want to change the base?
Conversation
Sorry, I was using the wrong copy of this library for the pubspec change. Fixed it now. |
Thanks for the PR. Sorry for the delays, day job. I plan on going through this PR in the next few days. |
Thanks, that would be great. This will enable proper targeting of ads on
both platforms.
…On Thu, Nov 7, 2019 at 5:48 PM Kevin McGill ***@***.***> wrote:
Thanks for the PR. Sorry for the delays, day job. I plan on going through
this PR in the next few days. testDevices has another PR open to
implementing. Between this PR and that, we will get the feature added. I
can help with the Swift stuff. I'm stronger on the iOS side of things than
Android.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#97?email_source=notifications&email_token=AD34O3TVVAJIOFFB3Q5VCQ3QSRBHNA5CNFSM4JF3NYNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDNBZOI#issuecomment-551165113>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD34O3WBJQ4OYJ42NHHUY6LQSRBHNANCNFSM4JF3NYNA>
.
|
A heads up: Flutter v1.12.13, the next flutter version, will apparently break the plugin via Android dependency changes! This branch contains a fix for that. |
pubspec.yaml
Outdated
@@ -14,6 +14,8 @@ dependencies: | |||
flutter: | |||
sdk: flutter | |||
meta: ^1.1.6 | |||
firebase_admob: 0.9.0+7 |
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 don't think we want to depend on firebase_admob
If you'd like to add targeting info it should be implemented in this code base.
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.
If you want to add MobileAdTargetingInfo
to the dart side here I can help with the Swift side.
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.
Please add MobileAdTargetingInfo
Dart class to this repo instead of adding a dependency to firebase_admob
Thanks for the reviews, I'll see about adding the class tomorrow (once I'm
back at my developer machine).
Also, please do help with the Swift part - Kotlin I could figure out, but
Swift is completely new to me.
Zalan Meggyesi
Chief Support Engineer
Skawa Innovation Kft.
Phone: 0036704627005
Mobile: +36205146666
…On Sun, Dec 15, 2019 at 5:48 AM Kevin McGill ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Please add MobileAdTargetingInfo Dart class to this repo instead of
adding a dependency to firebase_admob
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#97?email_source=notifications&email_token=AD34O3VHCJR6SYM65PLHECLQYWZKPA5CNFSM4JF3NYNKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPG2BQA#pullrequestreview-332243136>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD34O3VB25PWJYFBGJF43OLQYWZKPANCNFSM4JF3NYNA>
.
|
- Lifted MobileAdTargetingInfo from FlutterFire library - Broke dependency on FlutterFire - Update example
Hey guys, just wanted to check if there is any progress on this PR? |
@kmcgill88 check PR #142 |
@jpainam @kmcgill88 sorry I have been very busy for the past few months, if #142 has the correct fix should I close this one. If not @kmcgill88 let me know what's missing so I can close this PR :) |
Has there been any progress on merging these changes and releasing the targeting info support? |
Based on #7, because I also required more precise targeting. According to the plugin code at FlutterFire, there are several targeting factors that are not deprecated, most importantly
keywords
andtestDevices
(not using the latter of which can get your account suspended during testing!), which I propagated through the Kotlin interface, but I must admit I don't know enough Swift (in fact, none at all) to do the same for the iOS interface.Can someone help out with propagating the new map through Swift?