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.

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

Feat(event) - added nonInteraction option #8

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

adlenafane
Copy link
Contributor

Implementing the non-interaction events https://developers.google.com/analytics/devguides/collection/analyticsjs/events#non-interaction_events
@kaesonho I'm not sure about how you would like to implement this. Could you review please?

@hero78119
Copy link
Contributor

IMO the flatten-style arguments for ga is a little bit awkward :(
It strictly depends on the arguments order, which might be lead to trouble of scalability
According to fieldsObject, it should be better to refactor to sth like

var params = {
    hitType: 'event',
    eventCategory: model.category || DEFAULT_CATEGORY,
    eventAction: model.action || DEFAULT_ACTION,
    eventLabel: model.label || i13nNode.getText(payload.target) || DEFAULT_LABEL,
    eventValue: model.value || 0,
    transport: model.transport || 'none',
    nonInteraction: model.nonInteraction || false
};

_command.call(this, {
  tracker: model.tracker || '',
  commandName: 'send',
  arguments: [params]
});

@@ -91,6 +91,11 @@ ReactI13nGoogleAnalytics.prototype.getPlugin = function () {
if (model.value) {
params.push(model.value);
}
if (model.nonInteraction) {
Copy link
Owner

Choose a reason for hiding this comment

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

yeah will something be broken if value doesn't exist but nonInteraction does?

Copy link
Owner

Choose a reason for hiding this comment

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

@adlenafane hi any update for this?

@adlenafane
Copy link
Contributor Author

@hero78119 @kaesonho I did a bit of refacto to use the ga fieldsObject :)

@kaesonho
Copy link
Owner

thanks for the update 👍

@kaesonho kaesonho merged commit ac49035 into kaesonho:master Jun 23, 2016
@kaesonho
Copy link
Owner

it's published https://github.com/kaesonho/react-i13n-ga/releases/tag/v0.2.5 :)

@adlenafane adlenafane deleted the feat-params-nonInteraction branch June 23, 2016 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants