feat: support for App Open Ad en Android e iOS - fix comments by yarivgdidi · Pull Request #422 · capacitor-community/admob

yarivgdidi · 2026-03-19T19:48:41Z

No description provided.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

no single line ifs please

this should be inside the AdMob class.

comments in English.

no single line ifs please

comments in English

Replace any type with proper AppOpenAdOptions type for better type safety and API consistency.

Moved AppOpenAd Plugin initialization inside the AdMob class.

delete comment

test appOpen

…/AppOpenAdPlugin.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

README.md

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdManager.swift

- Fix failing Android test: stub runOnUiThread to execute runnable synchronously so bannerExecutor.initialize() is verified correctly - Fix README: use adId (not adUnitId) in AppOpenAdOptions example and table, fix grammar "Load an App Open ad", remove extra leading blank line - Fix iOS: clear callback closures after invoking them in delegate methods to avoid unintended retention of CAPPluginCall and notifier references Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

distante

So finally I had some time to check the code. While the construction differs from how the other kind of Ads are build, the main problem currently is that the errors are just eaten by the plugin and not passed to the consumer. This will lead to people creating issues here for things that are not handled by the plugin but by AdMob itself.

Please pass the errors from the Native layer to the browser.

Also let the LLM or Android Studio convert the code from java to Kotlin.

Thanks for the PR!

android/src/main/java/com/getcapacitor/community/admob/appopen/AppOpenAdPlugin.java

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdPlugin.swift

demo/angular/src/app/app.component.ts

- Android: add AppOpenAdPluginEvents.kt enum, replace magic strings - Android: pass AdMobPluginError with code/message on failure events - Android: propagate LoadAdError/AdError from manager through callbacks - iOS: propagate Error through onFailed/onFailedToShow callbacks - iOS: include code and message in failure event notifications Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/app-open/app-open-definitions.interface.ts

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdPlugin.swift

android/src/main/java/com/getcapacitor/community/admob/models/AdOptions.java

demo/angular/src/app/app.component.ts

- Fix initialization to properly chain promises (wait for AdMob.initialize before calling other methods), preventing demo app crash - Move App Open Ad from app.component to home page with load/show buttons, matching the pattern of other ad types (interstitial, reward) - Add App Open Ad section to demo UI with event listeners - Add App Open screenshots (iOS + Android) to README - Fix AppOpenAdPluginEvents.kt: use const val instead of override val to fix Java interop (private access error) - Use Google test ad ID for App Open (ca-app-pub-3940256099942544/5575463023) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Add per-event typed addListener overloads to AppOpenAdPlugin interface, matching the pattern in InterstitialDefinitions (FailedToLoad/FailedToShow receive AdMobError, others receive void) - iOS: check isAdLoaded() before attempting to show, giving a specific "App Open Ad is not loaded" rejection instead of a generic error - Remove unused createAppOpenOptions/APP_OPEN_TESTER_ID from AdOptions and their corresponding tests (App Open reads adId directly from call) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

README.md

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdManager.swift

- Add error parameter to FailedToLoad/FailedToShow listener examples in README to match typed API (AdMobError) - Remove unused rootViewController parameter from iOS loadAd method Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

distante · 2026-04-09T13:50:16Z

@yarivgdidi what happened with the tests? ff6eea6#diff-ce3f6b7ca8731f9babda30ed293b9135ef12b0b452b0ce52c01bf9c8a142463c

- iOS: add AppOpenAdPluginEvents enum (matching other ad types pattern), replace hardcoded event strings with enum rawValues - iOS: remove unused getRootViewController param from loadAppOpen - iOS/Android: include adUnitId in Loaded event and resolve response, matching interstitial/reward behavior - Android: use runOnMain() helper consistently in showAppOpen Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Reverts the removal of AdOptionsFactory.createAppOpenOptions() and APP_OPEN_TESTER_ID, along with their corresponding tests. These were removed per a Copilot suggestion but the maintainer wants them kept for consistency with other ad types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

yarivgdidi · 2026-04-09T13:58:03Z

Restored createAppOpenOptions, APP_OPEN_TESTER_ID, and all 6 tests in commit 182330b. They were accidentally removed when addressing a Copilot comment — apologies for the confusion.

yarivgdidi · 2026-04-09T14:00:06Z

