Skip to content
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

Jump to bottom

Backgrounded Variable added to Timer Trigger #5240

Merged
merged 12 commits into from
Oct 6, 2016
Merged

Backgrounded Variable added to Timer Trigger #5240

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
019e5fc
Added backgrounded variable to timer trigger.
dynes Sep 26, 2016
0d50d02
Merge branch 'master' of ssh://github.com/ampproject/amphtml into vie…
dynes Sep 26, 2016
8284e88
Merge branch 'master' of ssh://github.com/ampproject/amphtml into vie…
dynes Sep 27, 2016
e8799de
Inlined backgrounded field in vars object.
dynes Sep 27, 2016
ac17bf0
Merge branch 'master' of ssh://github.com/ampproject/amphtml into vie…
dynes Sep 30, 2016
12c2187
Hoisted timer event function.
dynes Sep 30, 2016
329663a
Merge branch 'master' of ssh://github.com/ampproject/amphtml into vie…
dynes Oct 5, 2016
ec024b3
Merge branch 'master' of ssh://github.com/ampproject/amphtml into vie…
dynes Oct 6, 2016
e92bba9
Added BACKGROUNDED_STATE to url replacements. Added support for BACK…
dynes Oct 6, 2016
d8591f3
Use correct set call for BACKGROUNDED_STATE call.
dynes Oct 6, 2016
3dab5b2
Changed BACKGROUNDED_STATE to BACKGROUND_STATE
dynes Oct 6, 2016
33c59b3
Update instrumentation.js
avimehta Oct 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/analytics.amp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
"endpoint": "https://raw.githubusercontent.com/ampproject/amphtml/master/examples/img/ampicon.png",
"base": "${endpoint}?${type}&path=${canonicalPath}",
"event": "${base}&scrollY=${scrollTop}&scrollX=${scrollLeft}&height=${availableScreenHeight}&width=${availableScreenWidth}&scrollBoundV=${verticalScrollBoundary}&scrollBoundH=${horizontalScrollBoundary}",
"visibility": "${base}&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}&subTitle=${subTitle}"
"visibility": "${base}&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}&subTitle=${subTitle}",
"timer": "${base}&backgrounded=${backgrounded}"
},
"vars": {
"title": "Example Request"
Expand Down Expand Up @@ -111,7 +112,7 @@
"interval": 5,
"maxTimerLength": 20
},
"request": "base",
"request": "timer",
"vars": {
"type": "timer"
}
Expand Down
15 changes: 10 additions & 5 deletions extensions/amp-analytics/0.1/instrumentation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,13 +492,17 @@ export class InstrumentationService {
createTimerListener_(listener, timerSpec) {
const hasImmediate = timerSpec.hasOwnProperty('immediate');
const callImmediate = hasImmediate ? Boolean(timerSpec.immediate) : true;
const intervalId = this.win_.setInterval(
listener.bind(null, new AnalyticsEvent(AnalyticsEventType.TIMER)),
timerSpec['interval'] * 1000
);
const viewer = this.viewer_;
const intervalId = this.win_.setInterval(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's hoist this function instead of passing it. Then we can call it in the callImmediate conditional below.

const vars = {};
vars['backgrounded'] = viewer.isVisible() ? '0' : '1';
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be done inline:

const vars = {
  backgrounded: viewer.isVisible ? '0' : '1',
};

listener(new AnalyticsEvent(AnalyticsEventType.TIMER, vars));
}, timerSpec['interval'] * 1000);

if (callImmediate) {
listener(new AnalyticsEvent(AnalyticsEventType.TIMER));
const vars = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

We could instead call the function above directly.

vars['backgrounded'] = viewer.isVisible() ? '0' : '1';
listener(new AnalyticsEvent(AnalyticsEventType.TIMER, vars));
}

const maxTimerLength = timerSpec['maxTimerLength'] ||
Expand All @@ -516,3 +520,4 @@ export function instrumentationServiceFor(window) {
return fromClass(window, 'amp-analytics-instrumentation',
InstrumentationService);
}