Skip to content
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

testDevices should not be a prop #44

Closed
halaei opened this issue Jun 29, 2020 · 5 comments
Closed

testDevices should not be a prop #44

halaei opened this issue Jun 29, 2020 · 5 comments
Labels
enhancement New feature or request work in progress

Comments

@halaei
Copy link
Contributor

halaei commented Jun 29, 2020

As described in https://developers.google.com/admob/android/test-ads, the process of setting test device ids can be done only once, and after that all the ads, no matter if native, or banner, or other types, will be test ads. However, in this package the NativeAdView components have a testDevices prop, with the default of empty array. So I need to set the prop everywhere in the code I need an ad, instead of calling a function just once, e.g. in App.js or index.js. This is a bit inefficient and causes problems when using this package alongside other AdMob related react native packages.

I suggest to remove testDevices prop from NativeAdView and instead expose a function like the following, not only to fix the mentioned problems, but also to have full support of RequestConfiguration.Builder:

@ReactMethod
// This is just a pseudocode for java without type validation and all the necessary castings: 
public void setRequestConfiguration(ReadableMap config) {
	RequestConfiguration configuration = new RequestConfiguration.Builder();
	if (config.hasKey("maxAdContentRating")) {
		configuration.setMaxAdContentRating(config.getString("maxAdContentRating"));
	}
	if (config.hasKey("tagForChildDirectedTreatment")) {
		configuration.setTagForChildDirectedTreatment(config.getInt("tagForChildDirectedTreatment"));
	}
	if (config.hasKey("tagForUnderAgeOfConsent")) {
		configuration.setTagForUnderAgeOfConsent(config.getInt("TagForUnderAgeOfConsent"));
	}
	if (config.hasKey("testDeviceIds")) {
		configuration.setTestDeviceIds(config.getArray("testDeviceIds"));
	}

	MobileAds.setRequestConfiguration(configuration.build());
}
@ammarahm-ed
Copy link
Owner

I think we can do it with a Native Module already present in the project and loading our configuration on app start. I will add this in the next release in a few days.

@ammarahm-ed ammarahm-ed added enhancement New feature or request work in progress labels Jul 1, 2020
@ammarahm-ed
Copy link
Owner

This has been implemented for v0.3.0

@halaei
Copy link
Contributor Author

halaei commented Jul 6, 2020

@ammarahm-ed Thank. But I guess you haven't yet remove testDevices prop. The default value is an empty array which cancels out any previous calls to setRequestConfiguration.
If you like to keep the prop, maybe setting the default to null instead of an empty array and not calling setRequestConfiguration when prop is null can help.

@halaei
Copy link
Contributor Author

halaei commented Jul 6, 2020

testDevices={props.testDevices? props.testDevices : []}

@ammarahm-ed
Copy link
Owner

fixed now. v0.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request work in progress
Projects
None yet
Development

No branches or pull requests

2 participants