Skip to content

Commit

Permalink
🐛⚗ [RUMF-1293] discard dead clicks when activity occurs on pointerdown (
Browse files Browse the repository at this point in the history
  • Loading branch information
BenoitZugmeyer authored Jan 30, 2023
1 parent 4ec9630 commit bcd2cd7
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,26 @@ describe('trackClickActions', () => {
resetExperimentalFeatures()
})

it('does not consider a click with activity happening on pointerdown as a dead click', () => {
const { clock } = setupBuilder.build()

emulateClick({ activity: { on: 'pointerdown' } })

clock.tick(EXPIRE_DELAY)
expect(events.length).toBe(1)
expect(events[0].frustrationTypes).toEqual([])
})

it('activity happening on pointerdown is not taken into account for the action duration', () => {
const { clock } = setupBuilder.build()

emulateClick({ activity: { on: 'pointerdown' } })

clock.tick(EXPIRE_DELAY)
expect(events.length).toBe(1)
expect(events[0].duration).toBe(0 as Duration)
})

it('does not consider a click with activity happening on pointerup as a dead click', () => {
const { clock } = setupBuilder.build()

Expand Down Expand Up @@ -471,7 +491,7 @@ describe('trackClickActions', () => {
target?: HTMLElement
activity?: {
delay?: number
on?: 'pointerup' | 'click'
on?: 'pointerup' | 'click' | 'pointerdown'
}
} = {}) {
const targetPosition = target.getBoundingClientRect()
Expand All @@ -487,13 +507,14 @@ describe('trackClickActions', () => {
isPrimary: true,
}
target.dispatchEvent(createNewEvent('pointerdown', eventProperties))
emulateActivityIfNeeded('pointerdown')
setupBuilder.clock!.tick(EMULATED_CLICK_DURATION)
target.dispatchEvent(createNewEvent('pointerup', eventProperties))
emulateActivityIfNeeded('pointerup')
target.dispatchEvent(createNewEvent('click', eventProperties))
emulateActivityIfNeeded('click')

function emulateActivityIfNeeded(event: 'pointerup' | 'click') {
function emulateActivityIfNeeded(event: 'pointerdown' | 'pointerup' | 'click') {
if (activity && (activity.on ?? 'click') === event) {
const delay = activity.delay ?? BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY
if (delay < 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { RumConfiguration } from '../../configuration'
import type { LifeCycle } from '../../lifeCycle'
import { LifeCycleEventType } from '../../lifeCycle'
import { trackEventCounts } from '../../trackEventCounts'
import { waitPageActivityEnd } from '../../waitPageActivityEnd'
import { PAGE_ACTIVITY_VALIDATION_DELAY, waitPageActivityEnd } from '../../waitPageActivityEnd'
import type { ClickChain } from './clickChain'
import { createClickChain } from './clickChain'
import { getActionNameFromElement } from './getActionNameFromElement'
Expand Down Expand Up @@ -79,9 +79,18 @@ export function trackClickActions(

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, stopClickChain)

const { stop: stopActionEventsListener } = listenActionEvents<ClickActionBase>({
onPointerDown: (pointerDownEvent) => processPointerDown(configuration, history, pointerDownEvent),
onStartEvent: (clickActionBase, startEvent, getUserActivity, getClickEventTimeStamp) =>
const { stop: stopActionEventsListener } = listenActionEvents<{
clickActionBase: ClickActionBase
hadActivityOnPointerDown: () => boolean
}>({
onPointerDown: (pointerDownEvent) =>
processPointerDown(configuration, lifeCycle, domMutationObservable, history, pointerDownEvent),
onStartEvent: (
{ clickActionBase, hadActivityOnPointerDown },
startEvent,
getUserActivity,
getClickEventTimeStamp
) =>
startClickAction(
configuration,
lifeCycle,
Expand All @@ -92,6 +101,7 @@ export function trackClickActions(
clickActionBase,
startEvent,
getUserActivity,
hadActivityOnPointerDown,
getClickEventTimeStamp
),
})
Expand Down Expand Up @@ -128,6 +138,8 @@ export function trackClickActions(

function processPointerDown(
configuration: RumConfiguration,
lifeCycle: LifeCycle,
domMutationObservable: Observable<void>,
history: ClickActionIdHistory,
pointerDownEvent: MouseEventOnElement
) {
Expand All @@ -144,7 +156,23 @@ function processPointerDown(
return
}

return clickActionBase
let hadActivityOnPointerDown = false

if (isExperimentalFeatureEnabled('dead_click_fixes')) {
waitPageActivityEnd(
lifeCycle,
domMutationObservable,
configuration,
(pageActivityEndEvent) => {
hadActivityOnPointerDown = pageActivityEndEvent.hadActivity
},
// We don't care about the activity duration, we just want to know whether an activity did happen
// within the "validation delay" or not. Limit the duration so the callback is called sooner.
PAGE_ACTIVITY_VALIDATION_DELAY
)
}

return { clickActionBase, hadActivityOnPointerDown: () => hadActivityOnPointerDown }
}

function startClickAction(
Expand All @@ -157,6 +185,7 @@ function startClickAction(
clickActionBase: ClickActionBase,
startEvent: MouseEventOnElement,
getUserActivity: () => UserActivity,
hadActivityOnPointerDown: () => boolean,
getClickEventTimeStamp: () => TimeStamp | undefined
) {
const click = newClick(lifeCycle, history, getUserActivity, getClickEventTimeStamp, clickActionBase, startEvent)
Expand All @@ -174,7 +203,17 @@ function startClickAction(
// If the clock is looking weird, just discard the click
click.discard()
} else {
click.stop(pageActivityEndEvent.hadActivity ? pageActivityEndEvent.end : undefined)
if (pageActivityEndEvent.hadActivity) {
click.stop(pageActivityEndEvent.end)
} else if (hadActivityOnPointerDown()) {
click.stop(
// using the click start as activity end, so the click will have some activity but its
// duration will be 0 (as the activity started before the click start)
click.startClocks.timeStamp
)
} else {
click.stop()
}

// Validate or discard the click only if we don't track frustrations. It'll be done when
// the click chain is finalized.
Expand Down

0 comments on commit bcd2cd7

Please sign in to comment.