🚀 Make sure amp-story ad is loaded before display #13618
Conversation
| * @private | ||
| */ | ||
| createPageElement_() { | ||
| const id = this.adsPlaced_ + 1; |
There was a problem hiding this comment.
Is it problematic that this function can return two different elements with the same ID? (since adsPlaced_ is not incremented here)
There was a problem hiding this comment.
I addressed both of these concerns by adding a new adPagesCreated_ which is incremented inside this method. I think this is the best solution because ads can be created but never placed due to bad fetch, not loading in time etc. so they should be tallied separately. Also this helps mapping the config ad number to the id (e.g.. template ad #2 is never placed, so template ad #3 gets and id of 2)
|
|
||
| ampStoryAdPage.appendChild(ampAd); | ||
|
|
||
| this.iscurrentAdLoaded_ = false; |
|
|
||
| if (!nextAdElement) { | ||
| if (!nextAdPageEl || !this.iscurrentAdLoaded_) { |
There was a problem hiding this comment.
will there be race condition that a new ad is not scheduled, and an old ad gets inserted twice?
There was a problem hiding this comment.
There shouldn't be. It doesn't start loading the next ad until the previous add is placed. This question does make me realize that there maybe should be some logic in a future to move onto the next ad if we are sure that the previous ad will never load.
| * @private | ||
| */ | ||
| createPageElement_() { | ||
| const id = this.adsPlaced_ + 1; |
There was a problem hiding this comment.
Is it problematic that this function can return two different elements with the same ID? (since adsPlaced_ is not incremented here)
* check if ad loaded * add created count
The idea with
amp-story-auto-adsis to place an ad as the next page after a certain amount of users interactions, but only after that ad is loaded. If the ad is not loaded it will wait for another user interaction before it tries again to place the ad.This PR ads a more realistic ad implementation (using the fake-ad component), and the logic to make sure that the
loadedevent has fired on the ad before it will insert the page.