Skip to content

Conversation

@BETOXL
Copy link

@BETOXL BETOXL commented Oct 11, 2025

  • Adds native and TypeScript support for the Open Ad App format on Android and iOS.
  • Includes usage examples in the Angular demo.
  • Documentation and exposed events.

@distante distante requested a review from Copilot October 11, 2025 09:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for App Open Ad format on both Android and iOS platforms, integrating it into the AdMob plugin with complete TypeScript definitions and usage examples.

  • Adds App Open Ad functionality with load, show, and status check methods
  • Implements native iOS and Android code using Google Mobile Ads SDK
  • Includes TypeScript definitions, events, and Angular demo integration

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/web.ts Adds stub methods for App Open Ad functionality in web implementation
src/index.ts Exports new App Open Ad module
src/definitions.ts Integrates App Open Ad plugin interface into main plugin definitions
src/app-open/* Complete TypeScript definitions for App Open Ad functionality including options, events, and plugin interface
ios/Sources/AdMobPlugin/* Native iOS implementation using GADAppOpenAd with manager pattern
android/src/main/java/* Native Android implementation using Google Mobile Ads AppOpenAd API
demo/angular/src/app/app.component.ts Angular demo showing App Open Ad usage with event listeners
README.md Documentation for App Open Ad API methods and usage examples

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@distante distante left a comment

Choose a reason for hiding this comment

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

Hello @BETOXL ! thanks for your PR, please check the comments and the CI. You probably need to run the formatter. (I will try to add it to the pre commit lint in the future)

Also can you please update the tests to include your changes?

Thank you!

README.md Outdated
} from '@capacitor-community/admob';

export async function showAppOpenAd(): Promise<void> {
// Escuchar eventos
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments in English.

@BETOXL BETOXL requested a review from distante October 13, 2025 21:53
@distante
Copy link
Contributor

a lot of comments are not correctly resolved.

Copy link
Contributor

@distante distante left a comment

Choose a reason for hiding this comment

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

Please do not resolve comments without fixing them.

BETOXL added 11 commits October 14, 2025 12:09
no single line ifs please
this should be inside the AdMob class.
comments in English.
no single line ifs please
Replace any type with proper AppOpenAdOptions type for better type safety and API consistency.
Copy link
Contributor

@distante distante left a comment

Choose a reason for hiding this comment

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

The CI is still failing.

You should check how the other implementation is done so you can fix and add your tests.

Moved AppOpenAd Plugin initialization inside the AdMob class.
Copy link
Author

@BETOXL BETOXL left a comment

Choose a reason for hiding this comment

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

delete comment

@BETOXL BETOXL changed the title feat: soporte para App Open Ad en Android e iOS feat: support for App Open Ad en Android e iOS Oct 25, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment on lines +54 to +66
extension AppOpenAdManager: GADFullScreenContentDelegate {
public func adDidDismissFullScreenContent(_ ad: GADFullScreenPresentingAd) {
appOpenAd = nil
isShowingAd = false
onClosed?()
}

public func ad(_ ad: GADFullScreenPresentingAd, didFailToPresentFullScreenContentWithError error: Error) {
appOpenAd = nil
isShowingAd = false
onFailedToShow?()
}
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The iOS implementation is missing the adWillPresentFullScreenContent delegate method to emit the appOpenAdOpened event.

Looking at other ad implementations (e.g., AdInterstitialExecutor.swift), when an ad is presented, the delegate method adWillPresentFullScreenContent is called and should notify listeners with the corresponding event.

Add the following method to the extension:

public func adWillPresentFullScreenContent(_ ad: GADFullScreenPresentingAd) {
    onOpened?()
}

And store the onOpened callback similar to how onClosed and onFailedToShow are stored.

Copilot uses AI. Check for mistakes.
loadAppOpen(options: AppOpenAdOptions) => Promise<void>
```

Carga un anuncio App Open
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Documentation for the App Open Ad methods is written in Spanish ("Carga un anuncio App Open", "Muestra el anuncio App Open si está cargado", "Verifica si el anuncio App Open está cargado", "Agrega listeners para eventos de App Open").

The rest of the README is in English, so these should be translated for consistency. For example:

  • Line 1120: "Load an App Open ad"
  • Line 1135: "Shows the App Open ad if loaded"
  • Line 1146: "Check if the App Open ad is loaded"
  • Line 1159: "Add listeners for App Open events"

Copilot uses AI. Check for mistakes.
Comment on lines +122 to 197
public void appOpen_ad_Id() {
final String expected = "Some Given AppOpen Test Id";
when(pluginCallMock.getString(eq("adId"), anyString())).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

assertEquals(expected, adOptions.adId);
}

@Test
public void appOpen_position() {
final String wantedProperty = "position";
final String expected = "TOP_CENTER";
final String defaultValue = "BOTTOM_CENTER";
when(pluginCallMock.getString(eq(wantedProperty), anyString())).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

verify(pluginCallMock).getString(wantedProperty, defaultValue);
assertEquals(expected, adOptions.position);
}

@Test
public void appOpen_margin() {
final String wantedProperty = "margin";
final int expected = 10;
final int defaultValue = 0;
when(pluginCallMock.getInt(eq(wantedProperty), anyInt())).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

verify(pluginCallMock).getInt(wantedProperty, defaultValue);
assertEquals(expected, adOptions.margin);
}

@Test
public void appOpen_isTesting() {
final String wantedProperty = "isTesting";
final boolean expected = true;
final boolean defaultValue = false;
when(pluginCallMock.getBoolean(eq(wantedProperty), anyBoolean())).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

verify(pluginCallMock).getBoolean(wantedProperty, defaultValue);
assertEquals(expected, adOptions.isTesting);
}

@Test
public void appOpen_npa() {
final String wantedProperty = "npa";
final boolean expected = true;
final boolean defaultValue = false;
lenient().when(pluginCallMock.getBoolean(eq(wantedProperty), anyBoolean())).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

verify(pluginCallMock).getBoolean(wantedProperty, defaultValue);
assertEquals(expected, adOptions.npa);
}

@Test
public void appOpen_ssv() {
final String customData = "customData";
final String userId = "userId";
final String wantedProperty = "ssv";
final JSObject expected = new JSObject();
expected.put(customData, customData);
expected.put(userId, userId);
lenient().when(pluginCallMock.getObject(eq(wantedProperty))).thenReturn(expected);

final AdOptions adOptions = AdOptions.getFactory().createAppOpenOptions(pluginCallMock);

verify(pluginCallMock, atLeastOnce()).getObject(wantedProperty);
assertEquals(userId, adOptions.ssvInfo.getUserId());
assertEquals(customData, adOptions.ssvInfo.getCustomData());
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The tests reference a createAppOpenOptions method on AdOptions.getFactory(), but this method doesn't exist in the AdOptionsFactory class.

Looking at the factory pattern in AdOptions.java, you need to add a createAppOpenOptions method to the AdOptionsFactory class, similar to createBannerOptions, createInterstitialOptions, etc. The method should return an AdOptions instance with an appropriate testing ID for App Open ads.

Copilot uses AI. Check for mistakes.
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