Add analytics to homepage #5278
Conversation
🦋 Changeset detectedLatest commit: 921efcb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Cloud11PL
force-pushed
the
MERX-1273-analytics-home-page
branch
from
2a4c06f
to
516659b
Compare
| "saleor-dashboard": patch | ||
| --- | ||
|
|
||
| Environments created via Saleor Cloud now identify and report to PostHog. This means Dashboard now sends telemetry data regarding home page onboarding steps and links. |
There was a problem hiding this comment.
I dont see where exactly it checks if this is Saleor Cloud only?
This PR couples open source dashboard with a 3rd party vendor. For non-cloud builds it can't be opt out and the code is loaded to the bundle.
I suggest to write a simple facade pattern that will
- check for env variable like ENABLE_ANALYTICS or ENABLE_POSHOG. Perform no-op if not enabled
- If analytics enabled, dynamically load library, instead of bundling it eagerly
- Use custom methods and hooks and call posthog behind it, so the codebase is not tight coupled with posthog
you can consider this
https://github.com/DavidWells/analytics/tree/master
or at least get inspired (to run analytics.track() instead posthog.track()
There was a problem hiding this comment.
Dashboard has been coupled with Posthog for the past 9 months, it just wasn't (and still isn't) enabled. Posthog will be enabled when Dashboard is in a Cloud instance and has posthog api keys and host set in environment variables -> https://github.com/saleor/saleor-dashboard/blob/main/src/components/ProductAnalytics/index.tsx#L14, for non-cloud builds nothing has changed.
Cloud11PL
force-pushed
the
MERX-1273-analytics-home-page
branch
from
29395e4
to
886040b
Compare
This reverts commit a92531b.
Cloud11PL
force-pushed
the
MERX-1273-analytics-home-page
branch
from
2451d4a
to
92fee57
Compare
| @@ -22,10 +23,12 @@ const getStepsData = ({ | |||
| intl, | |||
| isStepCompleted, | |||
| onStepComplete, | |||
| posthogCapture, | |||
There was a problem hiding this comment.
when you have abstraction, then you should not refer to the name "Posthog" anywhere apart the file where you build the abstraction
| @@ -0,0 +1,28 @@ | |||
| import { usePostHog } from "posthog-js/react"; | |||
|
|
|||
| export function useDashboardAnalytics() { | |||
There was a problem hiding this comment.
so perhaps just "useAnalytics" as the file name indicates?
What type of PR is this?
Related Issues or Documents
Usage Instructions, Screenshots, Recordings
Have you written tests?
[Optional] Description