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: simple analytics #2110

Merged
merged 9 commits into from Dec 1, 2022
Merged

feat: simple analytics #2110

merged 9 commits into from Dec 1, 2022

Conversation

cmunns
Copy link
Contributor

@cmunns cmunns commented Nov 17, 2022

Swap countly for simple analytics.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2022

@cmunns cmunns changed the title Simple analytics feat: simple analytics Nov 17, 2022
@cmunns
Copy link
Contributor Author

cmunns commented Nov 21, 2022

Here is a custom event getting fired. I was unable to test implementation on safari and firefox bc of a cors error which I believe will be solved on prod.
Screenshot 2022-11-21 at 12 35 18 PM

@cmunns cmunns marked this pull request as ready for review November 21, 2022 20:36
@cmunns cmunns requested a review from a team as a code owner November 21, 2022 20:36
@JeffLowe JeffLowe linked an issue Nov 22, 2022 that may be closed by this pull request
7 tasks
@hugomrdias
Copy link
Contributor

do we have any write up about simple vs plausible ?

@cmunns
Copy link
Contributor Author

cmunns commented Nov 30, 2022

@hugomrdias ultimately came down to the events tracking between the two. SA is about to release a complete overhaul of their funnels/goals/conversions feature that will allow the team to leverage this data per the original ticket. Plausible when I first started this task also didnt have visit duration as a metric (which it appears to now have implemented)

@hugomrdias
Copy link
Contributor

hugomrdias commented Nov 30, 2022

@cwaring this may be interesting, original issue #2030

Copy link

@travis travis left a comment

Choose a reason for hiding this comment

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

lgtm - had a couple questions but they are mostly for my information - happy to see this merge as-is if you think that makes sense!

@@ -44,6 +45,15 @@ function AccordionSection({ active, toggle, toggleOnLoad, reportUID, slug, disab
reportUID(uid);
}, [reportUID, uid]);

useEffect(() => {
if (open && header) {
const headerLabel = header.props.children.find(child => {
Copy link

Choose a reason for hiding this comment

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

this is a litttttle funky - hard for me to tell whether it would be possible to use a Ref (or whether that would be cleaner) but curious if you considered that rather than searching through children? not a blocker, but figured it's worth raising for posterity

@@ -27,6 +28,16 @@ const App = ({ Component, pageProps }) => {
<AppProviders authorizationProps={{ ...pageProps }}>
<Metadata {...pageProps} />
<RestrictedRoute {...pageProps}>
<Script src="https://track.web3.storage/latest.js" data-hostname="web3.storage" data-ignore-pages="/callback" />
Copy link

Choose a reason for hiding this comment

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

what's the best place to understand what these scripts are doing? it would be helpful to have a comment in here with a pointer for folks who come later and are trying to figure out where scripts come from and how they work

@@ -0,0 +1,4 @@
{
Copy link

Choose a reason for hiding this comment

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

should this be checked in? genuinely unfamiliar with conventions for this team/repo so this is a "I am not sure, teach me" question not a "I am skeptical" one ;-p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually have a similar question. I was getting vscode plugin conflicts on all the imports and this seemed to solve it. I thought it may help but others but if this was just me I probably should ignore the settings from getting checked in.

@cmunns cmunns merged commit 4d5a1a0 into main Dec 1, 2022
@cmunns cmunns deleted the simple-analytics branch December 1, 2022 21:52
adamalton pushed a commit that referenced this pull request Dec 13, 2022
🤖 I have created a release *beep* *boop*
---


##
[2.33.0](website-v2.32.0...website-v2.33.0)
(2022-12-01)


### Features

* add tooltip to dashboard storage totals
([#2101](#2101))
([2056ec6](2056ec6)),
closes [#2081](#2081)
* list and ability to delete psa request created though deleted tokens
([#2054](#2054))
([e150d1f](e150d1f))
* simple analytics
([#2110](#2110))
([4d5a1a0](4d5a1a0))


### Bug Fixes

* styles missing from custom storage form
([#2135](#2135))
([04b66bc](04b66bc))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Ensure web analytics receives needed KPI data & prototype dashboards
4 participants