✨ Implement sticky ad bottom type ad on amp-ad (ampproject#31491)

* Add 3 sticky ad positions to amp-ad * Implement sticky ad bottom type ad * Updates * Upgrade @babel/parser to support import assertions Not sure why this version is stale. v 7.11 uses `with` syntax, which was changed to `assert` syntax. * Fix style Co-authored-by: Justin Ridgewell <jridgewell@google.com>
mohammed-ibra · Jan 7, 2021 · 5f43080 · 5f43080
1 parent d20f097
commit 5f43080
Show file tree

Hide file tree

Showing 14 changed files with 216 additions and 42 deletions.
diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -22,5 +22,6 @@
   "build-in-chunks": 1,
   "visibility-trigger-improvements": 1,
   "fie-resources": 1,
-  "amp-cid-backup": 1
+  "amp-cid-backup": 1,
+  "sticky-ad-transition": 0.1
 }
diff --git a/build-system/global-configs/prod-config.json b/build-system/global-configs/prod-config.json
@@ -18,5 +18,6 @@
   "ios-fixed-no-transfer": 0,
   "pump-early-frame": 1,
   "fie-resources": 0.1,
-  "visibility-trigger-improvements": 1
+  "visibility-trigger-improvements": 1,
+  "sticky-ad-transition": 0.1
 }
diff --git a/build-system/tasks/runtime-test/helpers-unit.js b/build-system/tasks/runtime-test/helpers-unit.js
@@ -87,7 +87,9 @@ function extractCssJsFileMap() {
  */
 function getImports(jsFile) {
   const jsFileContents = fs.readFileSync(jsFile, 'utf8');
-  const {imports} = listImportsExports.parse(jsFileContents);
+  const {imports} = listImportsExports.parse(jsFileContents, [
+    'importAssertions',
+  ]);
   const files = [];
   const jsFileDir = path.dirname(jsFile);
   imports.forEach(function (file) {

diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js
@@ -21,6 +21,10 @@ 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 {
@@ -51,6 +55,7 @@ import {
   getConsentPolicyState,
 } from '../../../src/consent';
 import {getContextMetadata} from '../../../src/iframe-attributes';
+import {getExperimentBranch} from '../../../src/experiments';
 import {getMode} from '../../../src/mode';
 import {insertAnalyticsElement} from '../../../src/extension-analytics';
 import {
@@ -1759,7 +1764,6 @@ export class AmpA4A extends AMP.BaseElement {
       height,
       width
     );
-    this.applyFillContent(this.iframe);
 
     let body = '';
     const transferComplete = new Deferred();
@@ -1848,7 +1852,13 @@ export class AmpA4A extends AMP.BaseElement {
         'title': this.getIframeTitle(),
       })
     ));
-    this.applyFillContent(this.iframe);
+    divertStickyAdTransition(this.win);
+    if (
+      getExperimentBranch(this.win, STICKY_AD_TRANSITION_EXP.id) !==
+      STICKY_AD_TRANSITION_EXP.experiment
+    ) {
+      this.applyFillContent(this.iframe);
+    }
     const fontsArray = [];
     if (creativeMetaData.customStylesheets) {
       creativeMetaData.customStylesheets.forEach((s) => {

diff --git a/extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js b/extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
@@ -44,6 +44,7 @@ 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,
@@ -226,7 +227,16 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
    * @visibleForTesting
    */
   divertExperiments() {
-    const experimentInfoList = /** @type {!Array<!../../../src/experiments.ExperimentInfo>} */ ([]);
+    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 setExps = randomlySelectUnsetExperiments(
       this.win,
       experimentInfoList

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
@@ -64,6 +64,7 @@ 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 {
@@ -106,6 +107,7 @@ 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';
@@ -483,6 +485,14 @@ 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(

diff --git a/extensions/amp-ad/0.1/amp-ad-ui.js b/extensions/amp-ad/0.1/amp-ad-ui.js
@@ -14,21 +14,39 @@
  * 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 {setStyles} from '../../../src/style';
+import {setStyle, 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
@@ -46,10 +64,18 @@ export class AmpAdUIHandler {
     this.containerElement_ = null;
 
     /**
-     * Whether this is a sticky ad unit.
-     * @private {boolean}
+     * If this is a sticky ad unit, the sticky position option.
+     * @private {?StickyAdPositions}
      */
-    this.isStickyAd_ = this.element_.hasAttribute('sticky');
+    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_);
+    }
 
     /**
      * Whether the close button has been rendered for a sticky ad unit.
@@ -178,22 +204,26 @@ export class AmpAdUIHandler {
    * @return {boolean}
    */
   isStickyAd() {
-    return this.isStickyAd_;
+    return this.stickyAdPosition_ !== null;
   }
 
   /**
    * Initialize sticky ad related features
    */
   maybeInitStickyAd() {
-    if (this.isStickyAd_) {
-      setStyles(this.element_, {
-        position: 'fixed',
-        bottom: '0',
-        right: '0',
-        visibility: 'visible',
-      });
-
-      this.element_.classList.add('i-amphtml-amp-ad-sticky-layout');
+    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.'
+          )
+        );
+      }
     }
   }
 
@@ -202,7 +232,7 @@ export class AmpAdUIHandler {
    * @return {Promise}
    */
   getScrollPromiseForStickyAd() {
-    if (this.isStickyAd_) {
+    if (this.isStickyAd()) {
       return new Promise((resolve) => {
         const unlisten = Services.viewportForDoc(
           this.element_.getAmpDoc()
@@ -219,7 +249,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;
     }
@@ -299,10 +329,10 @@ export class AmpAdUIHandler {
     }
 
     // Special case: for sticky ads, we enforce 20% size limit and 50% height limit
-    if (this.element_.hasAttribute('sticky')) {
+    if (this.isStickyAd()) {
       const viewport = this.baseInstance_.getViewport();
       if (
-        newHeight * newWidth >
+        height * width >
           STICKY_AD_MAX_SIZE_LIMIT *
             viewport.getHeight() *
             viewport.getWidth() ||
@@ -315,14 +345,38 @@ export class AmpAdUIHandler {
     return this.baseInstance_
       .attemptChangeSize(newHeight, newWidth, event)
       .then(
-        () => resizeInfo,
+        () => {
+          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.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
    */

diff --git a/extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js b/extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js
@@ -100,7 +100,6 @@ 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.

diff --git a/extensions/amp-ad/0.1/amp-ad.css b/extensions/amp-ad/0.1/amp-ad.css
@@ -52,6 +52,7 @@ amp-embed iframe {
 
 amp-ad[sticky] {
   visibility: hidden;
+  align-items: center;
 }
 
 amp-ad[type='adsense'],
@@ -76,10 +77,6 @@ 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;
@@ -119,3 +116,42 @@ 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;
+}
diff --git a/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js b/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js
@@ -289,7 +289,7 @@ describes.realWin(
 
       describe('during layout', () => {
         it('sticky ad: should not layout w/o scroll', () => {
-          ad3p.uiHandler.isStickyAd_ = true;
+          ad3p.uiHandler.stickyAdPosition_ = 'bottom';
           expect(ad3p.xOriginIframeHandler_).to.be.null;
           const layoutPromise = ad3p.layoutCallback();
           return Promise.race([macroTask(), layoutPromise])