Skip to content

Commit

Permalink
🗑 Remove story-ad-progress experiment (#37493)
Browse files Browse the repository at this point in the history 
* delete a bunch of code

* z-index

* fix test
  • Loading branch information
@calebcordry
calebcordry committed Feb 4, 2022
1 parent af7aa61 commit 863b55a
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 245 deletions.
1 change: 0 additions & 1 deletion build-system/global-configs/canary-config.json
View file
Expand Up @@ -12,7 +12,6 @@
"ios-fixed-no-transfer": 1,
"ads-initialIntersection": 1,
"flexible-bitrate": 0.1,
"story-ad-auto-advance": 1,
"story-ad-placements": 1,
"story-load-first-page-only": 1,
"story-load-inactive-outside-viewport": 1,
Expand Down
1 change: 0 additions & 1 deletion css/Z_INDEX.md
View file
Expand Up @@ -41,7 +41,6 @@
| `.i-amphtml-story-has-new-page-notification-container` | 100002 | [extensions/amp-story/1.0/amp-story-system-layer.css](/extensions/amp-story/1.0/amp-story-system-layer.css) |
| `.i-amphtml-story-button-container` | 100002 | [extensions/amp-story/1.0/pagination-buttons.css](/extensions/amp-story/1.0/pagination-buttons.css) |
| `.i-amphtml-ad-overlay-container` | 100001 | [extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-ad-badge.css](/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-ad-badge.css) |
| `.i-amphtml-story-ad-progress-background` | 100001 | [extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-progress-bar.css](/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-progress-bar.css) |
| `.i-amphtml-story-info-dialog` | 100001 | [extensions/amp-story/1.0/amp-story-info-dialog.css](/extensions/amp-story/1.0/amp-story-info-dialog.css) |
| `.i-amphtml-story-progress-bar` | 100001 | [extensions/amp-story/1.0/amp-story-system-layer.css](/extensions/amp-story/1.0/amp-story-system-layer.css) |
| `.i-amphtml-story-focused-state-layer` | 100001 | [extensions/amp-story/1.0/amp-story-tooltip.css](/extensions/amp-story/1.0/amp-story-tooltip.css) |
Expand Down
9 changes: 0 additions & 9 deletions extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
View file
Expand Up @@ -42,7 +42,6 @@ import {
getExperimentBranch,
randomlySelectUnsetExperiments,
} from '#experiments';
import {StoryAdAutoAdvance} from '#experiments/story-ad-auto-advance';
import {StoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

Expand Down Expand Up @@ -246,14 +245,6 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
addExperimentIdToElement(storyAdPlacementsExpId, this.element);
}

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

const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
Expand Down
9 changes: 0 additions & 9 deletions extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
View file
Expand Up @@ -67,7 +67,6 @@ import {
isExperimentOn,
randomlySelectUnsetExperiments,
} from '#experiments';
import {StoryAdAutoAdvance} from '#experiments/story-ad-auto-advance';
import {StoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

Expand Down Expand Up @@ -498,14 +497,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
addExperimentIdToElement(storyAdPlacementsExpId, this.element);
}

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

const storyAdSegmentBranch = getExperimentBranch(
this.win,
StoryAdSegmentExp.ID
Expand Down
46 changes: 0 additions & 46 deletions extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-progress-bar.css
View file

This file was deleted.

84 changes: 1 addition & 83 deletions extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.js
View file
@@ -1,13 +1,6 @@
import {CommonSignals_Enum} from '#core/constants/common-signals';
import {toggleAttribute} from '#core/dom';
import {setStyle} from '#core/dom/style';

import {forceExperimentBranch, getExperimentBranch} from '#experiments';
import {
AdvanceExpToTime,
StoryAdAutoAdvance,
divertStoryAdAutoAdvance,
} from '#experiments/story-ad-auto-advance';
import {forceExperimentBranch} from '#experiments';
import {divertStoryAdPlacements} from '#experiments/story-ad-placements';
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';

Expand All @@ -27,7 +20,6 @@ import {StoryAdPageManager} from './story-ad-page-manager';

import {CSS} from '../../../build/amp-story-auto-ads-0.1.css';
import {CSS as adBadgeCSS} from '../../../build/amp-story-auto-ads-ad-badge-0.1.css';
import {CSS as progessBarCSS} from '../../../build/amp-story-auto-ads-progress-bar-0.1.css';
import {CSS as sharedCSS} from '../../../build/amp-story-auto-ads-shared-0.1.css';
import {getServicePromiseForDoc} from '../../../src/service-helpers';
import {
Expand Down Expand Up @@ -93,9 +85,6 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
/** @private {?Element} */
this.adBadgeContainer_ = null;

/** @private {?Element} */
this.progressBarBackground_ = null;

/** @private {?../../amp-story/1.0/amp-story-store-service.AmpStoryStoreService} */
this.storeService_ = null;

Expand Down Expand Up @@ -156,7 +145,6 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
this.config_
);
divertStoryAdPlacements(this.win);
divertStoryAdAutoAdvance(this.win);
this.placementAlgorithm_ = getPlacementAlgo(
this.win,
this.storeService_,
Expand All @@ -171,7 +159,6 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
STORY_AD_ANALYTICS
);
this.createAdOverlay_();
this.maybeCreateProgressBar_();
this.initializeListeners_();
this.initializePages_();
});
Expand Down Expand Up @@ -323,14 +310,8 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
this.mutateElement(() => {
if (isAd) {
this.adBadgeContainer_.setAttribute(Attributes.AD_SHOWING, '');
// TODO(#33969) can no longer be null when launched.
this.progressBarBackground_ &&
this.progressBarBackground_.setAttribute(Attributes.AD_SHOWING, '');
} else {
this.adBadgeContainer_.removeAttribute(Attributes.AD_SHOWING);
// TODO(#33969) can no longer be null when launched.
this.progressBarBackground_ &&
this.progressBarBackground_.removeAttribute(Attributes.AD_SHOWING);
}
});
}
Expand Down Expand Up @@ -359,15 +340,12 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
const {DESKTOP_FULLBLEED, DESKTOP_ONE_PANEL} = Attributes;
this.adBadgeContainer_.removeAttribute(DESKTOP_FULLBLEED);
this.adBadgeContainer_.removeAttribute(DESKTOP_ONE_PANEL);
// TODO(#33969) can no longer be null when launched.
this.progressBarBackground_?.removeAttribute(DESKTOP_ONE_PANEL);

