Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RUMF-1297] frustration signals: track input changes #1603

Merged
merged 8 commits into from
Jun 28, 2022
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(() => {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
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