Skip to content

Commit 5f4e7dc

Browse files
BenoitZugmeyerBenoitZugmeyer
committed
👌 split onClick and onPointerDown processing
1 parent d0a61ff commit 5f4e7dc

3 files changed

Lines changed: 108 additions & 84 deletions

File tree

‎packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,57 @@
11
import type { Clock } from '../../../../../core/test/specHelper'
22
import { createNewEvent, mockClock } from '../../../../../core/test/specHelper'
3-
import type { OnClickCallback, OnPointerDownCallback } from './listenActionEvents'
3+
import type { ActionEventsHooks } from './listenActionEvents'
44
import { listenActionEvents } from './listenActionEvents'
55

66
describe('listenActionEvents', () => {
7-
let onClickSpy: jasmine.Spy<OnClickCallback>
8-
let onPointerDownSpy: jasmine.Spy<OnPointerDownCallback>
7+
let actionEventsHooks: {
8+
onClick: jasmine.Spy<ActionEventsHooks<object>['onClick']>
9+
onPointerDown: jasmine.Spy<ActionEventsHooks<object>['onPointerDown']>
10+
}
911
let stopListenEvents: () => void
1012

1113
beforeEach(() => {
12-
onClickSpy = jasmine.createSpy()
13-
onPointerDownSpy = jasmine.createSpy().and.returnValue({ onClick: onClickSpy })
14-
;({ stop: stopListenEvents } = listenActionEvents({ onPointerDown: onPointerDownSpy }))
14+
actionEventsHooks = {
15+
onClick: jasmine.createSpy(),
16+
onPointerDown: jasmine.createSpy().and.returnValue({}),
17+
}
18+
;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks))
1519
})
1620

1721
afterEach(() => {
1822
stopListenEvents()
1923
})
2024

25+
it('listen to mousedown events', () => {
26+
emulateClick()
27+
expect(actionEventsHooks.onPointerDown).toHaveBeenCalledOnceWith(jasmine.objectContaining({ type: 'pointerdown' }))
28+
})
29+
2130
it('listen to click events', () => {
2231
emulateClick()
23-
expect(onClickSpy).toHaveBeenCalledOnceWith({
24-
event: jasmine.objectContaining({ type: 'click' }),
25-
getUserActivity: jasmine.any(Function),
26-
})
32+
expect(actionEventsHooks.onClick).toHaveBeenCalledOnceWith(
33+
{},
34+
jasmine.objectContaining({ type: 'click' }),
35+
jasmine.any(Function)
36+
)
2737
})
2838

2939
it('aborts click lifecycle if the pointerdown event occurs on a non-element', () => {
3040
emulateClick({ target: document.createTextNode('foo') })
31-
expect(onClickSpy).not.toHaveBeenCalled()
41+
expect(actionEventsHooks.onPointerDown).not.toHaveBeenCalled()
3242
})
3343

3444
it('can abort click lifecycle by returning undefined from the onPointerDown callback', () => {
35-
onPointerDownSpy.and.returnValue(undefined)
45+
actionEventsHooks.onPointerDown.and.returnValue(undefined)
46+
emulateClick()
47+
expect(actionEventsHooks.onClick).not.toHaveBeenCalled()
48+
})
49+
50+
it('passes the context created in onPointerDown to onClick', () => {
51+
const context = {}
52+
actionEventsHooks.onPointerDown.and.returnValue(context)
3653
emulateClick()
37-
expect(onClickSpy).not.toHaveBeenCalled()
54+
expect(actionEventsHooks.onClick.calls.mostRecent().args[0]).toBe(context)
3855
})
3956

