-
Notifications
You must be signed in to change notification settings - Fork 32
feat: ability to disable ga emit on load + expose optimize token #34
base: master
Are you sure you want to change the base?
Conversation
@@ -222,14 +211,87 @@ import './styles.scss' | |||
} | |||
``` | |||
|
|||
## Development |
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 development section removed?
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.
Because it no longer applies, that test page was removed in favor of automated E2E.
If you like I can re-create the playground.
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.
That would be nice or mentioning how to locally test (using e2e) if it is prefered
|
||
## emitOnLoad Caution | ||
|
||
By default emitOnLoad is true (for backward compatability). This means that when a user hits your site this plugin will tell Google Analytics (Optimize) that the user is in the experiment. |
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.
I think it depends. Not all experiments are specific to some pages so backward compatibility
is misleading here even if in next major we are going to disable by default. There could be also smarter ways like defining if an experiment is global or manual in experiment export file and skip auto emit if is not.
|
||
By default emitOnLoad is true (for backward compatability). This means that when a user hits your site this plugin will tell Google Analytics (Optimize) that the user is in the experiment. | ||
|
||
If your experiments aren't on every page, you'll want to turn this off and report the experiment yourself. If you don't, then your Google Optimize reports will contain a bunch of users who likely never saw your experiment. In other words **GARBAGE DATA**. |
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.
Is it really necessary to use negative word like GARBAGE DATA instead of a WARNING:
?
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.
Sorry, too much passion & personality showing :). I'll tweak.
```js | ||
mounted(){ | ||
// call Google Analytics directly and set the experiment for the user | ||
window.ga('set', 'exp', this.$exp.token) |
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.
ga and gtm (via dataLayer` are commonly used and best practices for google optimize integration also by our docs.
I strongly believe we should support built-in utility for manual triggering (like this.$exp.send()
) for both DX and additional checks like prevent adding extra events.
- With module options
handler
(ga
|gtm
) andhandlerName
(dataLayer
|ga
) we can configure howsend()
utility behaves - For advanced use cases without either, we may document manual trigger using exposed token (which currently no 3rd method is even mentioned in docs)
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.
Does it need to be this complex? Even if you use GTM you'd be loading GA via GTM, so window.ga should still exist (so .send() could just use window.ga). For advanced usage you'd still be able to use $exp.token (i can't think of any use-cases)
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.
Well at least the docs is advicing to use dataLayer.push({ exp: window.$nuxt.$exp.token })
which can be much easier IMO by using send
. And if changing the way of sending events, instead of refactoring all usages, we can simply change behavior with a config.
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.
We could have handler: (ga | gtm | auto - default) as config options
For auto we'd first check for window.dataLayer and use gtm/push else-if we detect window.ga we'd use window.ga
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.
That's even sexier 💯
const expCookie = experiment.experimentID + '.' + variantIndexes.join('-') | ||
|
||
// expose raw optimize token directly | ||
experiment.token = expCookie |
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.
Maybe can be merged with L66 like experiment.token = experiment.experimentID + '.' + variantIndexes.join('-')
? to avoid extra expCookie
variable?
## Integrating with Google Tag Manager | ||
There are many ways to integrate Google Optimize with GTM. Usually you have Google Analytics setup in GTM and you push the experiment token into GTM. | ||
|
||
### Method 1 - Push Token To the Data Layer |
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.
What do you think to also link each method to google docs?
googleOptimize: { | ||
// experimentsDir: '~/experiments', | ||
// maxAge: 60 * 60 * 24 * 7 // 1 Week | ||
// maxAge: 604800 // 1 week (in seconds) |
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.
60 * 60 * 24 * 7
convention is to allow developers quickly modify time without need to calculations and increasing source readability
I'm also toying with the idea of revamping the MVT support. Currently it's just 'sections:#' and weights but in practice I would find it very hard to code an experiment against a variant index and an intVal vs a named obj with values. For example. As a user, I'd prefer something like this if($exp.$activeVariants.buttonColor == 'red') .. vs An array of variant objects ($exp.$activeVariants) This idea is pretty new and not totally flushed out, but if we do decide to go in this direction, then we'd want a new major version for sure. |
Nice idea @hecktarzuli. Just created #38 based on your suggestion. Maybe we can do it via a non-breaking change too as part of |
No description provided.