Skip to content

Commit

Permalink
🐛 [RUMF-1295] treat "input" events as "selectionchange"
Browse files Browse the repository at this point in the history
  • Loading branch information
BenoitZugmeyer committed Jun 21, 2022
1 parent 3ed09ba commit 3731db3
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export function computeFrustration(clicks: Click[], rageClick: Click) {
}
if (
!click.hasActivity &&
!click.hasInputChanged &&
// Avoid considering clicks part of a double-click or triple-click selections as dead clicks
!hasSelectionChanged
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('listenActionEvents', () => {

it('click without selection impact should not report a selection change', () => {
emulateClick()
expect(onClickSpy.calls.mostRecent().args[1]).toBe(false)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(false)
})

it('click and drag to select text should reports a selection change', () => {
Expand All @@ -44,7 +44,7 @@ describe('listenActionEvents', () => {
emulateNodeSelection(0, 3)
},
})
expect(onClickSpy.calls.mostRecent().args[1]).toBe(true)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(true)
})

it('click and drag that adds a selection range should reports a selection change', () => {
Expand All @@ -55,7 +55,7 @@ describe('listenActionEvents', () => {
emulateNodeSelection(3, 6, { clearSelection: false })
},
})
expect(onClickSpy.calls.mostRecent().args[1]).toBe(true)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(true)
})

it('click to deselect previously selected text should report a selection change', () => {
Expand All @@ -65,7 +65,7 @@ describe('listenActionEvents', () => {
emulateNodeSelection(0, 0)
},
})
expect(onClickSpy.calls.mostRecent().args[1]).toBe(true)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(true)
})

// eslint-disable-next-line max-len
Expand All @@ -76,7 +76,7 @@ describe('listenActionEvents', () => {
emulateNodeSelection(0, 7)
},
})
expect(onClickSpy.calls.mostRecent().args[1]).toBe(true)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(true)
})

it('click that change the caret (collapsed selection) position should not report selection change', () => {
Expand All @@ -86,7 +86,7 @@ describe('listenActionEvents', () => {
emulateNodeSelection(1, 1)
},
})
expect(onClickSpy.calls.mostRecent().args[1]).toBe(false)
expect(onClickSpy.calls.mostRecent().args[1].hasSelectionChanged).toBe(false)
})

function emulateNodeSelection(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,48 +1,62 @@
import { addEventListener, DOM_EVENT } from '@datadog/browser-core'

export type OnClickCallback = (clickEvent: MouseEvent & { target: Element }, hasSelectionChanged: boolean) => void
export type OnClickCallback = (
clickEvent: MouseEvent & { target: Element },
changes: { hasSelectionChanged: boolean; hasInputChanged: boolean }
) => void

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

const { stop: stopMouseDownListener } = addEventListener(
window,
DOM_EVENT.MOUSE_DOWN,
() => {
hasSelectionChanged = false
selectionEmptyAtMouseDown = isSelectionEmpty()
},
{ capture: true }
)

const { stop: stopSelectionChangeListener } = addEventListener(
window,
DOM_EVENT.SELECTION_CHANGE,
() => {
if (!selectionEmptyAtMouseDown || !isSelectionEmpty()) {
hasSelectionChanged = true
}
},
{ capture: true }
)

const { stop: stopClickListener } = addEventListener(
window,
DOM_EVENT.CLICK,
(clickEvent: MouseEvent) => {
if (clickEvent.target instanceof Element) {
onClick(clickEvent as MouseEvent & { target: Element }, hasSelectionChanged)
}
},
{ capture: true }
)
const listeners = [
addEventListener(
window,
DOM_EVENT.MOUSE_DOWN,
() => {
hasSelectionChanged = false
hasInputChanged = false
selectionEmptyAtMouseDown = isSelectionEmpty()
},
{ capture: true }
),

addEventListener(
window,
DOM_EVENT.SELECTION_CHANGE,
() => {
if (!selectionEmptyAtMouseDown || !isSelectionEmpty()) {
hasSelectionChanged = true
}
},
{ capture: true }
),

addEventListener(
window,
DOM_EVENT.CLICK,
(clickEvent: MouseEvent) => {
if (clickEvent.target instanceof Element) {
onClick(clickEvent as MouseEvent & { target: Element }, { hasSelectionChanged, hasInputChanged })
}
},
{ capture: true }
),

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

return {
stop: () => {
stopMouseDownListener()
stopSelectionChangeListener()
stopClickListener()
listeners.forEach((listener) => listener.stop())
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ export function trackClickActions(
}
}

function processClick(event: MouseEvent & { target: Element }, hasSelectionChanged: boolean) {
function processClick(
event: MouseEvent & { target: Element },
changes: { hasSelectionChanged: boolean; hasInputChanged: boolean }
) {
if (!trackFrustrations && history.find()) {
// TODO: remove this in a future major version. To keep retrocompatibility, ignore any new
// action if another one is already occurring.
Expand All @@ -114,7 +117,7 @@ export function trackClickActions(

const startClocks = clocksNow()

const click = newClick(lifeCycle, history, hasSelectionChanged, {
const click = newClick(lifeCycle, history, changes, {
name,
event,
startClocks,
Expand Down Expand Up @@ -184,7 +187,7 @@ export type Click = ReturnType<typeof newClick>
function newClick(
lifeCycle: LifeCycle,
history: ClickActionIdHistory,
hasSelectionChanged: boolean,
changes: { hasSelectionChanged: boolean; hasInputChanged: boolean },
base: Pick<ClickAction, 'startClocks' | 'event' | 'name'>
) {
const id = generateUUID()
Expand Down Expand Up @@ -237,14 +240,19 @@ function newClick(
get hasActivity() {
return activityEndTime !== undefined
},
hasSelectionChanged,
get hasSelectionChanged() {
return changes.hasSelectionChanged
},
get hasInputChanged() {
return changes.hasInputChanged
},
addFrustration: (frustrationType: FrustrationType) => {
frustrationTypes.push(frustrationType)
},

isStopped: () => status === ClickStatus.STOPPED || status === ClickStatus.FINALIZED,

clone: () => newClick(lifeCycle, history, hasSelectionChanged, base),
clone: () => newClick(lifeCycle, history, changes, base),

validate: () => {
stop()
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 @@ -25,6 +25,7 @@ export function createFakeClick(partialClick?: {
validate: jasmine.createSpy(),
hasError: false,
hasActivity: true,
hasInputChanged: false,
hasSelectionChanged: false,
addFrustration: jasmine.createSpy(),
clone: jasmine.createSpy<typeof clone>().and.callFake(clone),
Expand Down

0 comments on commit 3731db3

Please sign in to comment.