From e3f9cfb9f5f3898d35c9b91bca1c0f4469bcd29c Mon Sep 17 00:00:00 2001 From: Ikenna Okpala Date: Thu, 28 Jul 2016 13:26:59 +0100 Subject: [PATCH 1/5] Cosmetic Changes: Removing the duplicate function names This is an attempt to clean up the function definitions and improve readability. --- .../javascripts/analytics/scroll-tracker.js | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/analytics/scroll-tracker.js b/app/assets/javascripts/analytics/scroll-tracker.js index 6f0380e38..82ec3303c 100644 --- a/app/assets/javascripts/analytics/scroll-tracker.js +++ b/app/assets/javascripts/analytics/scroll-tracker.js @@ -1,7 +1,7 @@ (function() { "use strict"; - window.GOVUK = window.GOVUK || {}; + window.GOVUK = window.GOVUK || {}; var CONFIG = { '/': [ @@ -152,13 +152,13 @@ this.trackVisibleNodes(); }; - ScrollTracker.prototype.getConfigForCurrentPath = function getConfigForCurrentPath(sitewideConfig) { + ScrollTracker.prototype.getConfigForCurrentPath = function (sitewideConfig) { for ( var path in sitewideConfig ) { if ( window.location.pathname == path ) return sitewideConfig[path]; } }; - ScrollTracker.prototype.buildNodes = function buildNodes(config) { + ScrollTracker.prototype.buildNodes = function (config) { var nodes = []; var nodeConstructor, nodeData; @@ -171,12 +171,12 @@ return nodes; }; - ScrollTracker.prototype.onScroll = function onScroll() { + ScrollTracker.prototype.onScroll = function () { clearTimeout(this.scrollTimeout); this.scrollTimeout = setTimeout($.proxy(this.trackVisibleNodes, this), this.SCROLL_TIMEOUT_DELAY); }; - ScrollTracker.prototype.trackVisibleNodes = function trackVisibleNodes() { + ScrollTracker.prototype.trackVisibleNodes = function () { for ( var i=0; i= this.percentage; }; - ScrollTracker.PercentNode.prototype.currentScrollPercent = function currentScrollPercent() { + ScrollTracker.PercentNode.prototype.currentScrollPercent = function () { var $document = $(document); var $window = $(window); return( ($window.scrollTop() / ($document.height() - $window.height())) * 100.0 ); }; - - - ScrollTracker.HeadingNode = function HeadingNode(headingText) { + ScrollTracker.HeadingNode = function (headingText) { this.$element = getHeadingElement(headingText); this.eventData = {action: "Heading", label: headingText}; @@ -220,19 +216,17 @@ } }; - ScrollTracker.HeadingNode.prototype.isVisible = function isVisible() { + ScrollTracker.HeadingNode.prototype.isVisible = function () { if ( !this.$element ) return false; return this.elementIsVisible(this.$element); } - ScrollTracker.HeadingNode.prototype.elementIsVisible = function elementIsVisible($element) { + ScrollTracker.HeadingNode.prototype.elementIsVisible = function ($element) { var $window = $(window); var positionTop = $element.offset().top; return ( positionTop > $window.scrollTop() && positionTop < ($window.scrollTop() + $window.height()) ); }; - - $().ready(function() { window.GOVUK.scrollTracker = new ScrollTracker(CONFIG); }); From 139c8500c684bd493b9a11661bbc74ee825b0d3c Mon Sep 17 00:00:00 2001 From: Ikenna Okpala Date: Thu, 28 Jul 2016 13:38:04 +0100 Subject: [PATCH 2/5] Handle path comparison better. The intention here is to return true for the two forms of paths. Example: 1) /register-to-vote 2) /register-to-vote/ Rightly the previous code only checked and returned true for first instance and failed in the second. This behaviour was only observed whilst working with the dev VM. On Integration, Staging and GOV.UK paths are very consistent and always resulting/returning the aforementioned first instance. I have added this method to better cater for both, It splits the pathnames with the forward slash delimiter and joins all. This in effect returns a combination of all the characters in the path for the current page, returning true for only paths that equal the defined slug. --- app/assets/javascripts/analytics/scroll-tracker.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/analytics/scroll-tracker.js b/app/assets/javascripts/analytics/scroll-tracker.js index 82ec3303c..a8c017ea4 100644 --- a/app/assets/javascripts/analytics/scroll-tracker.js +++ b/app/assets/javascripts/analytics/scroll-tracker.js @@ -154,7 +154,9 @@ ScrollTracker.prototype.getConfigForCurrentPath = function (sitewideConfig) { for ( var path in sitewideConfig ) { - if ( window.location.pathname == path ) return sitewideConfig[path]; + if (this.normalisePath(window.location.pathname) == this.normalisePath(path)) { + return sitewideConfig[path]; + } } }; @@ -171,6 +173,10 @@ return nodes; }; + ScrollTracker.prototype.normalisePath = function (path){ + return path.split("/").join(""); + }; + ScrollTracker.prototype.onScroll = function () { clearTimeout(this.scrollTimeout); this.scrollTimeout = setTimeout($.proxy(this.trackVisibleNodes, this), this.SCROLL_TIMEOUT_DELAY); From a85844a991880e6c808d57716d3d7d81ec66e6d7 Mon Sep 17 00:00:00 2001 From: Ikenna Okpala Date: Thu, 28 Jul 2016 13:53:17 +0100 Subject: [PATCH 3/5] Reduce the scroll delay. This appears to better react to fast scrolling and gives better feedback via GA debugger tool --- app/assets/javascripts/analytics/scroll-tracker.js | 2 +- spec/javascripts/analytics/scroll-tracker-spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/analytics/scroll-tracker.js b/app/assets/javascripts/analytics/scroll-tracker.js index a8c017ea4..bd1105430 100644 --- a/app/assets/javascripts/analytics/scroll-tracker.js +++ b/app/assets/javascripts/analytics/scroll-tracker.js @@ -138,7 +138,7 @@ function ScrollTracker(sitewideConfig) { this.config = this.getConfigForCurrentPath(sitewideConfig); - this.SCROLL_TIMEOUT_DELAY = 500; + this.SCROLL_TIMEOUT_DELAY = 10; if ( !this.config ) { this.enabled = false; diff --git a/spec/javascripts/analytics/scroll-tracker-spec.js b/spec/javascripts/analytics/scroll-tracker-spec.js index 0fdb18106..e8b4e80ce 100644 --- a/spec/javascripts/analytics/scroll-tracker-spec.js +++ b/spec/javascripts/analytics/scroll-tracker-spec.js @@ -116,6 +116,6 @@ describe("GOVUK.ScrollTracker", function() { return ( $element[0] == elementScrolledTo ); }); $(window).scroll(); - jasmine.clock().tick(510); + jasmine.clock().tick(20); }; }); From 44b7a3e5f5c3954186726d215eaeb56e0c2558fa Mon Sep 17 00:00:00 2001 From: Anika Henke Date: Wed, 3 Aug 2016 17:13:24 +0100 Subject: [PATCH 4/5] Remove obsolete paths from scroll tracking To reduce the amount of people who might be exposed to the scroll tracking unneccessarily, this removes all paths from the scroll tracking which are not required anymore. I went through all of them with Vinith (@vp2015) and we removed all for which data wasn't requested for ages. --- .../javascripts/analytics/scroll-tracker.js | 98 ------------------- 1 file changed, 98 deletions(-) diff --git a/app/assets/javascripts/analytics/scroll-tracker.js b/app/assets/javascripts/analytics/scroll-tracker.js index bd1105430..723af000b 100644 --- a/app/assets/javascripts/analytics/scroll-tracker.js +++ b/app/assets/javascripts/analytics/scroll-tracker.js @@ -4,34 +4,6 @@ window.GOVUK = window.GOVUK || {}; var CONFIG = { - '/': [ - ['Heading', 'Services and information'], - ['Heading', 'More on GOV.UK'], - ['Percent', 80] //To track 'Services and information' section in footer - ], - '/bank-holidays': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ], - '/jobsearch': [ - ['Heading', 'Registration'], - ['Heading', 'Help'] - ], - '/register-to-vote': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ], - '/apply-uk-visa': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ], - '/student-finance-register-login': [ - ['Heading', 'Log in problems'], - ['Heading', 'Manage your student finance'] - ], '/contact-the-dvla/y/driving-licences-and-applications': [ ['Heading', 'Driving licencing enquiries'], ['Heading', 'When to contact DVLA'] @@ -44,76 +16,6 @@ ['Heading', 'Vehicle registration enquiries'], ['Heading', 'When to contact DVLA'] ], - '/using-the-civil-service-jobs-website': [ - ['Heading', 'Your Civil Service Jobs account'], - ['Heading', 'Job alerts'], - ['Heading', 'Applying for a job'], - ['Heading', 'Civil Service Initial Sift Test'], - ['Heading', 'Results and feedback'], - ['Heading', 'Civil Service recruitment'], - ['Heading', 'Technical Support'], - ['Heading', 'Contact Information'] - ], - '/government/publications/spending-review-and-autumn-statement-2015-documents/spending-review-and-autumn-statement-2015': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ], - '/guidance/universal-credit-how-it-helps-you-into-work': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75], - ['Heading', 'Opening up work'], - ['Heading', 'Support from your work coach'], - ['Heading', 'When you can claim Universal Credit'], - ['Heading', 'More detailed advice'] - ], - '/openingupwork': [ - ['Heading', 'How Universal Credit makes work pay'], - ['Heading', 'When you can claim Universal Credit'], - ['Heading', 'Help and advice'] - ], - '/guidance/universal-credit-how-it-can-help-your-business': [ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ], - '/government/publications/see-potential-case-studies-and-guidance-for-employers/see-potential-case-studies-and-guidance-for-employers': [ - ['Heading', 'Case studies'], - ['Heading', 'The business benefits'], - ['Heading', 'What people are saying'], - ['Heading', 'Review your recruitment approach to make sure you’re not missing out on talent and potential'] - ], - '/government/groups/common-technology-services-cts': [ - ['Heading', 'Our products'], - ['Heading', 'Our services'], - ['Heading', 'Our priorities'] - ], - '/guidance/common-technology-services-cts-secure-email-blueprint': [ - ['Heading', '1. Understand government policy'], - ['Heading', '2. Follow our technical specification'], - ['Heading', '3. Change email domain names as required'], - ['Heading', '5. Get CTS’ assurance'], - ['Heading', '6. Maintain your documentation and end user policies'], - ['Heading', '7. Buy the solution'] - ], - '/guidance/common-technology-services-cts-guide-to-implementing-the-secure-email-blueprint': [ - ['Heading', 'Email service prerequisites'], - ['Heading', 'Transport Layer Security (TLS)'], - ['Heading', 'Domain-based Message Authentication, Reporting and Conformance (DMARC)'], - ['Heading', 'DomainKeys Identified Mail (DKIM)'], - ['Heading', 'Sender Policy Framework (SPF)'], - ['Heading', 'Other email sending services'], - ['Heading', 'Making DNS changes'], - ['Heading', 'Assurance'] - ], - '/government/publications/budget-2016-documents/budget-2016': [ - ['Percent', 20], - ['Percent', 40], - ['Percent', 60], - ['Percent', 80], - ['Percent', 100] - ], '/government/collections/disability-confident-campaign': [ ['Heading', 'Become a Disability Confident employer'], ['Heading', 'Aims and objectives'], From 6289f90d6e1b797d5cdd66486175b4285098d4be Mon Sep 17 00:00:00 2001 From: Anika Henke Date: Wed, 3 Aug 2016 19:11:41 +0100 Subject: [PATCH 5/5] Remove code for percent nodes The previous commit removed all existing percent nodes as tracking scrolling by percent did not prove to be very helpful in general according to Vinith (@vp2015). This removes all code related to percent scroll tracking as it's highly unlikely to be used again. --- .../javascripts/analytics/scroll-tracker.js | 15 ----- .../analytics/scroll-tracker-spec.js | 61 ++++++------------- 2 files changed, 17 insertions(+), 59 deletions(-) diff --git a/app/assets/javascripts/analytics/scroll-tracker.js b/app/assets/javascripts/analytics/scroll-tracker.js index 723af000b..2295a808f 100644 --- a/app/assets/javascripts/analytics/scroll-tracker.js +++ b/app/assets/javascripts/analytics/scroll-tracker.js @@ -97,21 +97,6 @@ } }; - ScrollTracker.PercentNode = function (percentage) { - this.percentage = percentage; - this.eventData = {action: "Percent", label: String(percentage)}; - }; - - ScrollTracker.PercentNode.prototype.isVisible = function () { - return this.currentScrollPercent() >= this.percentage; - }; - - ScrollTracker.PercentNode.prototype.currentScrollPercent = function () { - var $document = $(document); - var $window = $(window); - return( ($window.scrollTop() / ($document.height() - $window.height())) * 100.0 ); - }; - ScrollTracker.HeadingNode = function (headingText) { this.$element = getHeadingElement(headingText); this.eventData = {action: "Heading", label: headingText}; diff --git a/spec/javascripts/analytics/scroll-tracker-spec.js b/spec/javascripts/analytics/scroll-tracker-spec.js index e8b4e80ce..4aa860b87 100644 --- a/spec/javascripts/analytics/scroll-tracker-spec.js +++ b/spec/javascripts/analytics/scroll-tracker-spec.js @@ -10,9 +10,16 @@ describe("GOVUK.ScrollTracker", function() { }); describe("enabling on correct pages", function() { + var FIXTURE = "

A heading

"; + + beforeEach(function() { + setFixtures(FIXTURE); + spyOn(GOVUK.ScrollTracker.HeadingNode.prototype, 'elementIsVisible'); + }); + it("should be enabled on a tracked page", function() { var config = {} - config[window.location.pathname] = [ ['Percent', 50] ]; + config[window.location.pathname] = [ ['Heading', 'A heading'] ]; expect( (new GOVUK.ScrollTracker(config)).enabled ).toBeTruthy(); }); @@ -20,34 +27,13 @@ describe("GOVUK.ScrollTracker", function() { it("should not be enabled on an untracked page", function() { var config = { '/some/other/path': [ - ['Percent', 50] + ['Heading', 'A heading'] ] }; expect( (new GOVUK.ScrollTracker(config)).enabled ).toBeFalsy(); }); }); - describe("tracking by scrolled percentage", function() { - beforeEach(function() { - spyOn(GOVUK.ScrollTracker.PercentNode.prototype, "currentScrollPercent"); - }); - - it("should send an event when the page scrolls to >= the percentage specified", function() { - var config = buildConfigForThisPath([ - ['Percent', 25], - ['Percent', 50], - ['Percent', 75] - ]); - new GOVUK.ScrollTracker(config); - - scrollToPercent(60); - - expect(GOVUK.analytics.trackEvent.calls.count()).toBe(2); - expect(GOVUK.analytics.trackEvent.calls.argsFor(0)).toEqual(["ScrollTo", "Percent", {label: "25", nonInteraction: true}]); - expect(GOVUK.analytics.trackEvent.calls.argsFor(1)).toEqual(["ScrollTo", "Percent", {label: "50", nonInteraction: true}]); - }); - }); - describe("tracking by headings", function() { var FIXTURE = "\

This is the first heading

\ @@ -59,15 +45,14 @@ describe("GOVUK.ScrollTracker", function() { beforeEach(function() { setFixtures(FIXTURE); spyOn(GOVUK.ScrollTracker.HeadingNode.prototype, 'elementIsVisible'); - }); - - it("should send an event when the user scrolls so the heading is visible", function() { var config = buildConfigForThisPath([ ['Heading', "This is the first heading"], ['Heading', "This is the third heading"] ]); new GOVUK.ScrollTracker(config); + }); + it("should send an event when the user scrolls so the heading is visible", function() { scrollToShowHeadingNumber(1); expect(GOVUK.analytics.trackEvent.calls.count()).toBe(1); @@ -82,34 +67,22 @@ describe("GOVUK.ScrollTracker", function() { expect(GOVUK.analytics.trackEvent.calls.count()).toBe(2); expect(GOVUK.analytics.trackEvent.calls.argsFor(1)).toEqual(["ScrollTo", "Heading", {label: "This is the third heading", nonInteraction: true}]); }); - }); - it("should not send duplicate events", function() { - spyOn(GOVUK.ScrollTracker.PercentNode.prototype, "currentScrollPercent"); - - var config = buildConfigForThisPath([ - ['Percent', 25] - ]); - new GOVUK.ScrollTracker(config); + it("should not send duplicate events", function() { + scrollToShowHeadingNumber(1); + scrollToShowHeadingNumber(3); + scrollToShowHeadingNumber(1); - scrollToPercent(30); - scrollToPercent(35); - expect(GOVUK.analytics.trackEvent.calls.count()).toBe(1); + expect(GOVUK.analytics.trackEvent.calls.count()).toBe(2); + }); }); - function buildConfigForThisPath(thisPathData) { var config = {}; config[window.location.pathname] = thisPathData; return config; } - function scrollToPercent(percent) { - GOVUK.ScrollTracker.PercentNode.prototype.currentScrollPercent.and.returnValue(percent); - $(window).scroll(); - jasmine.clock().tick(510); - }; - function scrollToShowHeadingNumber(headingNumber) { var elementScrolledTo = $('h1, h2, h3, h4, h5, h6')[headingNumber-1]; GOVUK.ScrollTracker.HeadingNode.prototype.elementIsVisible.and.callFake(function($element) {