Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

listener on MobileAd shouldn't be final #650

Merged
merged 5 commits into from
Sep 4, 2018
Merged

Conversation

mickykebe
Copy link
Contributor

Ad listeners shouldn't necessarily have to be set during Ad initialization.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@mickykebe
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Jul 4, 2018
@kroikie kroikie requested review from mravn-google and kroikie and removed request for mravn-google July 9, 2018 15:12
@kroikie kroikie self-assigned this Jul 9, 2018
private void callIslAdLoaded(int id, MethodCall call, Result result) {
MobileAd ad = MobileAd.getAdForId(id);
if(ad == null) {
result.error("no_ad_for_id", "isAdLoaded failed, no add exists for id=" + id, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Type in message. Should be "no ad exists for id".

Maybe we could also use $id syntax for including the Ad ID in the message.

@@ -154,6 +154,15 @@ private void callShowAd(int id, MethodCall call, Result result) {
result.success(Boolean.TRUE);
}

private void callIslAdLoaded(int id, MethodCall call, Result result) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this method should be named callIsAdLoaded instead of callIslAdLoaded?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated method name.

@@ -32,6 +32,6 @@ android {
disable 'InvalidPackage'
}
dependencies {
api 'com.google.firebase:firebase-ads:15.+'
api 'com.google.firebase:firebase-ads:15.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the version of this dependency has been bumped to 15.0.1, rebasing on master should resolve this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used latest sdk version.

@kroikie
Copy link
Contributor

kroikie commented Aug 27, 2018

@mickykebe Really appreciate this PR, would you be able to update this PR so it can be merged in?

mickykebe and others added 3 commits September 4, 2018 15:30
@kroikie kroikie merged commit 7f8efac into flutter:master Sep 4, 2018
nichtverstehen pushed a commit to nichtverstehen/plugins that referenced this pull request Sep 6, 2018
* listener on MobileAd shouldn't be final

* Ad listeners shouldn't necessarily have to be set during Ad initialization.

* add isLoaded method to firebase_admob MobileAd checking whether ad has been loaded.
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
* listener on MobileAd shouldn't be final

* Ad listeners shouldn't necessarily have to be set during Ad initialization.

* add isLoaded method to firebase_admob MobileAd checking whether ad has been loaded.
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
* listener on MobileAd shouldn't be final

* Ad listeners shouldn't necessarily have to be set during Ad initialization.

* add isLoaded method to firebase_admob MobileAd checking whether ad has been loaded.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants