-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: product analytic #5099
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: product analytic #5099
Conversation
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.
Caution
Changes requested ❌
Reviewed everything up to ee9f0a6 in 1 minute and 46 seconds. Click for details.
- Reviewed
483
lines of code in10
files - Skipped
0
files when reviewing. - Skipped posting
1
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/providers/AnalyticProvider.tsx:10
- Draft comment:
Consider guarding against multiple initializations of PostHog. If productAnalytic is toggled during runtime, re-running posthog.init may lead to duplicate registrations. Adding a check to initialize only once would improve robustness. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid technical concern - reinitializing PostHog could be problematic. However, looking at the PostHog documentation would be needed to know if this is actually an issue. Without that context, we can't be certain this is a real problem. The code is also following React patterns correctly with useEffect. I don't have access to PostHog documentation to verify if multiple initializations are actually harmful. The current code follows standard React patterns. Without clear evidence that multiple initializations are harmful, this comment is speculative and could lead to unnecessary complexity. Delete the comment since it's speculative without strong evidence that multiple initializations are actually problematic.
Workflow ID: wflow_HmMiaDuUMMqYQShD
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed 78a0330 in 45 seconds. Click for details.
- Reviewed
30
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/hooks/useAnalytic.ts:30
- Draft comment:
Good fix: Renamed 'ProductAnalyticPrompState' to 'ProductAnalyticPromptState' for clarity and consistency. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. web-app/src/hooks/useAnalytic.ts:38
- Draft comment:
Removal of the outdated comment 'Initialize isDark based on OS preference if theme is auto' cleans up confusion, as it doesn’t relate to analytics state. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. web-app/src/hooks/useAnalytic.ts:40
- Draft comment:
Consider removing the 'async' keyword on setter functions if no asynchronous operations are performed. This can simplify the implementation. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_Lf5Iq5Gbk5tud3hy
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
This is the build for this pull request. You can download it from the Artifacts section here: Build URL. |
This is the build for this pull request. You can download it from the Artifacts section here: Build URL. |
This is the build for this pull request. You can download it from the Artifacts section here: Build URL. |
Describe Your Changes
This pull request introduces a product analytics feature using PostHog to collect anonymous usage data. Key changes include the integration of PostHog, a new analytics consent prompt, state management for user preferences, and updates to the app's configuration and environment setup.
Product Analytics Integration:
posthog-js
as a dependency inpackage.json
for analytics functionality.AnalyticProvider
to initialize PostHog with environment variables and manage user consent for data collection.User Consent Management:
PromptAnalytic
component to display a user prompt for opting into or out of analytics.useAnalytic
,useProductAnalyticPrompt
, anduseProductAnalytic
hooks using Zustand for managing analytics-related states and storing preferences inlocalStorage
.UI Updates:
App Configuration:
vite.config.ts
to loadPOSTHOG_KEY
andPOSTHOG_HOST
from environment files. [1] [2]analytic.ts
.Fixes Issues
Posthog



Self Checklist
Important
Introduces product analytics using PostHog with user consent management and updates to app configuration.
posthog-js
topackage.json
for analytics.AnalyticProvider
inAnalyticProvider.tsx
to initialize PostHog and manage user consent.PromptAnalytic
component inPromptAnalytic.tsx
for user consent prompt.useAnalytic
,useProductAnalyticPrompt
, anduseProductAnalytic
hooks inuseAnalytic.ts
for state management using Zustand.privacy.tsx
to include analytics toggle.__root.tsx
based on user preferences.vite.config.ts
to loadPOSTHOG_KEY
andPOSTHOG_HOST
from environment variables.analytic.ts
to manage distinct IDs for PostHog.This description was created by
for 78a0330. You can customize this summary. It will automatically update as commits are pushed.