-
Notifications
You must be signed in to change notification settings - Fork 33
feat: add analytics tracking to home page elements #1701
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
|
||
| return ( | ||
| <Button ref={ref} {...props} display="flex" gap="2" variant="tertiary"> | ||
| <Button |
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.
The filters button doesn't seem to be working after this change, the handler function probably just eats the click event.
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.
Fixed here: 22f7c8b
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 don't know why we have to do that (events should propagate unless otherwise stated), but it should do for now.
| }, [isOpen, onClose]) | ||
|
|
||
| const handleToggle = () => { | ||
| trackEvent(AnalyticsEvent.ClickNavBuild) |
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.
The track event code does not seem to differentiate between prod and the dev environments. That could probably skew the results a little.
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.
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 see there is some code before that which mades it only run on prod:
// Optional: Only track on production; remove these two lines if you want to track other environments
if (!isProd) return
The zen.balancer.fi does not point to anything though, we can remove it.
| }) { | ||
| const { linkColorFor } = useNav() | ||
|
|
||
| const handleLinkClick = (analyticsEvent?: string) => { |
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.
Some of the code where we are tracking events is shared with Beets, do we want to track Beets events?
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.
Nice catch. We definitely do not want to track events from Beets.
I have updated the code to only track on Balancer events here :3c7a918
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.
it is probably better to wrap the trackEvent function with our own one, so we don't have to add the if (isBalancer) ... logic everywhere.

Uh oh!
There was an error while loading. Please reload this page.