Skip to content

Add a configurable default label in button click tracking and fix disabling for specific trackers (close #1397 and #1421) #1423

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

Merged

Conversation

jethron
Copy link
Contributor

@jethron jethron commented Apr 4, 2025

  • Makes it possible to disable button click tracking for specific trackers (see disableButtonClickTracking disables tracking of buttons for all trackers #1397 but this will only fix for v4)
  • Fixes an issue where enabling button click tracking for specific trackers would disable it for all other trackers
  • Add a default label ((empty)) for button_click events where one can not be determined by the button's content or data attributes (see Add empty string to button_click event #1421)
  • Make the default label configurable (static string or via callback for the element)
  • Fixes an integration test that wasn't running properly and so wasn't actually testing the right thing

Technically a breaking change, but only in the sense that events that were previously bad will not be any more. A fixing change? So aiming for 4.5.0 since a minor release seems appropriate.

@jethron jethron requested review from matus-tomlein and greg-el April 4, 2025 07:10
@@ -18,6 +18,8 @@ export interface ButtonClickTrackingConfiguration {
filter?: Filter;
/** The dynamic context which will be evaluated for each button click event */
context?: DynamicContext;
/** A default label to use if one can not be determined by the content or data attributes */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, if this is not specified, the default will be (empty), should we note this here or somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this will be more visible in the tracker docs.

@matus-tomlein matus-tomlein changed the title button-click-tracking fixes Add a configurable default label in button click tracking and fix disabling for specific trackers (close #1397, #1421) Apr 4, 2025
@matus-tomlein matus-tomlein changed the title Add a configurable default label in button click tracking and fix disabling for specific trackers (close #1397, #1421) Add a configurable default label in button click tracking and fix disabling for specific trackers (close #1397 and #1421) Apr 4, 2025
Copy link
Contributor

@matus-tomlein matus-tomlein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍 Happy to include this in the 4.5 release

@matus-tomlein matus-tomlein merged commit 2f225f5 into snowplow:release/4.5.0 Apr 4, 2025
1 check failed
matus-tomlein pushed a commit that referenced this pull request Apr 4, 2025
@jethron jethron deleted the issue/button-click-disable branch April 7, 2025 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants