-
Notifications
You must be signed in to change notification settings - Fork 14
Google Analytics helper #37
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.
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
Changes from all commits
15531a4
324a620
6832800
555fe01
d06d2f6
4b8be83
71d0935
d4dc9cc
5c84ec4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| import app from 'main.module'; | ||
| import {ILogger, LoggerService} from 'helpers/logger/logger'; | ||
| import {IApplicationEnvironment} from 'main.constants'; | ||
|
|
||
| const analyticsScriptUrl = '//www.google-analytics.com/analytics.js'; | ||
|
|
||
| interface IWindowWithAnalytics extends ng.IWindowService { | ||
| ga: any; | ||
| } | ||
|
|
||
| /** | ||
| * Analytics service: insert Google Analytics library in the page. | ||
| */ | ||
| export class AnalyticsService { | ||
|
|
||
| private logger: ILogger; | ||
| private analyticsAreActive = false; | ||
|
|
||
| constructor(private $window: IWindowWithAnalytics, | ||
| private config: IApplicationEnvironment, | ||
| logger: LoggerService) { | ||
|
|
||
| this.logger = logger.getLogger('analyticsService'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logger not needed if it's not used, or maybe it could be used to log tracked events? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely, logging tracked events was the point. I must have lost it somewhere along the road ; will fix it now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done earlier (with previous changes). |
||
|
|
||
| this.init(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer a public This is also mandatory for mobile/Cordova compatibility, as specific configuration is needed for GA to work with This would also allow us to disable analytics in debug environment, as it is most of the time only wanted on production and it can mess with unit tests by generating extra HTTP calls. See this example except from a mobile of mine, to make it work with Cordova (located in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the responsibility of customizing the creation of the GA object lies within the analytics service. All the Cordova specific code can be added in analytics-service instead of main-run. Regarding disabling of analytics in debug, there are some cases where having them on is useful (typically when one wants to test out a newly-added event tracker). Of course you would want a different analytics account for debugging and for actual production, but that is just a matter of setting up the gulp-replace task around IApplicationConfig.googleAnalyticsId. If you want to disable them entirely in debug mode, it can also be done the same way, by setting the value to null in debug mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I completely agree with that, I just posted it to show a mobile setup ;)
In my idea disabling analytics in debug would be done in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: googleAnalyticsId moved from Config to Environment. |
||
| } | ||
|
|
||
| /** | ||
| * Tracks a page change in google analytics. | ||
| * @param {String} url The url of the new page. | ||
| */ | ||
| trackPage (url: string) { | ||
| if (this.analyticsAreActive) { | ||
| let urlWithoutParams = url; | ||
| let split = url.split('?'); | ||
| if (split.length > 1) { | ||
| urlWithoutParams = split[0]; | ||
| } | ||
| this.$window.ga('send', 'pageview', urlWithoutParams); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sends a track event to google analytics. | ||
| * @param {String} category The category to be sent. | ||
| * @param {String} action The action to be sent. | ||
| * @param {String=} label The label to be sent. | ||
| */ | ||
| trackEvent (category: string, action: string, label?: string) { | ||
| if (this.analyticsAreActive) { | ||
| this.$window.ga('send', 'event', category, action, label); | ||
| let logMessage = 'Event tracked: ' + category + ' | ' + action; | ||
| if (label) { | ||
| logMessage += ' | ' + label; | ||
| } | ||
| this.logger.log(logMessage); | ||
| } | ||
| } | ||
|
|
||
| private init(): void { | ||
| if (this.config.googleAnayticsId !== null) { | ||
| this.createGoogleAnalyticsObject(this.$window, document, 'script', analyticsScriptUrl, 'ga'); | ||
| this.$window.ga('create', this.config.googleAnayticsId, 'auto'); | ||
| this.analyticsAreActive = true; | ||
| } | ||
| } | ||
|
|
||
| private createGoogleAnalyticsObject(i: any, s: any, o: any, g: any, r: any, a?: any, m?: any) { | ||
| i.GoogleAnalyticsObject = r; | ||
| i[r] = i[r] || function () { | ||
| (i[r].q = i[r].q || []).push(arguments); | ||
| }; | ||
| i[r].l = new Date(); | ||
| a = s.createElement(o); | ||
| m = s.getElementsByTagName(o)[0]; | ||
| a.async = 1; | ||
| a.src = g; | ||
| m.parentNode.insertBefore(a, m); | ||
| } | ||
| } | ||
|
|
||
| app.service('analyticsService', AnalyticsService); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since it is a service, unit tests are mandatory (though they will be quite simple as GA will be mocked) and should be added :) |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use
$stateChangeSuccesslike the hook to update title on route changes?This should fix the need for the "hack" :)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per ui-router documentation (link) :
Note that the view load events are not deprecated. So maybe the real solution is to look into transition hooks ? But that's a different beast altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL, missed that since we are still on the 0.3.x version and the 1.x version has not been finalized yet 😅
I have created a separate issue for the migration to using
$transitionshooks (see #38 ), for the meanwhile it's still a valid change (until 1.0.0 version is out) that will get rid of the ugly hack, and it will simplify the migration task, as it will be needed anyway as we use it elsewhere.Thanks for pointing that!