-
Notifications
You must be signed in to change notification settings - Fork 2
Enable telemetry if cookies are accepted #235
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
base: main
Are you sure you want to change the base?
Conversation
| // Suppress CORS errors in local development | ||
| const isLocalhost = window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1'; | ||
| if (isLocalhost) { | ||
| console.log('[Analytics] Skipping analytics in local development (CORS restriction)'); | ||
| } else { | ||
| console.error('[Analytics] Fetch error: ', error); | ||
| } |
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.
Why do we need to suppress CORS errors in local development?
| // Skip analytics in local development to avoid CORS errors | ||
| const isLocalhost = window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1'; | ||
| if (isLocalhost) { | ||
| console.log('[Analytics] Skipping analytics in local development environment.'); | ||
| return; | ||
| } |
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.
I would argue it's preferred to have CORS errors in local development (and use appropriate workarounds) so that these errors don't get uncaught and show up in prod.
| const consent = localStorage.getItem('deepl_cookie_consent'); | ||
|
|
||
| // Only hide banner if user has explicitly accepted or rejected | ||
| if (consent === 'accepted' || consent === 'rejected') return; |
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.
These should be global variables, not magic strings.
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.
Updated
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.
Don't see the change?
This will enable us to collect additional insights on our docs if users opt-in to cookies.