Skip to content

Commit be129db

Browse files
mydeamydea
authored
feat(replay): Improve click target detection (#8026)
1 parent d8cf8d3 commit be129db

File tree

4 files changed

+164
-19
lines changed

4 files changed

+164
-19
lines changed

‎packages/browser-integration-tests/suites/replay/customEvents/template.html

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,9 @@
55
</head>
66
<body>
77
<div role="button" id="error" class="btn btn-error" aria-label="An Error">An Error</div>
8-
<button>
9-
<img id="img"
10-
alt="Alt Text"
11-
/>
12-
</button>
13-
<button class="sentry-unmask" aria-label="Unmasked label">
14-
Unmasked
8+
<button title="Button title">
9+
<img id="img" alt="Alt Text" />
1510
</button>
11+
<button class="sentry-unmask" aria-label="Unmasked label">Unmasked</button>
1612
</body>
1713
</html>

‎packages/browser-integration-tests/suites/replay/customEvents/test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,15 @@ sentryTest(
135135
expect.arrayContaining([
136136
{
137137
...expectedClickBreadcrumb,
138-
message: 'body > button > img#img[alt="Alt Text"]',
138+
message: 'body > button[title="Button title"]',
139139
data: {
140140
nodeId: expect.any(Number),
141141
node: {
142142
attributes: {
143-
alt: 'Alt Text',
144-
id: 'img',
143+
title: '****** *****',
145144
},
146145
id: expect.any(Number),
147-
tagName: 'img',
146+
tagName: 'button',
148147
textContent: '',
149148
},
150149
},

‎packages/replay/src/coreHandlers/handleDom.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { createBreadcrumb } from '../util/createBreadcrumb';
88
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
99
import { getAttributesToRecord } from './util/getAttributesToRecord';
1010

11-
interface DomHandlerData {
11+
export interface DomHandlerData {
1212
name: string;
1313
event: Node | { target: Node };
1414
}
@@ -31,15 +31,18 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa
3131

3232
/**
3333
* An event handler to react to DOM events.
34+
* Exported for tests only.
3435
*/
35-
function handleDom(handlerData: DomHandlerData): Breadcrumb | null {
36+
export function handleDom(handlerData: DomHandlerData): Breadcrumb | null {
3637
let target;
3738
let targetNode: Node | INode | undefined;
3839

40+
const isClick = handlerData.name === 'click';
41+
3942
// Accessing event.target can throw (see getsentry/raven-js#838, #768)
4043
try {
41-
targetNode = getTargetNode(handlerData);
42-
target = htmlTreeAsString(targetNode);
44+
targetNode = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event);
45+
target = htmlTreeAsString(targetNode, { maxStringLength: 200 });
4346
} catch (e) {
4447
target = '<unknown>';
4548
}
@@ -73,12 +76,29 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null {
7376
});
7477
}
7578

76-
function getTargetNode(handlerData: DomHandlerData): Node {
77-
if (isEventWithTarget(handlerData.event)) {
78-
return handlerData.event.target;
79+
function getTargetNode(event: DomHandlerData['event']): Node {
80+
if (isEventWithTarget(event)) {
81+
return event.target;
82+
}
83+
84+
return event;
85+
}
86+
87+
const INTERACTIVE_SELECTOR = 'button,a';
88+
89+
// For clicks, we check if the target is inside of a button or link
90+
// If so, we use this as the target instead
91+
// This is useful because if you click on the image in <button><img></button>,
92+
// The target will be the image, not the button, which we don't want here
93+
function getClickTargetNode(event: DomHandlerData['event']): Node {
94+
const target = getTargetNode(event);
95+
96+
if (!target || !(target instanceof Element)) {
97+
return target;
7998
}
8099

81-
return handlerData.event;
100+
const closestInteractive = target.closest(INTERACTIVE_SELECTOR);
101+
return closestInteractive || target;
82102
}
83103

84104
function isEventWithTarget(event: unknown): event is { target: Node } {

‎packages/replay/test/unit/coreHandlers/handleDom.test.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import type { DomHandlerData } from '../../../src/coreHandlers/handleDom';
2+
import { handleDom } from '../../../src/coreHandlers/handleDom';
3+
4+
describe('Unit | coreHandlers | handleDom', () => {
5+
test('it works with a basic click event on a div', () => {
6+
const parent = document.createElement('body');
7+
const target = document.createElement('div');
8+
target.classList.add('my-class', 'other-class');
9+
parent.appendChild(target);
10+
11+
const handlerData: DomHandlerData = {
12+
name: 'click',
13+
event: {
14+
target,
15+
},
16+
};
17+
const actual = handleDom(handlerData);
18+
expect(actual).toEqual({
19+
category: 'ui.click',
20+
data: {},
21+
message: 'body > div.my-class.other-class',
22+
timestamp: expect.any(Number),
23+
type: 'default',
24+
});
25+
});
26+
27+
test('it works with a basic click event on a button', () => {
28+
const parent = document.createElement('body');
29+
const target = document.createElement('button');
30+
target.classList.add('my-class', 'other-class');
31+
parent.appendChild(target);
32+
33+
const handlerData: DomHandlerData = {
34+
name: 'click',
35+
event: {
36+
target,
37+
},
38+
};
39+
const actual = handleDom(handlerData);
40+
expect(actual).toEqual({
41+
category: 'ui.click',
42+
data: {},
43+
message: 'body > button.my-class.other-class',
44+
timestamp: expect.any(Number),
45+
type: 'default',
46+
});
47+
});
48+
49+
test('it works with a basic click event on a span inside of <button>', () => {
50+
const parent = document.createElement('body');
51+
const interactive = document.createElement('button');
52+
interactive.classList.add('my-class', 'other-class');
53+
parent.appendChild(interactive);
54+
55+
const target = document.createElement('span');
56+
interactive.appendChild(target);
57+
58+
const handlerData: DomHandlerData = {
59+
name: 'click',
60+
event: {
61+
target,
62+
},
63+
};
64+
const actual = handleDom(handlerData);
65+
expect(actual).toEqual({
66+
category: 'ui.click',
67+
data: {},
68+
message: 'body > button.my-class.other-class',
69+
timestamp: expect.any(Number),
70+
type: 'default',
71+
});
72+
});
73+
74+
test('it works with a basic click event on a span inside of <a>', () => {
75+
const parent = document.createElement('body');
76+
const interactive = document.createElement('a');
77+
interactive.classList.add('my-class', 'other-class');
78+
parent.appendChild(interactive);
79+
80+
const target = document.createElement('span');
81+
interactive.appendChild(target);
82+
83+
const handlerData: DomHandlerData = {
84+
name: 'click',
85+
event: {
86+
target,
87+
},
88+
};
89+
const actual = handleDom(handlerData);
90+
expect(actual).toEqual({
91+
category: 'ui.click',
92+
data: {},
93+
message: 'body > a.my-class.other-class',
94+
timestamp: expect.any(Number),
95+
type: 'default',
96+
});
97+
});
98+
99+
test('it works with very deep nesting', () => {
100+
const parent = document.createElement('body');
101+
102+
let current: HTMLElement = parent;
103+
for (let i = 0; i < 20; i++) {
104+
const next = document.createElement('div');
105+
next.classList.add(`level-${i}`, `level-other-${i}`);
106+
current.appendChild(next);
107+
current = next;
108+
}
109+
110+
const target = document.createElement('div');
111+
target.classList.add('my-class', 'other-class');
112+
current.appendChild(target);
113+
114+
const handlerData: DomHandlerData = {
115+
name: 'click',
116+
event: {
117+
target,
118+
},
119+
};
120+
const actual = handleDom(handlerData);
121+
expect(actual).toEqual({
122+
category: 'ui.click',
123+
data: {},
124+
message:
125+
'div.level-16.level-other-16 > div.level-17.level-other-17 > div.level-18.level-other-18 > div.level-19.level-other-19 > div.my-class.other-class',
126+
timestamp: expect.any(Number),
127+
type: 'default',
128+
});
129+
});
130+
});

0 commit comments

Comments
 (0)