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

Addon: Added google analytics #4138

Merged
merged 1 commit into from
Oct 13, 2018
Merged

Addon: Added google analytics #4138

merged 1 commit into from
Oct 13, 2018

Conversation

pksunkara
Copy link
Member

@pksunkara pksunkara commented Sep 7, 2018

Fixes #3944

This PR adds a google analytics addon. This is not ready to be merged. Just a call for review for now.

TODO

  • Add integration tests
  • List the addon in docs

@storybook-safe-bot
Copy link
Contributor

Fails
🚫

PR is marked with "do not merge", "in progress" labels.

Generated by 🚫 dangerJS

Then, set an environment variable

```
STORYBOOK_GA_ID = UA-000000-01
Copy link
Member

Choose a reason for hiding this comment

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

Is this a global variable?

Might want to make that explicit:

window.STORYBOOK_GA_ID = UA-000000-01;

@ndelangen
Copy link
Member

So I don't think we want this in any example we have.. since really I don't fancy getting into tracking people who browser our examples.

So maybe add some unit test for this?

@Hypnosphi
Copy link
Member

Maybe a separate examples/google-analytics?

@informa
Copy link

informa commented Oct 1, 2018

Awesome work, is this addon nearly finished?

@pksunkara
Copy link
Member Author

Need some docs work and tests.

@ndelangen
Copy link
Member

I actually think the code is simple enough to not have any tests tbh.

Let's add some documentation and merge, so we can announce this with 4.0.0 @pksunkara

@codecov
Copy link

codecov bot commented Oct 13, 2018

Codecov Report

Merging #4138 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4138      +/-   ##
==========================================
- Coverage   36.07%   36.02%   -0.06%     
==========================================
  Files         556      557       +1     
  Lines        6633     6643      +10     
  Branches      868      869       +1     
==========================================
  Hits         2393     2393              
- Misses       3796     3805       +9     
- Partials      444      445       +1
Impacted Files Coverage Δ
addons/google-analytics/src/register.js 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5971cee...82615b9. Read the comment docs.

@pksunkara
Copy link
Member Author

@ndelangen This PR is ready to go.

@ndelangen ndelangen merged commit 827c5bc into master Oct 13, 2018
@ndelangen ndelangen deleted the pksunkara/ga branch October 13, 2018 13:52
@igor-dv
Copy link
Member

igor-dv commented Oct 15, 2018

@igor-dv igor-dv mentioned this pull request Oct 16, 2018
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants