Skip to content

Commit 2f225f5

Browse files
authored
Add a configurable default label in button click tracking and fix disabling for specific trackers (close #1397 and #1421)
PR #1423
1 parent 6c0dfa0 commit 2f225f5

File tree

9 files changed

+117
-16
lines changed

9 files changed

+117
-16
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/browser-plugin-button-click-tracking",
5+
"comment": "Allow per-tracker disabling",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/browser-plugin-button-click-tracking"
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/browser-plugin-button-click-tracking",
5+
"comment": "Add default label to avoid bad events",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/browser-plugin-button-click-tracking"
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/javascript-tracker",
5+
"comment": "Fix inactive button-click test",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/javascript-tracker"
10+
}

Diff for: plugins/browser-plugin-button-click-tracking/src/api.ts

+19-7
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,19 @@ export function enableButtonClickTracking(
5252
) {
5353
// Ensure that click tracking uses the latest configuration
5454
// In the case of `enableButtonClickTracking` being called multiple times in a row
55-
disableButtonClickTracking();
55+
disableButtonClickTracking(trackers);
5656

5757
trackers.forEach((trackerId) => {
5858
// Store the configuration for this tracker, if it doesn't already exist
5959
// This allows us to enable click tracking for a tracker if it has been disabled
6060
_listeners[trackerId] = (event: MouseEvent) => {
61-
eventHandler(event, trackerId, filterFunctionFromFilter(configuration.filter), configuration.context);
61+
eventHandler(
62+
event,
63+
trackerId,
64+
filterFunctionFromFilter(configuration.filter),
65+
configuration.defaultLabel,
66+
configuration.context
67+
);
6268
};
6369

6470
const addClickListener = () => {
@@ -80,12 +86,12 @@ export function enableButtonClickTracking(
8086
*
8187
* Can be re-enabled with {@link enableButtonClickTracking}
8288
*/
83-
export function disableButtonClickTracking() {
84-
for (const trackerId in _trackers) {
89+
export function disableButtonClickTracking(trackers: Array<string> = Object.keys(_trackers)) {
90+
trackers.forEach((trackerId) => {
8591
if (_listeners[trackerId]) {
8692
document.removeEventListener('click', _listeners[trackerId], true);
8793
}
88-
}
94+
});
8995
}
9096

9197
/**
@@ -96,12 +102,18 @@ export function disableButtonClickTracking() {
96102
* @param filter - The filter function to use for button click tracking
97103
* @param context - The dynamic context which will be evaluated for each button click event
98104
*/
99-
function eventHandler(event: MouseEvent, trackerId: string, filter: FilterFunction, context?: DynamicContext) {
105+
function eventHandler(
106+
event: MouseEvent,
107+
trackerId: string,
108+
filter: FilterFunction,
109+
defaultLabel?: string | ((element: HTMLElement) => string),
110+
context?: DynamicContext
111+
) {
100112
let elem = (event.composed ? event.composedPath()[0] : event.target) as HTMLElement | null;
101113
while (elem) {
102114
if (elem instanceof HTMLButtonElement || (elem instanceof HTMLInputElement && elem.type === 'button')) {
103115
if (filter(elem)) {
104-
const buttonClickEvent = createEventFromButton(elem);
116+
const buttonClickEvent = createEventFromButton(elem, defaultLabel);
105117
buttonClickEvent.context = resolveDynamicContext(context, buttonClickEvent, elem);
106118
trackButtonClick(buttonClickEvent, [trackerId]);
107119
}

Diff for: plugins/browser-plugin-button-click-tracking/src/types.ts

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export interface ButtonClickTrackingConfiguration {
1818
filter?: Filter;
1919
/** The dynamic context which will be evaluated for each button click event */
2020
context?: DynamicContext;
21+
/** A default label to use if one can not be determined by the content or data attributes */
22+
defaultLabel?: string | ((element: HTMLElement) => string);
2123
}
2224

2325
/**

Diff for: plugins/browser-plugin-button-click-tracking/src/util.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,19 @@ export function filterFunctionFromFilter(filter?: Filter): FilterFunction {
4242
* @returns The button click event
4343
*/
4444
export function createEventFromButton(
45-
button: HTMLButtonElement | HTMLInputElement
45+
button: HTMLButtonElement | HTMLInputElement,
46+
defaultLabel?: string | ((element: HTMLElement) => string)
4647
): ButtonClickEvent & CommonEventProperties {
4748
let ret = {} as ButtonClickEvent & CommonEventProperties;
4849

4950
if (button.tagName === 'INPUT') {
5051
ret.label = button.dataset.spButtonLabel || button.value;
5152
} else {
52-
ret.label = button.dataset.spButtonLabel || button.innerText;
53+
ret.label =
54+
button.dataset.spButtonLabel ||
55+
button.innerText ||
56+
(typeof defaultLabel === 'function' ? defaultLabel(button) : defaultLabel) ||
57+
'(empty)';
5358
}
5459

5560
button.id && (ret.id = button.id);

Diff for: plugins/browser-plugin-button-click-tracking/test/test.test.ts

+25
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,31 @@ describe('Button Click Tracking Plugin', () => {
145145
expect(createEventFromButton(button)).toEqual(event);
146146
});
147147

148+
it('should use specified default label', () => {
149+
const button = document.createElement('button');
150+
button.innerText = '';
151+
button.id = 'testId';
152+
button.className = 'testClass testClass2';
153+
button.name = 'testName';
154+
155+
const event1 = {
156+
label: 'defaultLabel',
157+
id: 'testId',
158+
classes: ['testClass', 'testClass2'],
159+
name: 'testName',
160+
};
161+
162+
expect(createEventFromButton(button, 'defaultLabel')).toEqual(event1);
163+
164+
const event2 = {
165+
label: 'testClass testClass2',
166+
id: 'testId',
167+
classes: ['testClass', 'testClass2'],
168+
name: 'testName',
169+
};
170+
expect(createEventFromButton(button, (btn) => btn.className)).toEqual(event2);
171+
});
172+
148173
it('should prefer the data-sp-button-label attribute over the innerText', () => {
149174
const button = document.createElement('button');
150175
button.innerText = 'testLabel';

Diff for: trackers/javascript-tracker/test/integration/buttonClick.test.ts

+24-4
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ describe('Snowplow Micro integration', () => {
3030
name?: string;
3131
};
3232

33-
const makeEvent = (button: Button, method: string) => {
33+
const makeEvent = (button: Button, method: string, suffix: string = '') => {
3434
return {
3535
event: {
3636
event: 'unstruct',
37-
app_id: 'button-click-tracking-' + testIdentifier,
37+
app_id: 'button-click-tracking-' + testIdentifier + suffix,
3838
page_url: `http://snowplow-js-tracker.local:8080/button-click-tracking.html?eventMethod=${method}`,
3939
unstruct_event: {
4040
data: {
@@ -81,6 +81,11 @@ describe('Snowplow Micro integration', () => {
8181
await (await $('#disabled-click')).click();
8282
await browser.pause(500);
8383

84+
await (await $('#selective')).click();
85+
await browser.pause(500);
86+
await (await $('#selective-click')).click();
87+
await browser.pause(500);
88+
8489
await (await $('#enable')).click();
8590
await browser.pause(500);
8691
await (await $('#enabled-click')).click();
@@ -155,14 +160,29 @@ describe('Snowplow Micro integration', () => {
155160
expect(logContains(ev)).toBe(false);
156161
});
157162

163+
it('should get one selective-click', () => {
164+
const ev1 = makeEvent({ id: 'selective-click', label: 'SelectiveEnabledClick' }, method);
165+
expect(logContains(ev1)).toBe(false);
166+
167+
const ev2 = makeEvent({ id: 'selective-click', label: 'SelectiveEnabledClick' }, method, '-second');
168+
logContainsButtonClick(ev2);
169+
});
170+
158171
it('should get enabled-click', () => {
159172
const ev = makeEvent({ id: 'enabled-click', label: 'EnabledClick' }, method);
160173
logContainsButtonClick(ev);
161174
});
162175

163176
it('should get `final-config` as it is the last config set', () => {
164-
const ev = makeEvent({ id: 'final-config', classes: ['final-config'], label: 'Final Config' }, method);
165-
logContainsButtonClick(ev);
177+
const ev1 = makeEvent({ id: 'final-config', classes: ['final-config'], label: 'Final Config' }, method);
178+
logContainsButtonClick(ev1);
179+
180+
const ev2 = makeEvent(
181+
{ id: 'final-config', classes: ['final-config'], label: 'Final Config' },
182+
method,
183+
'-second'
184+
);
185+
expect(logContains(ev2)).toBe(false);
166186
});
167187
});
168188
});

Diff for: trackers/javascript-tracker/test/pages/button-click-tracking.html

+10-3
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,20 @@
5959
<button id="disable" onclick="snowplow('disableButtonClickTracking')">Disable</button>
6060
<button id="disabled-click">DisabledClick</button>
6161

62+
<button id="selective" onclick="snowplow('enableButtonClickTracking:sp2')">Selective enable</button>
63+
<button id="selective-click">SelectiveEnabledClick</button>
64+
6265
<button id="enable" onclick="snowplow('enableButtonClickTracking')">Enable</button>
6366
<button id="enabled-click">EnabledClick</button>
6467

6568
<!-- Ensuring the final config is used after multiple calls to `enableButtonClickTracking` -->
66-
<button id="set-multiple-configs" onClick="setMultipleConfigs">Set Multiple Configs</button>
69+
<button id="set-multiple-configs" onClick="setMultipleConfigs()">Set Multiple Configs</button>
6770
<button id="final-config" class="final-config">Final Config</button>
6871

6972
<script>
7073
function setMultipleConfigs() {
71-
enableButtonClickTracking({ denylist: ['final-config'] });
72-
enableButtonClickTracking();
74+
snowplow('enableButtonClickTracking', { filter: { denylist: ['final-config'] } });
75+
snowplow('enableButtonClickTracking:sp');
7376
}
7477
</script>
7578

@@ -112,6 +115,10 @@
112115
appId: 'button-click-tracking-' + testIdentifier,
113116
method: parseQuery().eventMethod,
114117
});
118+
snowplow('newTracker', 'sp2', collector_endpoint, {
119+
appId: 'button-click-tracking-' + testIdentifier + '-second',
120+
method: parseQuery().eventMethod,
121+
});
115122
snowplow(function () {
116123
document.getElementById('init').innerText = 'true';
117124
});

0 commit comments

Comments
 (0)