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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃悰story-ads: prevent users getting stuck #19335

Merged
merged 1 commit into from Nov 15, 2018
Merged

馃悰story-ads: prevent users getting stuck #19335

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 13 additions & 9 deletions extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.js
View file
Open in desktop
Expand Up @@ -152,13 +152,8 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
/** @private {?../../amp-story/0.1/navigation-state.NavigationState} */
this.navigationState_ = null;

/**
* We are counting pages on page-clicks. We initialize this to 1 because
* even though we have not yet seen a page click, we have already seen one
* page.
* @private {number}
*/
this.uniquePagesCount_ = 1;
/** @private {number} */
this.uniquePagesCount_ = 0;

/** @private {!Object<string, boolean>} */
this.uniquePageIds_ = dict({});
Expand Down Expand Up @@ -196,6 +191,9 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
/** @private {boolean} */
this.firstAdViewed_ = false;

/** @private {boolean} */
this.pendingAdView_ = false;

/**
* Version of the story store service depends on which version of amp-story
* the publisher is loading. They all have the same implementation.
Expand Down Expand Up @@ -565,6 +563,9 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
[Vars.AD_INDEX]: adIndex,
});

// Previously inserted ad has been viewed.
this.pendingAdView_ = false;

// Start loading next ad.
this.startNextAdPage_();

Expand All @@ -573,14 +574,17 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
this.idOfAdShowing_ = adIndex;
}


if (this.enoughContentPagesViewed_()) {
// If there is already an ad inserted, but not viewed it doesn't matter how
// many pages we have seen, we should not keep trying to insert more ads.
if (!this.pendingAdView_ && this.enoughContentPagesViewed_()) {
const adState = this.tryToPlaceAdAfterPage_(pageId);

if (adState === AD_STATE.INSERTED) {
this.analyticsEventWithCurrentAd_(Events.AD_INSERTED,
{[Vars.AD_INSERTED]: Date.now()});
this.adsPlaced_++;
// We have an ad inserted that has yet to be viewed.
this.pendingAdView_ = true;
}

if (adState === AD_STATE.FAILED) {
Expand Down