@distante, sorry for the tests
Claude code sometimes goes to far
restored

…otlin Convert both App Open Ad files to idiomatic Kotlin per reviewer request. Uses Kotlin features: named parameters, apply blocks, null safety, fun interface, property accessors, and lambda syntax. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

distante

Thanks I will merge this

distante · 2026-04-10T14:24:24Z

@rdlabo FYI

yarivgdidi · 2026-04-11T17:16:19Z

@distante
Thanks for supporting this.
I see the merge but no version update (still 8.0.0)
the npm package also not updated

distante · 2026-04-11T17:18:13Z

@yarivgdidi the one with rights to deploy is @rdlabo

yarivgdidi and others added 27 commits March 19, 2026 21:25

rebase main

4f9ea4f

Update src/web.ts

ce515c3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/web.ts

2cb85f3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update demo/angular/src/app/app.component.ts

8a46c8a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

79654eb

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

540573c

Update README.md

8c8d6f9

Update AppOpenAdManager.java

544528f

Update AppOpenAdManager.java

538cc30

Update AppOpenAdManager.java

cd21f66

no single line ifs please

Update AdMob.java

88df068

this should be inside the AdMob class.

Update app.component.ts

cb76260

comments in English.

Update AppOpenAdManager.swift

4a8071e

no single line ifs please

Update app-open-definitions.interface.ts

ff7f28f

comments in English

Update web.ts

8be2121

Replace any type with proper AppOpenAdOptions type for better type safety and API consistency.

Update README.md

8cbc810

Refactor AppOpenAd Plugin initialization

2377c8f

Moved AppOpenAd Plugin initialization inside the AdMob class.

Define AppOpenAdOptions interface and update loadAppOpen

31ac6e5

delete comment

Update AppOpenAdManager.java

088fe10

test appOpen

Update android/src/main/java/com/getcapacitor/community/admob/appopen…

13b0992

…/AppOpenAdPlugin.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/web.ts

0d64991

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

2a2fc5e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update demo/angular/src/app/app.component.ts

b42a565

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

7564a9b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

8ec3d1b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update demo/angular/src/app/app.component.ts

f925ea0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix App Open ad lifecycle, event emission, and docs across Android/iOS

c1c19ce

yarivgdidi changed the title ~~App open ad review fixes~~ feat: support for App Open Ad en Android e iOS - fix comments Mar 19, 2026

yarivgdidi added 2 commits March 21, 2026 13:04

GoogleMobileAds fixes

9b08fdf

GoogleMobileAds fixes

066e0ff

Copilot AI reviewed Apr 8, 2026

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdManager.swift Show resolved Hide resolved

distante requested changes Apr 8, 2026

View reviewed changes

distante reviewed Apr 8, 2026

View reviewed changes

demo/angular/src/app/app.component.ts Outdated Show resolved Hide resolved

distante mentioned this pull request Apr 8, 2026

feat: support for App Open Ad en Android e iOS #386

Closed

distante requested a review from Copilot April 8, 2026 16:55

Copilot started reviewing on behalf of distante April 8, 2026 16:55 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

yarivgdidi and others added 2 commits April 8, 2026 20:48

yarivgdidi requested a review from distante April 8, 2026 17:53

distante requested a review from Copilot April 9, 2026 09:03

Copilot started reviewing on behalf of distante April 9, 2026 09:04 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

ios/Sources/AdMobPlugin/AppOpen/AppOpenAdManager.swift Outdated Show resolved Hide resolved

yarivgdidi and others added 2 commits April 9, 2026 16:53

distante approved these changes Apr 10, 2026

View reviewed changes

distante merged commit 6946bfb into capacitor-community:main Apr 10, 2026
5 checks passed

This was referenced Apr 10, 2026

Needs app open ad support #260

Closed

Google Ad type "App open" #167

Closed

App open ad unit is now needed more then ever #338

Closed

Conversation

yarivgdidi commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

distante left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

distante commented Apr 9, 2026

Uh oh!

yarivgdidi commented Apr 9, 2026

Uh oh!

yarivgdidi commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

distante left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

distante commented Apr 10, 2026

Uh oh!

yarivgdidi commented Apr 11, 2026

Uh oh!

distante commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yarivgdidi commented Apr 9, 2026 •

edited

Loading