4057
describe('selection change', () => {
@@ -106,7 +123,7 @@ describe('listenActionEvents', () => {
106123
})
107124

108125
function hasSelectionChanged() {
109-
return onClickSpy.calls.mostRecent().args[0].getUserActivity().selection
126+
return actionEventsHooks.onClick.calls.mostRecent().args[2]().selection
110127
}
111128

112129
function emulateNodeSelection(
@@ -171,7 +188,7 @@ describe('listenActionEvents', () => {
171188
window.dispatchEvent(createNewEvent('input'))
172189
}
173190
function hasInputUserActivity() {
174-
return onClickSpy.calls.mostRecent().args[0].getUserActivity().input
191+
return actionEventsHooks.onClick.calls.mostRecent().args[2]().input
175192
}
176193
})
177194

‎packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,17 @@ import { addEventListener, DOM_EVENT, monitor } from '@datadog/browser-core'
22

33
export type MouseEventOnElement = MouseEvent & { target: Element }
44

5-
export type OnPointerDownCallback = (event: MouseEventOnElement) => { onClick: OnClickCallback } | undefined
6-
export type OnClickCallback = (context: OnClickContext) => void
7-
export interface OnClickContext {
8-
event: MouseEventOnElement
9-
getUserActivity(): { selection: boolean; input: boolean }
5+
export type GetUserActivity = () => { selection: boolean; input: boolean }
6+
export interface ActionEventsHooks<ClickContext> {
7+
onPointerDown: (event: MouseEventOnElement) => ClickContext | undefined
8+
onClick: (context: ClickContext, event: MouseEventOnElement, getUserActivity: GetUserActivity) => void
109
}
1110

12-
export function listenActionEvents({ onPointerDown }: { onPointerDown: OnPointerDownCallback }) {
11+
export function listenActionEvents<ClickContext>({ onPointerDown, onClick }: ActionEventsHooks<ClickContext>) {
1312
let hasSelectionChanged = false
1413
let selectionEmptyAtPointerDown: boolean
1514
let hasInputChanged = false
16-
let onClick: ((context: OnClickContext) => void) | undefined
15+
let clickContext: ClickContext | undefined
1716

1817
const listeners = [
1918
addEventListener(
@@ -23,7 +22,7 @@ export function listenActionEvents({ onPointerDown }: { onPointerDown: OnPointer
2322
hasSelectionChanged = false
2423
selectionEmptyAtPointerDown = isSelectionEmpty()
2524
if (isMouseEventOnElement(event)) {
26-
onClick = onPointerDown(event)?.onClick
25+
clickContext = onPointerDown(event)
2726
}
2827
},
2928
{ capture: true }
@@ -44,7 +43,7 @@ export function listenActionEvents({ onPointerDown }: { onPointerDown: OnPointer
4443
window,
4544
DOM_EVENT.CLICK,
4645
(clickEvent: MouseEvent) => {
47-
if (isMouseEventOnElement(clickEvent) && onClick) {
46+
if (isMouseEventOnElement(clickEvent) && clickContext) {
4847
// Use a scoped variable to make sure the value is not changed by other clicks
4948
const userActivity = {
5049
selection: hasSelectionChanged,
@@ -58,11 +57,8 @@ export function listenActionEvents({ onPointerDown }: { onPointerDown: OnPointer
5857
)
5958
}
6059

61-
onClick({
62-
event: clickEvent,
63-
getUserActivity: () => userActivity,
64-
})
65-
onClick = undefined
60+
onClick(clickContext, clickEvent, () => userActivity)
61+
clickContext = undefined
6662
}
6763
},
6864
{ capture: true }

‎packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts‎

Lines changed: 66 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type { ClickChain } from './clickChain'
2323
import { createClickChain } from './clickChain'
2424
import { getActionNameFromElement } from './getActionNameFromElement'
2525
import { getSelectorsFromElement } from './getSelectorsFromElement'
26-
import type { MouseEventOnElement, OnClickContext } from './listenActionEvents'
26+
import type { MouseEventOnElement, GetUserActivity } from './listenActionEvents'
2727
import { listenActionEvents } from './listenActionEvents'
2828
import { computeFrustration } from './computeFrustration'
2929

@@ -78,16 +78,19 @@ export function trackClickActions(
7878
lifeCycle.subscribe(LifeCycleEventType.BEFORE_UNLOAD, stopClickChain)
7979
lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, stopClickChain)
8080

81-
const { stop: stopActionEventsListener } = listenActionEvents({
82-
onPointerDown: (pointerDownEvent) =>
83-
onPointerDown(
81+
const { stop: stopActionEventsListener } = listenActionEvents<ClickActionBase>({
82+
onPointerDown: (pointerDownEvent) => onPointerDown(configuration, history, pointerDownEvent),
83+
onClick: (clickActionBase, clickEvent, getUserActivity) =>
84+
onClick(
8485
configuration,
8586
lifeCycle,
8687
domMutationObservable,
8788
history,
8889
stopObservable,
8990
appendClickToClickChain,
90-
pointerDownEvent
91+
clickActionBase,
92+
clickEvent,
93+
getUserActivity
9194
),
9295
})
9396

@@ -123,11 +126,7 @@ export function trackClickActions(
123126

124127
function onPointerDown(
125128
configuration: RumConfiguration,
126-
lifeCycle: LifeCycle,
127-
domMutationObservable: Observable<void>,
128129
history: ClickActionIdHistory,
129-
stopObservable: Observable<void>,
130-
appendClickToClickChain: (click: Click) => void,
131130
pointerDownEvent: MouseEventOnElement
132131
) {
133132
if (!configuration.trackFrustrations && history.find()) {
@@ -143,59 +142,71 @@ function onPointerDown(
143142
return
144143
}
145144

146-
return {
147-
onClick({ event: clickEvent, getUserActivity }: OnClickContext) {
148-
const click = newClick(lifeCycle, history, getUserActivity, clickActionBase, clickEvent)
145+
return clickActionBase
146+
}
149147

150-
if (configuration.trackFrustrations) {
151-
appendClickToClickChain(click)
152-
}
148+
function onClick(
149+
configuration: RumConfiguration,
150+
lifeCycle: LifeCycle,
151+
domMutationObservable: Observable<void>,
152+
history: ClickActionIdHistory,
153+
stopObservable: Observable<void>,
154+
appendClickToClickChain: (click: Click) => void,
155+
clickActionBase: ClickActionBase,
156+
clickEvent: MouseEventOnElement,
157+
getUserActivity: GetUserActivity
158+
) {
159+
const click = newClick(lifeCycle, history, getUserActivity, clickActionBase, clickEvent)
153160

154-
const { stop: stopWaitPageActivityEnd } = waitPageActivityEnd(
155-
lifeCycle,
156-
domMutationObservable,
157-
configuration,
158-
(pageActivityEndEvent) => {
159-
if (pageActivityEndEvent.hadActivity && pageActivityEndEvent.end < click.startClocks.timeStamp) {
160-
// If the clock is looking weird, just discard the click
161+
if (configuration.trackFrustrations) {
162+
appendClickToClickChain(click)
163+
}
164+
165+
const { stop: stopWaitPageActivityEnd } = waitPageActivityEnd(
166+
lifeCycle,
167+
domMutationObservable,
168+
configuration,
169+
(pageActivityEndEvent) => {
170+
if (pageActivityEndEvent.hadActivity && pageActivityEndEvent.end < click.startClocks.timeStamp) {
171+
// If the clock is looking weird, just discard the click
172+
click.discard()
173+
} else {
174+
click.stop(pageActivityEndEvent.hadActivity ? pageActivityEndEvent.end : undefined)
175+
176+
// Validate or discard the click only if we don't track frustrations. It'll be done when
177+
// the click chain is finalized.
178+
if (!configuration.trackFrustrations) {
179+
if (!pageActivityEndEvent.hadActivity) {
180+
// If we are not tracking frustrations, we should discard the click to keep backward
181+
// compatibility.
161182
click.discard()
162183
} else {
163-
click.stop(pageActivityEndEvent.hadActivity ? pageActivityEndEvent.end : undefined)
164-
165-
// Validate or discard the click only if we don't track frustrations. It'll be done when
166-
// the click chain is finalized.
167-
if (!configuration.trackFrustrations) {
168-
if (!pageActivityEndEvent.hadActivity) {
169-
// If we are not tracking frustrations, we should discard the click to keep backward
170-
// compatibility.
171-
click.discard()
172-
} else {
173-
click.validate()
174-
}
175-
}
184+
click.validate()
176185
}
177-
},
178-
CLICK_ACTION_MAX_DURATION
179-
)
186+
}
187+
}
188+
},
189+
CLICK_ACTION_MAX_DURATION
190+
)
180191

181-
const viewEndedSubscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
182-
click.stop(endClocks.timeStamp)
183-
})
192+
const viewEndedSubscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
193+
click.stop(endClocks.timeStamp)
194+
})
184195

185-
const stopSubscription = stopObservable.subscribe(() => {
186-
click.stop()
187-
})
196+
const stopSubscription = stopObservable.subscribe(() => {
197+
click.stop()
198+
})
188199

189-
click.stopObservable.subscribe(() => {
190-
viewEndedSubscription.unsubscribe()
191-
stopWaitPageActivityEnd()
192-
stopSubscription.unsubscribe()
193-
})
194-
},
195-
}
200+
click.stopObservable.subscribe(() => {
201+
viewEndedSubscription.unsubscribe()
202+
stopWaitPageActivityEnd()
203+
stopSubscription.unsubscribe()
204+
})
196205
}
197206

198-
function computeClickActionBase(event: MouseEventOnElement, actionNameAttribute?: string) {
207+
type ClickActionBase = Pick<ClickAction, 'type' | 'name' | 'target' | 'position'>
208+
209+
function computeClickActionBase(event: MouseEventOnElement, actionNameAttribute?: string): ClickActionBase {
199210
let target: ClickAction['target']
200211
let position: ClickAction['position']
201212

@@ -216,7 +227,7 @@ function computeClickActionBase(event: MouseEventOnElement, actionNameAttribute?
216227
}
217228

218229
return {
219-
type: 'click',
230+
type: ActionType.CLICK,
220231
target,
221232
position,
222233
name: getActionNameFromElement(event.target, actionNameAttribute),
@@ -237,8 +248,8 @@ export type Click = ReturnType<typeof newClick>
237248
function newClick(
238249
lifeCycle: LifeCycle,
239250
history: ClickActionIdHistory,
240-
getUserActivity: OnClickContext['getUserActivity'],
241-
clickActionBase: Pick<ClickAction, 'name' | 'target' | 'position'>,
251+
getUserActivity: GetUserActivity,
252+
clickActionBase: ClickActionBase,
242253
clickEvent: MouseEventOnElement
243254
) {
244255
const id = generateUUID()

0 commit comments

Comments
 (0)