Skip to content

Conversation

@shirgoldbird
Copy link
Member

@shirgoldbird shirgoldbird commented Jan 20, 2026

This will enable us to collect additional insights on our docs if users opt-in to cookies.

⚠️ Claude Coded, human reviewed 😄

Comment on lines +152 to +158
// 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);
}
Copy link
Member

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?

Comment on lines +398 to +403
// 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;
}
Copy link
Member

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;
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

Copy link
Member

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?

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.

3 participants