if (uiState === UIType.DESKTOP_FULLBLEED) {
this.adBadgeContainer_.setAttribute(DESKTOP_FULLBLEED, '');
}
if (uiState === UIType.DESKTOP_ONE_PANEL) {
this.adBadgeContainer_.setAttribute(DESKTOP_ONE_PANEL, '');
this.progressBarBackground_?.setAttribute(DESKTOP_ONE_PANEL, '');
}
});
}
Expand All @@ -392,66 +370,6 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
this.ampStory_.element.appendChild(root);
}

/**
* Create progress bar if auto advance exp is on.
* TODO(#33969) move to chosen UI and delete the others.
*/
maybeCreateProgressBar_() {
const autoAdvanceExpBranch = getExperimentBranch(
this.win,
StoryAdAutoAdvance.ID
);
if (getExperimentBranch(this.win, StoryAdSegmentExp.ID)) {
// In the viewer controlled experiment progress bar is created by
// progress-bar.js
return;
} else if (
autoAdvanceExpBranch &&
autoAdvanceExpBranch !== StoryAdAutoAdvance.CONTROL
) {
this.createProgressBar_(AdvanceExpToTime[autoAdvanceExpBranch]);
}
}

/**
* Create progress bar that will be shown when ad is advancing.
* @param {string} time
*/
createProgressBar_(time) {
const progressBar = this.doc_.createElement('div');
progressBar.className = 'i-amphtml-story-ad-progress-bar';
setStyle(progressBar, 'animationDuration', time);

this.progressBarBackground_ = this.doc_.createElement('div');
this.progressBarBackground_.className =
'i-amphtml-story-ad-progress-background';

const host = this.doc_.createElement('div');
host.className = 'i-amphtml-story-ad-progress-bar-host';

this.progressBarBackground_.appendChild(progressBar);
createShadowRootWithStyle(host, this.progressBarBackground_, progessBarCSS);
this.ampStory_.element.appendChild(host);

// TODO(#33969) move this to init listeners when no longer conditional.
this.storeService_.subscribe(StateProperty.PAUSED_STATE, (isPaused) => {
this.onPauseStateUpdate_(isPaused);
});
}

/**
* If video is paused and ad is showing pause the progress bar.
* @param {boolean} isPaused
*/
onPauseStateUpdate_(isPaused) {
const adShowing = this.storeService_.get(StateProperty.AD_STATE);
if (!adShowing) {
return;
}

toggleAttribute(this.progressBarBackground_, Attributes.PAUSED, isPaused);
}

