[bug 1032455] Add helpfulness interface using GA #3282
Conversation
|
I think this is not ready -- needs l10n at least. |
| @@ -0,0 +1,62 @@ | |||
| (function(win, doc, $) { | |||
| if (document.cookie.replace(/(?:(?:^|.*;\s*)helpful-stfu\s*\=\s*([^;]*).*$)|^.*$/, "$1") !== 'true') { | |||
There was a problem hiding this comment.
Maybe we use localStorage instead?
|
Looks good to me, with 1 nit-pick: an article always asks me if it was helpful even if I've already voted 'Yes' or 'No' on it? Since users often navigate back and forth on pages on MDN, should we suspend the prompt if the user has already answered for the article in the last NN days? @stephaniehobson ? If we wrap the |
|
That's a good point -- an oversight, really. I'd rather fix it (making our interaction with localstorage a bit more complex) than bug people about the same article. Maybe a flag that expires after 6(?) months. Will also fix the waffle setup. |
|
O dang. Merge commit in there. Will extricate. |
hoosteeno
force-pushed
the
1032455-helpfulness
branch
2 times, most recently
from
90340d5
to
edb6af6
Compare
|
Also need to re-word the commit message to be prefixed with "bug NNNNNNN" |
| var notification = mdn.Notifier.growl(ask, {closable: true, duration: 0}).question(); | ||
|
|
||
| // answers to the simple question include... | ||
| $('#helpful-yes').click(function() { |
There was a problem hiding this comment.
The new syntax is .on('click', function() {
hoosteeno
force-pushed
the
1032455-helpfulness
branch
4 times, most recently
from
ca530ce
to
abf41d4
Compare
|
@hoosteeno - want to assign someone specific to review? |
|
@darkwing has been giving great feedback so far. |
|
@darkwing - got a chance to re-review this soon? I'd love to launch this tomorrow if possible so we can start collecting the data. |
| (function(win, doc, $) { | ||
| // no localStorage, no helpfulness rating | ||
| if (('localStorage' in win)) { | ||
| var stfu = localStorage.getItem('helpful-stfu') === 'true'; // true if ever clicked stfu |
There was a problem hiding this comment.
For the sake of positivity, we should probably change the stfu references to ignore or something like that. stfu may reflect bad on us.
|
Outside of these two nits, I really like what you've done here! |
hoosteeno
force-pushed
the
1032455-helpfulness
branch
from
abf41d4
to
19964e8
Compare
| // create a notification | ||
| function inquire() { | ||
| // dimension7 is "helpfulness" | ||
| ga('set', 'dimension7', 'Yes'); |
There was a problem hiding this comment.
My last feedback is that this line should be moved to:
templates/includes/google_analytics.html
There was a problem hiding this comment.
I think I understand why this is a good idea in principle. But we're setting this dimension flag for users flagged by waffle, who have not said "Never ask me again" and who have not already responded to the inquiry on the page they're looking at. Replicating all that non-GA business logic in the GA file seems like a violation of DRY. Do you see another way to do it, or do you think maybe this should be an exception to the rule?
There was a problem hiding this comment.
My 💵 0.02:
The point of the custom dimension is to analyze between 2 segments/cohorts in GA: 1. Those who saw the helpfulness feature and 2. Those who did not.
Given that, only setting the dimension when we create the (initial) notification makes sense to me. Otherwise, we'll be putting visitors into the "Saw helpfulness dialog" cohort even if they never saw the notification.
There was a problem hiding this comment.
OK. In that case, I suggest:
if(win.ga) ga('set', 'dimension7', 'Yes');
..since we don't use GA locally. Threw an error for me.
hoosteeno
force-pushed
the
1032455-helpfulness
branch
from
e99566c
to
9647828
Compare
|
I added code to resolve @darkwing's most recent comment (hoosteeno@9647828#diff-25a3d06887fce3344ebdad7742003900R16). Unless there are any further comments, this code is ready to merge. We can switch the flag after further coordination. |
|
|
|
Any chance we can merge this in the next 30-60m? I'd like to push it (along with #3346) today if possible ... |
|
Looks good, let's give it a try |
|
R+ |
[bug 1032455] Add helpfulness interface using GA
|
Just a post-deploy note ... there was confusing behavior in prod where the |
|
Yeah, something needs to be done about that. I think a check for the waffle needs to be added to helpfulness.js |
This is an experiment to determine whether people use the feature and whether we use the data from it.