revolunet / angular-google-analytics

Google Analytics tracking for your AngularJS apps
MIT License
652 stars 172 forks source link

Mobile app: Is it possible to track screens #145

Open S-K-Lieren opened 8 years ago

S-K-Lieren commented 8 years ago

Developing a mobile app. There are 'Screens' instead of 'Pages' in Google Analytics. trackPage() doesn't produce any real-time results while trackEvent() works like a charm. I'd expect either a trackScreen()-method or another behaviour for trackPage() when setHybridMobileSupport(true) has been called. #

justinsa commented 8 years ago

This sounds like an issue you have with Google Analytics and not this angularized wrapper around it. What exactly would you expect trackScreen to do? Would it call trackEvent with some default parameters? How is trackPage not doing this? There is conceptually no difference between a screen and page, they are the same thing.

MichaelBorde commented 8 years ago

For a mobile app on GA you have to send a "screenview" hit type.

The (simplified) workaround i use:

$rootScope.$on('$stateChangeSuccess', (event, state) => {
    Analytics.send('screenview', {
      screenName: state.name
    });
});
S-K-Lieren commented 8 years ago

Thank you!

Toxantron commented 8 years ago

@justinsa, @revolunet maybe it is possible to use a delegate approach in Line 1086 to apply different listener logic.

Something like:

// As default
eventListener = function() {},
// In config for this issue
AnalyticsProvider.setEvent('$stateChangeSuccess')
                           .useMobileMode(true);
// Later only switch delegates
if (trackRoutes) {
  eventListener = function () {
    that._trackPage();
  }
}
if (trackRoutes && readFromRoute) {
  eventListener = function () {
    if (!$route.current || !$route.current.templateUrl || $route.current.doNotTrack) {
      return;
    }

   that._trackPage();
  }
}
if (useMobileMode) {
  eventListener = function(event, state) {
    _send('screenview', {
      screenName: state.name
    }
  }
}

// And finally register it
$rootScope.$on(pageEvent, eventListener);

Such a structure could be easily extended if more requirements pop up for modified tracking behavior without breaking exisiting logic and blowing up the code too much. What do you think?

justinsa commented 8 years ago

@Toxantron Why not have the _trackPage method do the screenview send if useMobileAppMode is true? This isolates the concern to the _trackPage method and leaves all of the existing logic untouched.

Toxantron commented 8 years ago

True, might have overengineered that. ;-)

Toxantron commented 8 years ago

Now I remember why I went for delegate approach. I wasn't sure whether or not the method parameters like state are required. And using above mentioned approach would allow method signatures to match the event definition.

justinsa commented 8 years ago

A delegate makes more sense if we push it up a notch and allow the user to define the delegate via configuration. The default behavior of trackPage in mobile mode could just pass the current URL via screenview. Providing a sensible default usually covers a lot of cases and the configurable delegate then gives others an easy add.

Toxantron commented 8 years ago

Exactly where I was going with it. I just thought instead of adding more if statements into the code we rather go with the strategy pattern. Using the newly build API (method delegate) to cover readFromRoute, trackRoutes and the new mobile mode. This reduces number of if (configValue) statements and opens new API for custom extensions.

Toxantron commented 8 years ago

I could prepare a PR with the delegate API without adding any new features and you can take a look. If you like it we merge it and provide API for custom delegate and mobile screens based on it. What do you say @justinsa?

justinsa commented 8 years ago

@Toxantron I am always happy to consider a PR for a reasonable feature addition.

Toxantron commented 8 years ago

Layed the Groundwork in #149.