Skip to content

Commit

Permalink
Revert "✨ Implement sticky ad bottom type ad on amp-ad (ampproject#31491
Browse files Browse the repository at this point in the history
)"

This reverts commit 5f43080.
  • Loading branch information
jridgewell committed Jan 19, 2021
1 parent 6b95dbb commit 366302c
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 216 deletions.
3 changes: 1 addition & 2 deletions build-system/global-configs/canary-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@
"visibility-trigger-improvements": 1,
"fie-resources": 1,
"ads-initialIntersection": 1,
"amp-cid-backup": 1,
"sticky-ad-transition": 0.1
"amp-cid-backup": 1
}
3 changes: 1 addition & 2 deletions build-system/global-configs/prod-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@
"intersect-resources": 0,
"ios-fixed-no-transfer": 0,
"fie-resources": 1,
"visibility-trigger-improvements": 1,
"sticky-ad-transition": 0.1
"visibility-trigger-improvements": 1
}
4 changes: 1 addition & 3 deletions build-system/tasks/runtime-test/helpers-unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ function extractCssJsFileMap() {
*/
function getImports(jsFile) {
const jsFileContents = fs.readFileSync(jsFile, 'utf8');
const {imports} = listImportsExports.parse(jsFileContents, [
'importAssertions',
]);
const {imports} = listImportsExports.parse(jsFileContents);
const files = [];
const jsFileDir = path.dirname(jsFile);
imports.forEach(function (file) {
Expand Down
15 changes: 3 additions & 12 deletions extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ import {DetachedDomStream} from '../../../src/utils/detached-dom-stream';
import {DomTransformStream} from '../../../src/utils/dom-tranform-stream';
import {GEO_IN_GROUP} from '../../amp-geo/0.1/amp-geo-in-group';
import {Layout, LayoutPriority, isLayoutSizeDefined} from '../../../src/layout';
import {
STICKY_AD_TRANSITION_EXP,
divertStickyAdTransition,
} from '../../../src/experiments/sticky-ad-transition-exp';
import {Services} from '../../../src/services';
import {SignatureVerifier, VerificationStatus} from './signature-verifier';
import {
Expand Down Expand Up @@ -55,7 +51,6 @@ import {
getConsentPolicyState,
} from '../../../src/consent';
import {getContextMetadata} from '../../../src/iframe-attributes';
import {getExperimentBranch, isExperimentOn} from '../../../src/experiments';
import {getMode} from '../../../src/mode';
import {insertAnalyticsElement} from '../../../src/extension-analytics';
import {
Expand All @@ -69,6 +64,7 @@ import {
} from '../../../src/utils/intersection';
import {isAdPositionAllowed} from '../../../src/ad-helper';
import {isArray, isEnumValue, isObject} from '../../../src/types';
import {isExperimentOn} from '../../../src/experiments';
import {listenOnce} from '../../../src/event-helper';
import {
observeWithSharedInOb,
Expand Down Expand Up @@ -1768,6 +1764,7 @@ export class AmpA4A extends AMP.BaseElement {
height,
width
);
this.applyFillContent(this.iframe);

let body = '';
const transferComplete = new Deferred();
Expand Down Expand Up @@ -1857,13 +1854,7 @@ export class AmpA4A extends AMP.BaseElement {
'title': this.getIframeTitle(),
})
));
divertStickyAdTransition(this.win);
if (
getExperimentBranch(this.win, STICKY_AD_TRANSITION_EXP.id) !==
STICKY_AD_TRANSITION_EXP.experiment
) {
this.applyFillContent(this.iframe);
}
this.applyFillContent(this.iframe);
const fontsArray = [];
if (creativeMetaData.customStylesheets) {
creativeMetaData.customStylesheets.forEach((s) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {
maybeAppendErrorParameter,
} from '../../../ads/google/a4a/utils';
import {ResponsiveState} from './responsive-state';
import {STICKY_AD_TRANSITION_EXP} from '../../../src/experiments/sticky-ad-transition-exp';
import {Services} from '../../../src/services';
import {
addAmpExperimentIdToElement,
Expand Down Expand Up @@ -227,16 +226,7 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
* @visibleForTesting
*/
divertExperiments() {
const experimentInfoList = /** @type {!Array<!../../../src/experiments.ExperimentInfo>} */ ([
{
experimentId: STICKY_AD_TRANSITION_EXP.id,
isTrafficEligible: () => true,
branches: [
STICKY_AD_TRANSITION_EXP.control,
STICKY_AD_TRANSITION_EXP.experiment,
],
},
]);
const experimentInfoList = /** @type {!Array<!../../../src/experiments.ExperimentInfo>} */ ([]);
const setExps = randomlySelectUnsetExperiments(
this.win,
experimentInfoList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import {
RefreshManager, // eslint-disable-line no-unused-vars
getRefreshManager,
} from '../../amp-a4a/0.1/refresh-manager';
import {STICKY_AD_TRANSITION_EXP} from '../../../src/experiments/sticky-ad-transition-exp';
import {SafeframeHostApi} from './safeframe-host';
import {Services} from '../../../src/services';
import {
Expand Down Expand Up @@ -107,7 +106,6 @@ import {
} from '../../../src/experiments';
import {getMode} from '../../../src/mode';
import {getMultiSizeDimensions} from '../../../ads/google/utils';

import {getOrCreateAdCid} from '../../../src/ad-cid';

import {getPageLayoutBoxBlocking} from '../../../src/utils/page-layout-box';
Expand Down Expand Up @@ -485,14 +483,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
},
branches: Object.values(IDLE_CWV_EXP_BRANCHES),
},
{
experimentId: STICKY_AD_TRANSITION_EXP.id,
isTrafficEligible: () => true,
branches: [
STICKY_AD_TRANSITION_EXP.control,
STICKY_AD_TRANSITION_EXP.experiment,
],
},
]);
const setExps = this.randomlySelectUnsetExperiments_(experimentInfoList);
Object.keys(setExps).forEach(
Expand Down
92 changes: 19 additions & 73 deletions extensions/amp-ad/0.1/amp-ad-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,21 @@
* limitations under the License.
*/

import {
STICKY_AD_TRANSITION_EXP,
divertStickyAdTransition,
} from '../../../src/experiments/sticky-ad-transition-exp';
import {Services} from '../../../src/services';
import {
ancestorElementsByTag,
createElementWithAttributes,
removeElement,
} from '../../../src/dom';
import {devAssert} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {getExperimentBranch} from '../../../src/experiments';

import {getAdContainer} from '../../../src/ad-helper';
import {listen} from '../../../src/event-helper';
import {setStyle, setStyles} from '../../../src/style';
import {setStyles} from '../../../src/style';

const STICKY_AD_MAX_SIZE_LIMIT = 0.2;
const STICKY_AD_MAX_HEIGHT_LIMIT = 0.5;

/**
* Permissible sticky ad options.
* @const @enum {string}
*/
const StickyAdPositions = {
TOP: 'top',
BOTTOM: 'bottom',
BOTTOM_RIGHT: 'bottom-right',
};

const STICKY_AD_PROP = 'sticky';

export class AmpAdUIHandler {
/**
* @param {!AMP.BaseElement} baseInstance
Expand All @@ -64,18 +46,10 @@ export class AmpAdUIHandler {
this.containerElement_ = null;

/**
* If this is a sticky ad unit, the sticky position option.
* @private {?StickyAdPositions}
* Whether this is a sticky ad unit.
* @private {boolean}
*/
this.stickyAdPosition_ = null;
if (this.element_.hasAttribute(STICKY_AD_PROP)) {
// TODO(powerivq@) Kargo is currently running an experiment using empty sticky attribute, so
// we default the position to bottom right. Remove this default afterwards.
this.stickyAdPosition_ =
this.element_.getAttribute(STICKY_AD_PROP) ||
StickyAdPositions.BOTTOM_RIGHT;
this.element_.setAttribute(STICKY_AD_PROP, this.stickyAdPosition_);
}
this.isStickyAd_ = this.element_.hasAttribute('sticky');

/**
* Whether the close button has been rendered for a sticky ad unit.
Expand Down Expand Up @@ -204,26 +178,22 @@ export class AmpAdUIHandler {
* @return {boolean}
*/
isStickyAd() {
return this.stickyAdPosition_ !== null;
return this.isStickyAd_;
}

/**
* Initialize sticky ad related features
*/
maybeInitStickyAd() {
if (this.isStickyAd()) {
setStyle(this.element_, 'visibility', 'visible');

if (this.stickyAdPosition_ == StickyAdPositions.BOTTOM) {
const paddingBar = this.doc_.createElement('amp-ad-sticky-padding');
this.element_.insertBefore(
paddingBar,
devAssert(
this.element_.firstChild,
'amp-ad should have been expanded.'
)
);
}
if (this.isStickyAd_) {
setStyles(this.element_, {
position: 'fixed',
bottom: '0',
right: '0',
visibility: 'visible',
});

this.element_.classList.add('i-amphtml-amp-ad-sticky-layout');
}
}

Expand All @@ -232,7 +202,7 @@ export class AmpAdUIHandler {
* @return {Promise}
*/
getScrollPromiseForStickyAd() {
if (this.isStickyAd()) {
if (this.isStickyAd_) {
return new Promise((resolve) => {
const unlisten = Services.viewportForDoc(
this.element_.getAmpDoc()
Expand All @@ -249,7 +219,7 @@ export class AmpAdUIHandler {
* When a sticky ad is shown, the close button should be rendered at the same time.
*/
onResizeSuccess() {
if (this.isStickyAd() && !this.closeButtonRendered_) {
if (this.isStickyAd_ && !this.closeButtonRendered_) {
this.addCloseButton_();
this.closeButtonRendered_ = true;
}
Expand Down Expand Up @@ -329,10 +299,10 @@ export class AmpAdUIHandler {
}

// Special case: for sticky ads, we enforce 20% size limit and 50% height limit
if (this.isStickyAd()) {
if (this.element_.hasAttribute('sticky')) {
const viewport = this.baseInstance_.getViewport();
if (
height * width >
newHeight * newWidth >
STICKY_AD_MAX_SIZE_LIMIT *
viewport.getHeight() *
viewport.getWidth() ||
Expand All @@ -345,38 +315,14 @@ export class AmpAdUIHandler {
return this.baseInstance_
.attemptChangeSize(newHeight, newWidth, event)
.then(
() => {
divertStickyAdTransition(this.baseInstance_.win);
if (
getExperimentBranch(
this.baseInstance_.win,
STICKY_AD_TRANSITION_EXP.id
) === STICKY_AD_TRANSITION_EXP.experiment
) {
this.setSize_(this.element_.querySelector('iframe'), height, width);
}
return resizeInfo;
},
() => resizeInfo,
() => {
resizeInfo.success = false;
return resizeInfo;
}
);
}

/**
* Force set the dimensions for an element
* @param {Any} element
* @param {number} newHeight
* @param {number} newWidth
*/
setSize_(element, newHeight, newWidth) {
setStyles(element, {
'height': `${newHeight}px`,
'width': `${newWidth}px`,
});
}

/**
* Clean up the listeners
*/
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export class AmpAdXOriginIframeHandler {
devAssert(!this.iframe, 'multiple invocations of init without destroy!');
this.iframe = iframe;
this.iframe.setAttribute('scrolling', 'no');
this.baseInstance_.applyFillContent(this.iframe);
const timer = Services.timerFor(this.baseInstance_.win);

// Init the legacy observeInterection API service.
Expand Down
44 changes: 4 additions & 40 deletions extensions/amp-ad/0.1/amp-ad.css
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ amp-embed iframe {

amp-ad[sticky] {
visibility: hidden;
align-items: center;
}

amp-ad[type='adsense'],
Expand All @@ -77,6 +76,10 @@ amp-ad[data-a4a-upgrade-type='amp-ad-network-doubleclick-impl'][height='fluid']
width: 100% !important;
}

amp-ad.i-amphtml-amp-ad-sticky-layout {
overflow: visible !important;
}

amp-ad .amp-ad-close-button {
position: absolute;
visibility: visible;
Expand Down Expand Up @@ -116,42 +119,3 @@ amp-ad .amp-ad-close-button:before {
right: -20px;
left: 0;
}

amp-ad-sticky-padding {
display: block;
width: 100% !important;
background: #fff;
height: 4px;
max-height: 5px !important;
overflow-x: hidden;
overflow-y: hidden;
/** Must be above the dismiss button to cover bottom shadow */
z-index: 12;
}

amp-ad[sticky] {
z-index: 11;
position: fixed;
overflow: visible !important;
padding-bottom: env(safe-area-inset-bottom, 0px);
box-shadow: 0 0 5px 0 rgba(0, 0, 0, 0.2) !important;
display: flex;
flex-direction: column;
}

amp-ad[sticky='top'] {
width: 100% !important;
background: #fff;
top: 0;
}

amp-ad[sticky='bottom'] {
width: 100% !important;
background: #fff;
bottom: 0;
}

amp-ad[sticky='bottom-right'] {
bottom: 0;
right: 0;
}
2 changes: 1 addition & 1 deletion extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ describes.realWin(

describe('during layout', () => {
it('sticky ad: should not layout w/o scroll', () => {
ad3p.uiHandler.stickyAdPosition_ = 'bottom';
ad3p.uiHandler.isStickyAd_ = true;
expect(ad3p.xOriginIframeHandler_).to.be.null;
const layoutPromise = ad3p.layoutCallback();
return Promise.race([macroTask(), layoutPromise])
Expand Down
Loading

0 comments on commit 366302c

Please sign in to comment.