Skip to content

🧪 Experiment: story auto advance and new cta button #38097

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
powerivq merged 2 commits into from
Apr 16, 2022
Merged

🧪 Experiment: story auto advance and new cta button #38097

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50fd083
Experiment: story auto advance and new cta button
powerivq Apr 13, 2022
594e25b
Review comments
powerivq Apr 15, 2022
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
Next Next commit
Experiment: story auto advance and new cta button
  • Loading branch information
@powerivq
powerivq committed Apr 14, 2022
commit 50fd083b464888a1a599818a8b41a75aa5f1b93b
9 changes: 9 additions & 0 deletions extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
randomlySelectUnsetExperiments,
} from '#experiments';
import {StoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';
import {Navigation} from '#service/navigation';
Expand Down Expand Up @@ -243,6 +244,14 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
if (storyAdPlacementsExpId) {
addExperimentIdToElement(storyAdPlacementsExpId, this.element);
}

const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
);
if (storyAdSegmentBranch) {
addExperimentIdToElement(storyAdSegmentBranch, this.element);
}
}

/**
Expand Down
9 changes: 9 additions & 0 deletions extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
randomlySelectUnsetExperiments,
} from '#experiments';
import {StoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';
import {Navigation} from '#service/navigation';
Expand Down Expand Up @@ -495,6 +496,14 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
if (storyAdPlacementsExpId) {
addExperimentIdToElement(storyAdPlacementsExpId, this.element);
}

const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
);
if (storyAdSegmentBranch) {
addExperimentIdToElement(storyAdSegmentBranch, this.element);
}
}

/**
Expand Down
8 changes: 7 additions & 1 deletion extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {CommonSignals_Enum} from '#core/constants/common-signals';

import {forceExperimentBranch} from '#experiments';
import {divertStoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';

Expand Down Expand Up @@ -43,7 +44,12 @@ const MUSTACHE_TAG = 'amp-mustache';
* @const {Object<string, string>}
* @visibleForTesting
*/
export const RELEVANT_PLAYER_EXPS = {};
export const RELEVANT_PLAYER_EXPS = {
[StoryAdSegmentExp.CONTROL]: StoryAdSegmentExp.ID,
[StoryAdSegmentExp.AUTO_ADVANCE_OLD_CTA]: StoryAdSegmentExp.ID,
[StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA]: StoryAdSegmentExp.ID,
[StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA_NOT_ANIMATED]: StoryAdSegmentExp.ID,
};

/** @enum {string} */
export const Attributes = {
Expand Down
12 changes: 10 additions & 2 deletions extensions/amp-story-auto-ads/0.1/story-ad-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {setStyle} from '#core/dom/style';
import {map} from '#core/types/object';
import {parseJson} from '#core/types/object/json';

import {isExperimentOn} from '#experiments';
import {getExperimentBranch} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {getData, listen} from '#utils/event-helper';
import {dev, devAssert, userAssert} from '#utils/log';
Expand Down Expand Up @@ -313,7 +314,14 @@ export class StoryAdPage {
'id': this.id_,
};

if (isExperimentOn(this.win_, 'story-ad-auto-advance')) {
const storyAdSegmentBranch = getExperimentBranch(
this.win_,
StoryAdSegmentExp.ID
);
if (
storyAdSegmentBranch &&
storyAdSegmentBranch != StoryAdSegmentExp.CONTROL
) {
attributes['auto-advance-after'] = '10s';
}

Expand Down
13 changes: 11 additions & 2 deletions extensions/amp-story-auto-ads/0.1/story-ad-ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import {createElementWithAttributes} from '#core/dom';
import {map} from '#core/types/object';
import {getWin} from '#core/window';

import {isExperimentOn} from '#experiments';
import {getExperimentBranch} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';

Expand Down Expand Up @@ -309,7 +310,15 @@ export function createCta(doc, buttonFitter, container, uiMetadata) {
return null;
}

if (isExperimentOn(doc.defaultView, 'story-ad-auto-advance')) {
const storyAdSegmentBranch = getExperimentBranch(
doc.defaultView,
StoryAdSegmentExp.ID
);
if (
storyAdSegmentBranch == StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA ||
storyAdSegmentBranch ==
StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA_NOT_ANIMATED
) {
return createPageOutlink_(doc, uiMetadata, container);
} else {
return createCtaLayer_(a, doc, container);
Expand Down
9 changes: 7 additions & 2 deletions extensions/amp-story-auto-ads/0.1/test/test-story-ad-ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {toggleExperiment} from '#experiments';
import {forceExperimentBranch} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {ButtonTextFitter} from '../story-ad-button-text-fitter';
import {
Expand Down Expand Up @@ -248,7 +249,11 @@ describes.realWin('story-ad-ui', {amp: true}, (env) => {

beforeEach(() => {
buttonFitter = new ButtonTextFitter(env.ampdoc);
toggleExperiment(win, 'story-ad-auto-advance', true);
forceExperimentBranch(
win,
StoryAdSegmentExp.ID,
StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA
);
});

it('createCta page outlink custom theme element', () => {
Expand Down
8 changes: 8 additions & 0 deletions extensions/amp-story-page-attachment/0.1/amp-story-open-page-attachment.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@
animation: i-amphtml-tap-scale var(--i-amphtml-page-attachment-ui-animation-duration) var(--i-amphtml-page-attachment-ui-animation-delay) both !important;
}

.i-amphtml-story-page-open-attachment-outlink-no-animation-exp.i-amphtml-story-page-open-attachment-outlink-no-animation-exp[active] {
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to avoid the animations above, it's better to do .i-amphtml-story-page-open-attachment-outlink[active]:not(.i-amphtml-story-page-open-attachment-outlink-no-animation-exp) {animation: i-amphtml-tap-scale...} instead of overriding it with animation: none on a new property. Same below.

animation: none !important;
}

.i-amphtml-story-outlink-page-attachment-arrow {
display: block !important;
cursor: pointer !important;
Expand All @@ -180,6 +184,10 @@
animation: i-amphtml-move-up-arrow var(--i-amphtml-page-attachment-ui-animation-duration) var(--i-amphtml-page-attachment-ui-animation-delay) both !important;
}

.i-amphtml-story-page-open-attachment.i-amphtml-story-page-open-attachment-outlink-no-animation-exp[active] .i-amphtml-story-outlink-page-attachment-arrow {
animation: none !important;
}

@keyframes i-amphtml-move-up-arrow {
0%, 100% {
opacity: 1;
Expand Down
14 changes: 13 additions & 1 deletion extensions/amp-story-page-attachment/0.1/amp-story-open-page-attachment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
/**
* @fileoverview Helper for amp-story rendering of page-attachment UI.
*/
import objStr from 'obj-str';

import * as Preact from '#core/dom/jsx';
import {scopedQuerySelector} from '#core/dom/query';
import {computedStyle, setImportantStyles} from '#core/dom/style';
import {getWin} from '#core/window';

import {getExperimentBranch} from '#experiments';

import {Services} from '#service';
import {LocalizedStringId_Enum} from '#service/localization/strings';

Expand Down Expand Up @@ -128,13 +132,21 @@ const renderOutlinkUI = (pageEl, attachmentEl) => {
// Set image.
const openImgAttr = attachmentEl.getAttribute('cta-image');

const noAnimation =
getExperimentBranch(getWin(pageEl), 'story-ad-auto-advance') == 'c';
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this constant c from? Do you need to replace it with the correct value from StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA_NOT_ANIMATED?


return openLabelOrFallback(
pageEl,
anchorChild?.textContent || ctaLabelFromAttr(attachmentEl)
).then((openLabel) => {
const openAttachmentEl = (
<a
class="i-amphtml-story-page-open-attachment i-amphtml-story-page-open-attachment-outlink"
class={objStr({
'i-amphtml-story-page-open-attachment': true,
'i-amphtml-story-page-open-attachment-outlink': true,
'i-amphtml-story-page-open-attachment-outlink-no-animation-exp':
noAnimation,
})}
role="button"
target="_top"
title={attachmentTitle}
Expand Down
12 changes: 10 additions & 2 deletions extensions/amp-story/1.0/amp-story-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import {isAutoplaySupported, tryPlay} from '#core/dom/video';
import {toArray} from '#core/types/array';
import {debounce, once} from '#core/types/function';

import {isExperimentOn} from '#experiments';
import {getExperimentBranch, isExperimentOn} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';
import {LocalizedStringId_Enum} from '#service/localization/strings';
Expand Down Expand Up @@ -1105,8 +1106,15 @@ export class AmpStoryPage extends AMP.BaseElement {
emitProgress_(progress) {
// Don't emit progress for ads, since the progress bar is hidden.
// Don't emit progress for inactive pages, because race conditions.
const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
);
const progressBarExpDisabled =
!storyAdSegmentBranch ||
storyAdSegmentBranch == StoryAdSegmentExp.CONTROL;
if (
(!isExperimentOn(this.win, 'story-ad-auto-advance') && this.isAd()) ||
(progressBarExpDisabled && this.isAd()) ||
this.state_ === PageState.NOT_ACTIVE
) {
return;
Expand Down
10 changes: 8 additions & 2 deletions extensions/amp-story/1.0/amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ import {endsWith} from '#core/types/string';
import {parseQueryString} from '#core/types/string/url';
import {getHistoryState as getWindowHistoryState} from '#core/window/history';

import {isExperimentOn} from '#experiments';
import {getExperimentBranch, isExperimentOn} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';
import {calculateExtensionFileUrl} from '#service/extension-script';
Expand Down Expand Up @@ -655,9 +656,14 @@ export class AmpStory extends AMP.BaseElement {
return;
}

const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
);
if (
!this.activePage_.isAd() ||
isExperimentOn(this.win, 'story-ad-auto-advance')
(storyAdSegmentBranch &&
storyAdSegmentBranch != StoryAdSegmentExp.CONTROL)
) {
this.systemLayer_.updateProgress(pageId, progress);
}
Expand Down
11 changes: 9 additions & 2 deletions extensions/amp-story/1.0/progress-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {scale, setImportantStyles} from '#core/dom/style';
import {debounce} from '#core/types/function';
import {hasOwn, map} from '#core/types/object';

import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';

import {dev, devAssert} from '#utils/log';

import {isExperimentOn} from 'src/experiments';
import {getExperimentBranch} from 'src/experiments';

import {
StateProperty,
Expand Down Expand Up @@ -155,12 +157,17 @@ export class ProgressBar {
this.clear_();
}

const storyAdSegmentBranch = getExperimentBranch(
this.win_,
StoryAdSegmentExp.ID
);
/** @type {!Array} */ (pageIds).forEach((id) => {
if (
// Do not show progress bar for the ad page (control group).
// Show progress bar for the ad page (experiment group).
(!this.isAdSegment_(id) ||
isExperimentOn(this.win_, 'story-ad-auto-advance')) &&
(storyAdSegmentBranch &&
Comment thread
mszylkowski marked this conversation as resolved.
storyAdSegmentBranch != StoryAdSegmentExp.CONTROL)) &&
!(id in this.segmentIdMap_)
) {
this.addSegment_(id);
Expand Down
9 changes: 7 additions & 2 deletions extensions/amp-story/1.0/test/test-amp-story-progress-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {expect} from 'chai';

import {toggleExperiment} from '#experiments';
import {forceExperimentBranch} from '#experiments';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

import {Services} from '#service';

Expand Down Expand Up @@ -45,7 +46,11 @@ describes.realWin('amp-story-progress-bar', {amp: true}, (env) => {

describe('story ad progress segment', async () => {
it('should create progress bar with ad pages', () => {
toggleExperiment(win, 'story-ad-auto-advance', true);
forceExperimentBranch(
win,
StoryAdSegmentExp.ID,
StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA
);
expect(doc.querySelector('.i-amphtml-story-ad-progress-value')).not.to
.exist;
progressBar.activeSegmentId_ = 'page-3';
Expand Down
11 changes: 11 additions & 0 deletions src/experiments/story-ad-progress-segment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @const
* @type {Object<string, string>}
*/
export const StoryAdSegmentExp = {
ID: 'story-ad-auto-advance',
CONTROL: '31067116', // No change
AUTO_ADVANCE_OLD_CTA: '31067117', // Auto advance, old CTA button
AUTO_ADVANCE_NEW_CTA: '31067118', // Auto advance, new CTA button
AUTO_ADVANCE_NEW_CTA_NOT_ANIMATED: '31067119', // Auto advance, new CTA button but not animated
};