ratson / cordova-plugin-admob-free

New development has been moved to "admob-plus-cordova", https://github.com/admob-plus/admob-plus/tree/master/packages/cordova
https://github.com/admob-plus/admob-plus
MIT License
499 stars 214 forks source link

Support for rewarded video ads #35

Closed warcry2000 closed 7 years ago

warcry2000 commented 7 years ago

Hello,

I have developed the code to use rewarded video ads.

I created this mediation who uses it (AdColony): https://github.com/warcry2000/cordova-plugin-admob-mediation-adcolony

Regards

ratson commented 7 years ago

@warcry2000 Thank you for the PR. I have few questions before merging it.

Is there TEST_REWARDED_VIDEO_ID correct? I see it is the same as TEST_INTERSTITIAL_ID, or any reference documentation about it?

Would you run npm test and ensure it is passed?

Overall, the code looks good.

warcry2000 commented 7 years ago

Hello @ratson, I did not know what to put it and copy it. I used this for my tests 'ca-app-pub-#######'

I've done tests myself, both on android and ios to see that it worked.

I do not understand the tests, I have never done 'npm test', can you give me a notion or give documentation of how to do it?

Regards

ratson commented 7 years ago

@warcry2000 Is that an official id for testing, any reference for it? If it is your personal one, I recommend you to update your comment to mask it.

Or maybe my question is not relevant, the Google test id would work?

I have added a contributing guide, please have a look here.

Let me know if you need any help.

warcry2000 commented 7 years ago

@ratson that id was mine, you have to keep in mind that the videos have to be by mediator and admob does not have defined tests. For the videos to work it is necessary to have a Mediation Networks, in addition to Admob, that is why I have tried with AdColony.

Passing the test and returned me: "expected linebreaks to be 'LF' but found 'CRLF' linebreak-style" I modified: linebreak-style to 0, and it has disappeared.

Then I just had to remove some spaces from Admob.js, and it's all ok

I create another pull request with this change removing the spaces?

ratson commented 7 years ago

I see, then leave test ad ID as it is right now.

I create another pull request with this change removing the spaces? No need, as you can see Github is able to pick up your change after push.

I see there are many places using tab instead of whitespace, I have add npm run lint:spaces to check them. please fix them. You may follow Keeping Fork Updated to update your fork, so you can run the new lint script.

warcry2000 commented 7 years ago

Hello @ratson, I already made the changes Regards

ratson commented 7 years ago

@warcry2000 Thank you for adding this awesome feature, I have invited you to be collaborator of this project, so you could help answering questions about this feature, as you know it better than me.

Before including this in new release, I plan to spend some this weekend testing it and rename some old APIs. Stay tuned.

warcry2000 commented 7 years ago

I’m happy to help...

Perfect. To test the videos it is necessary to have another mediation plugin, like the one I put up. Any questions, happy to answer.

Regards

ezazwar1 commented 7 years ago

is there any success or fail block for rewards video

Regards Ezaz

warcry2000 commented 7 years ago

Hello @ezazwar1

When the version comes out, the following methods can be used.

document.addEventListener('onReceiveRewardVideoAd', function() {}); document.addEventListener('onPresentRewardVideoAd', function() {}); document.addEventListener('onPresentStartedRewardVideoAd', function() {}); document.addEventListener('onDismissRewardVideoAd', function() {}); document.addEventListener('onRewardedVideo', function(data) {}); // data.rewardType, data.rewardAmount

Regards

ratson commented 7 years ago

@ezazwar1 I am planing the next release to have some cleanup about the APIs, the work is on next branch I also need some feedback about the event system (https://github.com/ratson/cordova-plugin-admob-free/issues/38), it would be great if you could share some thoughts.