/**
* Create new page containing ad and start preloading.
* @private
Expand Down
56 changes: 2 additions & 54 deletions extensions/amp-story-auto-ads/0.1/test/test-amp-story-auto-ads.js
View file
Expand Up @@ -2,7 +2,6 @@ import {CommonSignals_Enum} from '#core/constants/common-signals';

import * as experiments from '#experiments';
import {forceExperimentBranch, getExperimentBranch} from '#experiments';
import {StoryAdAutoAdvance} from '#experiments/story-ad-auto-advance';

import {Services} from '#service';

Expand Down Expand Up @@ -320,11 +319,6 @@ describes.realWin(
describe('system layer', () => {
beforeEach(async () => {
// TODO(#33969) remove when launched.
forceExperimentBranch(
win,
'story-ad-auto-advance',
StoryAdAutoAdvance.EIGHT_SECONDS
);
// Force sync mutateElement.
env.sandbox.stub(autoAds, 'mutateElement').callsArg(0);
addStoryAutoAdsConfig(adElement);
Expand All @@ -341,48 +335,27 @@ describes.realWin(
expect(adBadge).to.exist;
});

it('should create progress bar', () => {
const progressBar = doc.querySelector(
'.i-amphtml-story-ad-progress-bar'
);
expect(progressBar).to.exist;
});

it('should propagate the ad-showing attribute to badge & progress bar', () => {
it('should propagate the ad-showing attribute to badge', () => {
const adBadgeContainer = doc.querySelector(
'.i-amphtml-ad-overlay-container'
);
const progressBackground = doc.querySelector(
'.i-amphtml-story-ad-progress-background'
);
expect(adBadgeContainer).not.to.have.attribute(Attributes.AD_SHOWING);
expect(progressBackground).not.to.have.attribute(Attributes.AD_SHOWING);
storeService.dispatch(Action.TOGGLE_AD, true);
expect(adBadgeContainer).to.have.attribute(Attributes.AD_SHOWING);
expect(progressBackground).to.have.attribute(Attributes.AD_SHOWING);
});

it('should propagate the desktop-one-panel attribute to badge & progress bar', () => {
it('should propagate the desktop-one-panel attribute to ad badge', () => {
const adBadgeContainer = doc.querySelector(
'.i-amphtml-ad-overlay-container'
);
const progressBackground = doc.querySelector(
'.i-amphtml-story-ad-progress-background'
);
storeService.dispatch(Action.TOGGLE_UI, UIType.MOBILE);
expect(adBadgeContainer).not.to.have.attribute(
Attributes.DESKTOP_ONE_PANEL
);
expect(progressBackground).not.to.have.attribute(
Attributes.DESKTOP_ONE_PANEL
);
storeService.dispatch(Action.TOGGLE_UI, UIType.DESKTOP_ONE_PANEL);
expect(adBadgeContainer).to.have.attribute(
Attributes.DESKTOP_ONE_PANEL
);
expect(progressBackground).to.have.attribute(
Attributes.DESKTOP_ONE_PANEL
);
});

it('should propagate the dir=rtl attribute', () => {
Expand All @@ -393,31 +366,6 @@ describes.realWin(
storeService.dispatch(Action.TOGGLE_RTL, true);
expect(adBadgeContainer).to.have.attribute(Attributes.DIR, 'rtl');
});

it('should propagate the pause state if ad showing', () => {
const progressBackground = doc.querySelector(
'.i-amphtml-story-ad-progress-background'
);
storeService.dispatch(Action.TOGGLE_AD, true);
expect(progressBackground).not.to.have.attribute(Attributes.PAUSED);
storeService.dispatch(Action.TOGGLE_PAUSED, true);
expect(progressBackground).to.have.attribute(Attributes.PAUSED);
storeService.dispatch(Action.TOGGLE_PAUSED, false);
expect(progressBackground).not.to.have.attribute(Attributes.PAUSED);
});

// TODO(calebcordry): Skipping test since it's failing on main, marking for review.
it.skip('should not propagate the pause state if no ad showing', () => {
const progressBackground = doc.querySelector(
'.i-amphtml-story-ad-progress-background'
);
storeService.dispatch(Action.TOGGLE_AD, false);
expect(progressBackground).not.to.have.attribute(Attributes.PAUSED);
storeService.dispatch(Action.TOGGLE_PAUSED, true);
expect(progressBackground).not.to.have.attribute(Attributes.PAUSED);
storeService.dispatch(Action.TOGGLE_PAUSED, false);
expect(progressBackground).not.to.have.attribute(Attributes.PAUSED);
});
});

describe('analytics triggers', () => {
Expand Down

0 comments on commit 863b55a

Please sign in to comment.