Skip to content

refactor(admob): add snippets from quickstart-android #272

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

Merged
merged 4 commits into from
Jan 14, 2021

Conversation

thatfiredev
Copy link
Member

@thatfiredev thatfiredev commented Jan 14, 2021

As discussed in firebase/quickstart-android#1247, we're moving the admob snippets to this repo.

Snippets added in this PR:

Filename Snippets added
MainActivity.kt ads_on_create,load_banner_ad, instantiate_interstitial_ad, create_interstitial_ad_listener, display_interstitial_ad, request_new_interstitial, add_lifecycle_methods
MainActivity.java ads_on_create,load_banner_ad, instantiate_interstitial_ad, create_interstitial_ad_listener, display_interstitial_ad, request_new_interstitial, add_lifecycle_methods
activity_main.xml define_ad_layout
build.gradle gradle_play_config

There are snippets in AndroidManifest.xml which were kept in the quickstarts repo.

I also noticed that the AdListener.onAdFailedToLoad(int) method used here was deprecated in favor of AdListener.onAdFailedToLoad(LoadAdError), so I replaced it.

MobileAds.initialize(context);

// [START load_banner_ad]
AdRequest adRequest = new AdRequest.Builder().build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason all of these snippets are in the onCreate function instead of in helper functions? Having most snippets live in their own function is generally easier to maintain, especially when they don't need to refer to each other's results/variables.

Copy link
Member Author

@thatfiredev thatfiredev Jan 14, 2021

Choose a reason for hiding this comment

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

Sounds good. And that actually leaves our onCreate snippet untouched.
I've updated the PR.

@samtstern
Copy link
Contributor

@rosariopfernandes this is great thank you! I'll make the changes on the doc site to point to these.

@samtstern samtstern merged commit 3cdf635 into firebase:master Jan 14, 2021
@thatfiredev thatfiredev deleted the rpf-admob-snippets branch January 14, 2021 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants