Added support for a new trigger on=hidden. (#4265)

* Added support for a new trigger `on=hidden`. The new trigger fires when the viewer is hidden. In addition, visibilitySpec can be used to add additional conditions on when the trigger fires. * variable rename.
ampproject · Aug 8, 2016 · 572c687 · 572c687
1 parent b4ea647
commit 572c687
Show file tree

Hide file tree

Showing 6 changed files with 185 additions and 113 deletions.
diff --git a/examples/analytics.amp.html b/examples/analytics.amp.html
@@ -36,7 +36,7 @@
     "base": "https://example.com/?domain=${canonicalHost}&path=${canonicalPath}&title=${title}&time=${timestamp}&tz=${timezone}&pid=${pageViewId}&_=${random}",
     "pageview": "${base}&name=${eventName}&type=${eventId}&screenSize=${screenWidth}x${screenHeight}",
     "event": "${base}&name=${eventName}&scrollY=${scrollTop}&scrollX=${scrollLeft}&height=${availableScreenHeight}&width=${availableScreenWidth}&scrollBoundV=${verticalScrollBoundary}&scrollBoundH=${horizontalScrollBoundary}",
-    "visibility": "https://example.com/visibility?a=${maxContinuousTime}&b=${totalVisibleTime}&c=${firstSeenTime}&d=${lastSeenTime}&e=${fistVisibleTime}&f=${lastVisibleTime}&g=${minVisiblePercentage}&h=${maxVisiblePercentage}&i=${elementX}&j=${elementY}&k=${elementWidth}&l=${elementHeight}&m=${totalTime}&n=${loadTimeVisibility}&o=${backgroundedAtStart}&p=${backgrounded}&eventName=${eventName}&subTitle=${subTitle}"
+    "visibility": "https://example.com/visibility?a=${maxContinuousVisibleTime}&b=${totalVisibleTime}&c=${firstSeenTime}&d=${lastSeenTime}&e=${fistVisibleTime}&f=${lastVisibleTime}&g=${minVisiblePercentage}&h=${maxVisiblePercentage}&i=${elementX}&j=${elementY}&k=${elementWidth}&l=${elementHeight}&m=${totalTime}&n=${loadTimeVisibility}&o=${backgroundedAtStart}&p=${backgrounded}&eventName=${eventName}&subTitle=${subTitle}"
   },
   "vars": {
     "title": "Example Request"
@@ -74,6 +74,16 @@
         "eventName": "visibility event"
       }
     },
+    "hidden": {
+      "on": "hidden",
+      "request": "visibility",
+      "visibilitySpec": {
+        "selector": "#anim-id",
+        "visiblePercentageMin": 20,
+        "continuousTimeMax": 5000
+      },
+      "extraUrlParams": { "on": "hidden"}
+    },
     "scrollPings": {
       "on": "scroll",
       "request": "event",

diff --git a/extensions/amp-analytics/0.1/instrumentation.js b/extensions/amp-analytics/0.1/instrumentation.js
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import {dev} from '../../../src/log';
 import {isVisibilitySpecValid} from './visibility-impl';
 import {Observable} from '../../../src/observable';
 import {fromClass} from '../../../src/service';
@@ -57,6 +58,7 @@ export const AnalyticsEventType = {
   CLICK: 'click',
   TIMER: 'timer',
   SCROLL: 'scroll',
+  HIDDEN: 'hidden',
 };
 
 /**
@@ -133,7 +135,8 @@ export class InstrumentationService {
   addListener(config, listener) {
     const eventType = config['on'];
     if (eventType === AnalyticsEventType.VISIBLE) {
-      this.createVisibilityListener_(listener, config);
+      this.createVisibilityListener_(listener, config,
+          AnalyticsEventType.VISIBLE);
     } else if (eventType === AnalyticsEventType.CLICK) {
       if (!config['selector']) {
         user().error(this.TAG_, 'Missing required selector on click trigger');
@@ -162,6 +165,9 @@ export class InstrumentationService {
       if (this.isTimerSpecValid_(config['timerSpec'])) {
         this.createTimerListener_(listener, config['timerSpec']);
       }
+    } else if (eventType === AnalyticsEventType.HIDDEN) {
+      this.createVisibilityListener_(listener, config,
+          AnalyticsEventType.HIDDEN);
     } else {
       let observers = this.customEventObservers_[eventType];
       if (!observers) {
@@ -214,51 +220,49 @@ export class InstrumentationService {
    * @param {!AnalyticsEventListenerDef} The callback to call when the event
    *   occurs.
    * @param {!JSONType} config Configuration for instrumentation.
+   * @param {AnalyticsEventType} eventType Event type for which the callback is triggered.
    * @private
    */
-  createVisibilityListener_(callback, config) {
+  createVisibilityListener_(callback, config, eventType) {
+    dev().assert(eventType == AnalyticsEventType.VISIBLE ||
+        eventType == AnalyticsEventType.HIDDEN,
+        'createVisibilityListener should be called with visible or hidden ' +
+        'eventType');
+    const shouldBeVisible = eventType == AnalyticsEventType.VISIBLE;
     const spec = config['visibilitySpec'];
     if (spec) {
       if (!isVisibilitySpecValid(config)) {
         return;
       }
-      this.runOrSchedule_(() => {
-        visibilityFor(this.win_).then(visibility => {
-          visibility.listenOnce(spec, vars => {
-            if (spec['selector']) {
-              const attr = getDataParamsFromAttributes(
-                this.win_.document.getElementById(spec['selector'].slice(1)),
-                null,
-                VARIABLE_DATA_ATTRIBUTE_KEY
-              );
-              for (const key in attr) {
-                vars[key] = attr[key];
-              }
+
+      visibilityFor(this.win_).then(visibility => {
+        visibility.listenOnce(spec, vars => {
+          if (spec['selector']) {
+            const attr = getDataParamsFromAttributes(
+              this.win_.document.getElementById(spec['selector'].slice(1)),
+              null,
+              VARIABLE_DATA_ATTRIBUTE_KEY
+            );
+            for (const key in attr) {
+              vars[key] = attr[key];
             }
-            callback(new AnalyticsEvent(AnalyticsEventType.VISIBLE, vars));
-          });
-        });
-      });
-    } else {
-      this.runOrSchedule_(() => {
-        callback(new AnalyticsEvent(AnalyticsEventType.VISIBLE));
+          }
+          callback(new AnalyticsEvent(eventType, vars));
+        }, shouldBeVisible);
       });
-    }
-  }
-
-  /**
-   * @param {function()} fn function to run or schedule.
-   * @private
-   */
-  runOrSchedule_(fn) {
-    if (this.viewer_.isVisible()) {
-      fn();
     } else {
-      this.viewer_.onVisibilityChanged(() => {
-        if (this.viewer_.isVisible()) {
-          fn();
-        }
-      });
+      if (this.viewer_.isVisible() == shouldBeVisible) {
+        callback(new AnalyticsEvent(eventType));
+        config['called'] = true;
+      } else {
+        this.viewer_.onVisibilityChanged(() => {
+          if (!config['called'] &&
+              this.viewer_.isVisible() == shouldBeVisible) {
+            callback(new AnalyticsEvent(eventType));
+            config['called'] = true;
+          }
+        });
+      }
     }
   }
 

diff --git a/extensions/amp-analytics/0.1/test/test-instrumentation.js b/extensions/amp-analytics/0.1/test/test-instrumentation.js
@@ -16,6 +16,7 @@
 
 import {InstrumentationService} from '../instrumentation.js';
 import {adopt} from '../../../../src/runtime';
+import {VisibilityState} from '../../../../src/visibility-state';
 import * as sinon from 'sinon';
 
 adopt(window);
@@ -47,6 +48,19 @@ describe('amp-analytics.instrumentation', function() {
     sandbox.restore();
   });
 
+  it('works for visible event', () => {
+    const fn = sandbox.stub();
+    ins.addListener({'on': 'visible'}, fn);
+    expect(fn.calledOnce).to.be.true;
+  });
+
+  it('works for hidden event', () => {
+    const fn = sandbox.stub();
+    ins.addListener({'on': 'hidden'}, fn);
+    ins.viewer_.setVisibilityState_(VisibilityState.HIDDEN);
+    expect(fn.calledOnce).to.be.true;
+  });
+
   it('always fires click listeners when selector is set to *', () => {
     const el1 = document.createElement('test');
     const fn1 = sandbox.stub();

diff --git a/extensions/amp-analytics/0.1/test/test-visibility-impl.js b/extensions/amp-analytics/0.1/test/test-visibility-impl.js
@@ -19,18 +19,17 @@ import {
   isPositiveNumber_,
   isValidPercentage_,
   isVisibilitySpecValid,
+  Visibility,
 } from '../visibility-impl';
 import {layoutRectLtwh, rectIntersection} from '../../../../src/layout-rect';
-import {visibilityFor} from '../../../../src/visibility';
 import {VisibilityState} from '../../../../src/visibility-state';
+import {viewerFor} from '../../../../src/viewer';
 import * as sinon from 'sinon';
 
 
 adopt(window);
 
-// The tests have amp-analytics tag because they should be run whenever
-// amp-analytics is changed.
-describe('Visibility (tag: amp-analytics)', () => {
+describe('amp-analytics.visibility', () => {
 
   let sandbox;
   let visibility;
@@ -55,14 +54,13 @@ describe('Visibility (tag: amp-analytics)', () => {
     getIntersectionStub = sandbox.stub();
     callbackStub = sandbox.stub();
 
-    return visibilityFor(window).then(v => {
-      visibility = v;
-      sandbox.stub(visibility.resourcesService_,
-        'getResourceForElement').returns({
+    viewerFor(window).setVisibilityState_(VisibilityState.VISIBLE);
+    visibility = new Visibility(window);
+    sandbox.stub(visibility.resourcesService_, 'getResourceForElement')
+        .returns({
           element: {getIntersectionChangeEntry: getIntersectionStub},
           getId: getIdStub,
           isLayoutPending: () => false});
-    });
   });
 
   afterEach(() => {
@@ -79,22 +77,23 @@ describe('Visibility (tag: amp-analytics)', () => {
     };
   }
 
-  function listen(intersectionChange, config, expectedCalls, opt_expectedVars) {
+  function listen(intersectionChange, config, expectedCalls, opt_expectedVars,
+      opt_visible) {
+    opt_visible = opt_visible === undefined ? true : opt_visible;
     getIntersectionStub.returns(intersectionChange);
     config['selector'] = '#abc';
-    visibility.listenOnce(config, callbackStub);
+    visibility.listenOnce(config, callbackStub, opt_visible);
     clock.tick(20);
-    expect(callbackStub.callCount).to.equal(expectedCalls);
-    if (opt_expectedVars && expectedCalls > 0) {
-      for (let c = 0; c < opt_expectedVars.length; c++) {
-        sinon.assert.calledWith(callbackStub.getCall(c), opt_expectedVars[c]);
-      }
-    }
+    verifyExpectedVars(expectedCalls, opt_expectedVars);
   }
 
   function verifyChange(intersectionChange, expectedCalls, opt_expectedVars) {
     getIntersectionStub.returns(intersectionChange);
     visibility.scrollListener_();
+    verifyExpectedVars(expectedCalls, opt_expectedVars);
+  }
+
+  function verifyExpectedVars(expectedCalls, opt_expectedVars) {
     expect(callbackStub.callCount).to.equal(expectedCalls);
     if (opt_expectedVars && expectedCalls > 0) {
       for (let c = 0; c < opt_expectedVars.length; c++) {
@@ -103,12 +102,20 @@ describe('Visibility (tag: amp-analytics)', () => {
     }
   }
 
-  it('fires for trivial config', () => {
+  it('fires for trivial on=visible config', () => {
     listen(INTERSECTION_50P, {
       visiblePercentageMin: 0, visiblePercentageMax: 100}, 1);
   });
 
-  it('fires for non-trivial config', () => {
+  it('fires for trivial on=hidden config', () => {
+    listen(INTERSECTION_50P, {
+      visiblePercentageMin: 0, visiblePercentageMax: 100}, 0, undefined, false);
+
+    visibility.viewer_.setVisibilityState_(VisibilityState.HIDDEN);
+    expect(callbackStub.callCount).to.equal(1);
+  });
+
+  it('fires for non-trivial on=visible config', () => {
     listen(makeIntersectionEntry([51, 0, 100, 100], [0, 0, 100, 100]),
           {visiblePercentageMin: 49, visiblePercentageMax: 80}, 0);
 
@@ -126,6 +133,27 @@ describe('Visibility (tag: amp-analytics)', () => {
     })]);
   });
 
+  it('fires for non-trivial on=hidden config', () => {
+    listen(makeIntersectionEntry([51, 0, 100, 100], [0, 0, 100, 100]),
+          {visiblePercentageMin: 49, visiblePercentageMax: 80}, 0, undefined,
+          false);
+
+    verifyChange(INTERSECTION_50P, 0, undefined);
+    visibility.viewer_.setVisibilityState_(VisibilityState.HIDDEN);
+    verifyExpectedVars(1, [sinon.match({
+      backgrounded: '1',
+      backgroundedAtStart: '0',
+      elementX: '50',
+      elementY: '0',
+      elementWidth: '100',
+      elementHeight: '100',
+      loadTimeVisibility: '50',
+      totalTime: sinon.match(value => {
+        return !isNaN(Number(value));
+      }),
+    })]);
+  });
+
   it('fires only once', () => {
     listen(INTERSECTION_50P, {
       visiblePercentageMin: 49, visiblePercentageMax: 80,
@@ -223,6 +251,7 @@ describe('Visibility (tag: amp-analytics)', () => {
 
   it('populates backgroundedAtStart=0', () => {
     const viewerStub = sandbox.stub(visibility.viewer_, 'getVisibilityState');
+    viewerStub.returns(VisibilityState.VISIBLE);
     visibility.backgroundedAtStart_ = false;
     listen(INTERSECTION_50P, {
       visiblePercentageMin: 0, visiblePercentageMax: 100}, 1, [sinon.match({
@@ -232,6 +261,7 @@ describe('Visibility (tag: amp-analytics)', () => {
 
     viewerStub.returns(VisibilityState.HIDDEN);
     visibility.visibilityListener_();
+    viewerStub.returns(VisibilityState.VISIBLE);
     listen(INTERSECTION_50P, {
       visiblePercentageMin: 0, visiblePercentageMax: 100}, 2, [
         sinon.match({}),
@@ -251,6 +281,7 @@ describe('Visibility (tag: amp-analytics)', () => {
       it('for visibility state=' + state, () => {
         viewerStub.returns(state);
         visibility.visibilityListener_();
+        viewerStub.returns(VisibilityState.VISIBLE);
 
         listen(INTERSECTION_50P, {
           visiblePercentageMin: 0, visiblePercentageMax: 100}, 1, [sinon.match({
@@ -259,48 +290,35 @@ describe('Visibility (tag: amp-analytics)', () => {
       });
     }
 
-    // TODO(jridgewell, #4261): Reenable when I understand what the hell
-    // is going on.
-    // verifyState(VisibilityState.VISIBLE, '0');
+    verifyState(VisibilityState.VISIBLE, '0');
     verifyState(VisibilityState.HIDDEN, '1');
     verifyState(VisibilityState.PAUSED, '1');
     verifyState(VisibilityState.INACTIVE, '1');
   });
 
   describe('isVisibilitySpecValid', () => {
-    it('passes valid visibility spec', () => {
-      const specs = [
-        undefined,
-        {selector: '#abc'},
-        {
-          selector: '#a', continuousTimeMin: 10, totalTimeMin: 1000,
-          visiblePercentageMax: 99, visiblePercentageMin: 10,
-        },
-        {selector: '#a', continuousTimeMax: 1000, unload: true},
-      ];
-      for (const s in specs) {
-        expect(isVisibilitySpecValid({visibilitySpec: specs[s]}, true),
-            JSON.stringify(specs[s])).to.be.true;
-      }
-    });
+    function isSpecValid(spec, result) {
+      it('check for visibility spec: ' + JSON.stringify(spec), () => {
+        expect(isVisibilitySpecValid({visibilitySpec: spec}),
+            JSON.stringify(spec)).to.equal(result);
+      });
+    }
 
-    it('rejects invalid visibility spec', () => {
-      const specs = [
-        {},
-        {selector: 'abc'},
-        {selector: '#a', continuousTimeMin: -10},
-        {
-          selector: '#a', continuousTimeMax: 10, continuousTimeMin: 100,
-          unload: true,
-        },
-        {selector: '#a', continuousTimeMax: 100, continuousTimeMin: 10},
-        {selector: '#a', visiblePercentageMax: 101},
-      ];
-      for (const s in specs) {
-        expect(isVisibilitySpecValid({visibilitySpec: specs[s]}, true),
-            JSON.stringify(specs[s])).to.be.false;
-      }
-    });
+    isSpecValid(undefined, true);
+    isSpecValid({selector: '#abc'}, true);
+    isSpecValid({
+      selector: '#a', continuousTimeMin: 10, totalTimeMin: 1000,
+      visiblePercentageMax: 99, visiblePercentageMin: 10,
+    }, true);
+    isSpecValid({selector: '#a', continuousTimeMax: 1000}, true);
+
+    isSpecValid({}, false);
+    isSpecValid({selector: 'abc'}, false);
+    isSpecValid({selector: '#a', continuousTimeMax: 10, continuousTimeMin: 100},
+      false);
+    isSpecValid({selector: '#a', continuousTimeMax: 100, continuousTimeMin: 10},
+      true);
+    isSpecValid({selector: '#a', visiblePercentageMax: 101}, false);
   });
 
   describe('utils', () => {