-
Notifications
You must be signed in to change notification settings - Fork 3
Record clicks via glean's builtin element click recording feature #154
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
Conversation
417372a to
558bb63
Compare
- Manually call the builtin api provided by Glean to record element click events
- Added data-element-label attribute for "Back to top" button to automatically record clicks on it
558bb63 to
3f0b1a2
Compare
rosahbruno
left a comment
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.
More generally, why are we choosing to use the GleanMetrics.recordElementClick in some places and then in others we are using data-glean-label?
Is there a reason we can't use data-glean-label in all the places?
@rosahbruno I just wanted to use both options in the code i.e. the automatic event capturing as well as calling manual apis. Apart from this, no other reason. If you think we should switch to using just one option, I am fine with that as well. I have no strong opinion on this one 👍🏾 |
|
@abhi-agg |
- Removed all click event recording API calls
- Automatic element click recording is not working on some elements, hence reverted back to using manual click recording
|
@rosahbruno I had made the switch to automatically capture clicks (i.e. via I investigated the issue and figured out that
Please let me know if that sounds good with you. |
rosahbruno
left a comment
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 changes look good. I think as long as we plan to work on those follow ups, we should be good here. I'm not sure why the div tags are causing issues, but I can take a look as well.
|
@rosahbruno I found out the issue and I believe I know how to fix it. I will update the details in the ticket https://bugzilla.mozilla.org/show_bug.cgi?id=1882747 today asap. |
This PR adds capability to record element clicks via Glean's builtin element click recording api (in mozilla/glean.js#1848).
Scenarios tested:
GleanMetrics.recordElementClick()api anddata-glean-labelattribute forBack to topbutton)Steps to test:
npm starton local machinewindow.Glean.setLogPings(true)in consoleglean.element_clickping is sent for each of the button clickEDIT: We are more interested in testing the automatic click event feature. Changed PR accordingly. See #154 (comment) for details.
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1884644