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

🚀 Make sure amp-story ad is loaded before display #13618

Merged
merged 2 commits into from
Feb 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 80 additions & 25 deletions extensions/amp-story/0.1/amp-story-auto-ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,26 @@
* limitations under the License.
*/

import {CommonSignals} from '../../../src/common-signals';
import {Services} from '../../../src/services';
import {StateChangeType} from './navigation-state';
import {createElementWithAttributes} from '../../../src/dom';
import {dev, user} from '../../../src/log';
import {dict} from '../../../src/utils/object';

// temp before config in passed in

// TODO(ccordry) replace these constants with user config
/** @const */
const MIN_INTERVAL = 3;

/** @const */
const MAX_NUMBER = 2;

/** @const */
// const EXPERIMENT = 'amp-story-auto-ad';
const TAG = 'amp-story-auto-ads';

/** @const */
const TAG = 'amp-story-auto-ads';
const AD_TAG = 'amp-ad';

export class AmpStoryAutoAds extends AMP.BaseElement {

Expand All @@ -46,10 +51,16 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
this.interactions_ = 0;

/** @private {!Array} */
this.adElements_ = [];
this.adPageEls_ = [];

/** @private {number} */
this.adsPlaced_ = 0;

/** @private {number} */
this.adPagesCreated_ = 0;

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

/** @override */
Expand All @@ -58,6 +69,10 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
user().assert(ampStoryElement.tagName === 'AMP-STORY',
`<${TAG}> should be child of <amp-story>`);

const ampdoc = this.getAmpDoc();
Services.extensionsFor(this.win)./*OK*/installExtensionForDoc(
ampdoc, AD_TAG);

ampStoryElement.getImpl().then(impl => {
this.ampStory_ = impl;
this.navigationState_ = this.ampStory_.getNavigationState();
Expand All @@ -78,43 +93,84 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
return Promise.resolve();
}


/**
* build page and start preloading
* @private
*/
schedulePage_() {
const page = this.makeMockPage();
this.adElements_.push(page);
const page = this.createAdPage_();
this.adPageEls_.push(page);

this.ampStory_.element.appendChild(page);

// TODO(ccordry): need to fake distance from page to force load

page.getImpl().then(impl => {
this.ampStory_.addPage(impl);
});
}


/**
* temporary to be replaced with real fetching
* create an `amp-story-page` containing an `amp-ad`
* @private
*/
makeMockPage() {
const ampStoryAdPage = document.createElement('amp-story-page');
const id = this.adsPlaced_ + 1;
ampStoryAdPage.id = `i-amphtml-ad-page-${id}`;
ampStoryAdPage.setAttribute('ad', '');
ampStoryAdPage./*OK*/innerHTML = `
<amp-story-grid-layer template="vertical">
<h1>Ad Page #${id}</h1>
<p>This is ad #${id} shown in this story.</p>
</amp-story-grid-layer>
`;
createAdPage_() {
// TODO(ccordry) add new <amp-story-cta-layer>
const ampStoryAdPage = this.createPageElement_();
const ampAd = this.createAdElement_();

ampStoryAdPage.appendChild(ampAd);

this.isCurrentAdLoaded_ = false;

// set up listener for ad-loaded event
ampAd.getImpl().then(impl => {
const signals = impl.signals();
return signals.whenSignal(CommonSignals.INI_LOAD);
}).then(() => {
this.isCurrentAdLoaded_ = true;
});

return ampStoryAdPage;
}


/**
* Get array containing all pages of associated story
* @return {!Element}
* @private
*/
createPageElement_() {
const id = ++this.adPagesCreated_;
const attributes = dict({
'id': `i-amphtml-ad-page-${id}`,
'ad': '',
'distance': '1',
});

return createElementWithAttributes(
document, 'amp-story-page', attributes);
}


/**
* @return {!Element}
* @private
*/
createAdElement_() {
// TODO(ccordry) get this info (e.g. source) from config
const attributes = dict({
'id': 'i-amphtml-demo-ad',
'height': '300',
'src': '/extensions/amp-ad-network-fake-impl/0.1/data/fake_amp.json',
'type': 'fake',
});

return createElementWithAttributes(
document, 'amp-ad', attributes);
}


/**
* @private
*/
handleStateChange_(stateChangeEvent) {
Expand Down Expand Up @@ -158,14 +214,13 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
* @private
*/
placeAdAfterPage_(currentPageId) {
// TODO(ccordry) make sure ad is loaded
const nextAdElement = this.adElements_[this.adElements_.length - 1];
const nextAdPageEl = this.adPageEls_[this.adPageEls_.length - 1];

if (!nextAdElement) {
if (!nextAdPageEl || !this.isCurrentAdLoaded_) {
return;
}

this.ampStory_.insertPage(currentPageId, nextAdElement.id);
this.ampStory_.insertPage(currentPageId, nextAdPageEl.id);
this.adsPlaced_++;

if (!this.allAdsPlaced_()) {
Expand Down
5 changes: 5 additions & 0 deletions extensions/amp-story/0.1/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,11 @@ export class AmpStoryPage extends AMP.BaseElement {
});
}


/**
* check to see if this page is a wrapper for an ad
* @return {boolean}
*/
isAd() {
return this.element.hasAttribute(ADVERTISEMENT_ATTR_NAME);
}
Expand Down