-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add ability to enable only form_change, or form_submit, focus_form or any combination (close #371) #1065
Add ability to enable only form_change, or form_submit, focus_form or any combination (close #371) #1065
Conversation
Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://docs.snowplowanalytics.com/docs/contributing/contributor-license-agreement/ to learn more and sign. Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks. |
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.
LGTM! Tiny nitpick about the test description, but the code seems fine to me.
@@ -795,4 +804,36 @@ describe('Auto tracking', () => { | |||
}) | |||
).toBe(false); | |||
}); | |||
|
|||
it('should only track focus_form and not change_form events events when focus_form events are filtered', () => { |
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.
nitpick: duplicate 'events' in the test desc:
change_form events events when focus_form
6af9daa
to
d4c84e5
Compare
Issue #371
This PR adds ability to configure which of the three types of form events to track. It adds an option to the
enableFormTracking
call to list the events to be tracked.API
The
events
option is optional and, if not present, all three events are tracked.Docs
Here is a PR in data-value-resources repo for the documentation changes.
Implementation
The list of subscribed events is checked in
addFormListeners
before adding event listeners to form fields (for focus and change) and the form (for submit).I added an end-to-end test that checks that change events are not tracked if not subscribed to.
Also tried it out in a demo ReactJS app, but that's not part of this PR.