Skip to content

Commit

Permalink
🐛 [RUMF-1295] do not consider clicks related to "input" as dead
Browse files Browse the repository at this point in the history
  • Loading branch information
BenoitZugmeyer committed Jun 23, 2022
1 parent 08b19dc commit ed16a61
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Clock } from '../../../../../core/test/specHelper'
import { mockClock } from '../../../../../core/test/specHelper'
import type { FakeClick } from '../../../../test/createFakeClick'
import { createFakeClick } from '../../../../test/createFakeClick'
import { computeFrustration, isRage } from './computeFrustration'
import { computeFrustration, isRage, isDead } from './computeFrustration'

describe('computeFrustration', () => {
let clicks: FakeClick[]
Expand Down Expand Up @@ -34,6 +34,12 @@ describe('computeFrustration', () => {
expect(getFrustrations(rageClick)).toEqual([FrustrationType.RAGE_CLICK, FrustrationType.DEAD_CLICK])
})

it('do not add a dead frustration to the rage click if clicks are associated with an "input" event', () => {
clicksConsideredAsRage[1] = createFakeClick({ hasPageActivity: false, userActivity: { input: true } })
computeFrustration(clicksConsideredAsRage, rageClick)
expect(getFrustrations(rageClick)).toEqual([FrustrationType.RAGE_CLICK])
})

it('adds an error frustration to the rage click if an error occurs during the rage click lifetime', () => {
rageClick = createFakeClick({ hasError: true })
computeFrustration(clicksConsideredAsRage, rageClick)
Expand Down Expand Up @@ -123,3 +129,17 @@ describe('isRage', () => {
expect(isRage(clicks)).toBe(true)
})
})

describe('isDead', () => {
it('considers as dead when the click has no page activity', () => {
expect(isDead(createFakeClick({ hasPageActivity: false }))).toBe(true)
})

it('does not consider as dead when the click has page activity', () => {
expect(isDead(createFakeClick({ hasPageActivity: true }))).toBe(false)
})

it('does not consider as dead when the click is related to an "input" event', () => {
expect(isDead(createFakeClick({ hasPageActivity: false, userActivity: { input: true } }))).toBe(false)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const MIN_CLICKS_PER_SECOND_TO_CONSIDER_RAGE = 3
export function computeFrustration(clicks: Click[], rageClick: Click) {
if (isRage(clicks)) {
rageClick.addFrustration(FrustrationType.RAGE_CLICK)
if (clicks.some((click) => !click.hasPageActivity)) {
if (clicks.some(isDead)) {
rageClick.addFrustration(FrustrationType.DEAD_CLICK)
}
if (rageClick.hasError) {
Expand All @@ -22,7 +22,7 @@ export function computeFrustration(clicks: Click[], rageClick: Click) {
click.addFrustration(FrustrationType.ERROR_CLICK)
}
if (
!click.hasPageActivity &&
isDead(click) &&
// Avoid considering clicks part of a double-click or triple-click selections as dead clicks
!hasSelectionChanged
) {
Expand All @@ -46,3 +46,7 @@ export function isRage(clicks: Click[]) {
}
return false
}

export function isDead(click: Click) {
return !click.hasPageActivity && !click.getUserActivity().input
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createNewEvent } from '../../../../../core/test/specHelper'
import type { Clock } from '../../../../../core/test/specHelper'
import { createNewEvent, mockClock } from '../../../../../core/test/specHelper'
import type { OnClickContext } from './listenActionEvents'
import { listenActionEvents } from './listenActionEvents'

Expand Down Expand Up @@ -113,6 +114,55 @@ describe('listenActionEvents', () => {
}
})

describe('input user activity', () => {
let clock: Clock

beforeEach(() => {
clock = mockClock()
})

afterEach(() => {
clock.cleanup()
})

it('click that do not trigger an input input event should not report input user activity', () => {
emulateClick()
expect(hasInputUserActivity()).toBe(false)
})

it('click that triggers an input event during the click should report an input user activity', () => {
emulateClick({
beforeMouseUp() {
emulateInputEvent()
},
})
expect(hasInputUserActivity()).toBe(true)
})

it('click that triggers an input event slightly after the click should report an input user activity', () => {
emulateClick()
emulateInputEvent()
clock.tick(1)
expect(hasInputUserActivity()).toBe(true)
})

it('click and type should report an input user activity', () => {
emulateClick({
beforeMouseUp() {
emulateInputEvent()
},
})
expect(hasInputUserActivity()).toBe(true)
})

function emulateInputEvent() {
window.dispatchEvent(createNewEvent('input'))
}
function hasInputUserActivity() {
return onClickSpy.calls.mostRecent().args[0].getUserActivity().input
}
})

function emulateClick({ beforeMouseUp }: { beforeMouseUp?(): void } = {}) {
document.body.dispatchEvent(createNewEvent('mousedown'))
beforeMouseUp?.()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import { addEventListener, DOM_EVENT } from '@datadog/browser-core'

export interface OnClickContext {
event: MouseEvent & { target: Element }
getUserActivity(): { selection: boolean }
getUserActivity(): { selection: boolean; input: boolean }
}

export function listenActionEvents({ onClick }: { onClick(context: OnClickContext): void }) {
let hasSelectionChanged = false
let selectionEmptyAtMouseDown: boolean
let hasInputChanged = false

const listeners = [
addEventListener(
Expand Down Expand Up @@ -37,7 +38,15 @@ export function listenActionEvents({ onClick }: { onClick(context: OnClickContex
(clickEvent: MouseEvent) => {
if (clickEvent.target instanceof Element) {
// Use a scoped variable to make sure the value is not changed by other clicks
const userActivity = { selection: hasSelectionChanged }
const userActivity = {
selection: hasSelectionChanged,
input: hasInputChanged,
}
if (!hasInputChanged) {
setTimeout(() => {
userActivity.input = hasInputChanged
})
}

onClick({
event: clickEvent as MouseEvent & { target: Element },
Expand All @@ -47,6 +56,15 @@ export function listenActionEvents({ onClick }: { onClick(context: OnClickContex
},
{ capture: true }
),

addEventListener(
window,
DOM_EVENT.INPUT,
() => {
hasInputChanged = true
},
{ capture: true }
),
]

return {
Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/test/createFakeClick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function createFakeClick({
hasPageActivity,
getUserActivity: () => ({
selection: false,
input: false,
...userActivity,
}),
addFrustration: jasmine.createSpy<Click['addFrustration']>(),
Expand Down

0 comments on commit ed16a61

Please sign in to comment.