-
Notifications
You must be signed in to change notification settings - Fork 1
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
140 integrate additional events into google analytics query into the prototype #255
140 integrate additional events into google analytics query into the prototype #255
Conversation
✅ Deploy Preview for pendulum-pay ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Looks very good!
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.
Great changes! ✅ Just minor things
src/contexts/events.tsx
Outdated
}); | ||
|
||
previousChainId.current = chainId; | ||
}, [chainId, previousChainId]); |
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.
missing dependency: trackEvent
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.
previousChainId
is a useRef and it doesn't change across renders. We don't have to include it in the dependency array
src/contexts/events.tsx
Outdated
|
||
trackEvent({ | ||
event: 'network_change', | ||
from_network: previousChainId.current || chainId, |
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.
We already check if previousChainId.current
is undefined, we don't have to || chainId
Nice catch @Sharqiewicz, addressed all your comments ✔️ |
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.
Tested a few times normal cases and works okay 👍
It took me some time to figure out how to be able to properly watch the connected network. I noticed that the watchChainId and useChainId both don't fire when switching from or to a non-supported network, ie a network that is not defined in our
wagmiConfig
. Since onlypolygon
is defined in ourwagmiConfig
, it never triggered when switching from any network to polygon.Instead, we are now using the
chainId
contained in the data of the useAccount hook as it's always updated when the network changes.GA config
New Variables
DLV | from_network
andDLV | to_network
DLV | account_address
New triggers
DL Event | network_change
Events
GA4 Event | network_change
from_network
- >{{DLV | from_network }}
andto_network
->{{DLV | to_network}}
DL Event | network_change
GA4 Event | wallet_connect
account_address
->{{DLV | account_address}}
Closes #140.