-
Notifications
You must be signed in to change notification settings - Fork 336
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: insights feedback functionality #10342
feat: insights feedback functionality #10342
Conversation
Hey @tianrunhe, would you mind reviewing this one? 🙏 Curious to get your thoughts on the Hopefully, |
Sorry I didn't quite follow why you need a new hook? Can you explain a little bit more? Thank you! |
@tianrunhe the But what if we don't want to track the event when the component loads, but when a user does an action, like submitting a form. At the moment, I think we can only handle this on the server, but creating a server mutation seems like overkill if you're not updating the db. So I created this hook so that we can track the action once an action happens, like submitting a form. |
(event: string, options: Record<string, any>) => { | ||
SendClientSideEvent(atmosphere, event, { | ||
...options, | ||
eventId: ++eventId |
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 eventId
here?
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 have it in the other hook as there seemed to be a bug where some users were sending it non-stop, and we were trying to debug whether it was happening from that hook or elsewhere.
If it is happening in the other hook, it's more likely to be related to the useEffect
which isn't in this hook. I'll remove it, and if the event fires non-stop too, I'll add eventId back.
Fix #10243
To test