Skip to content

Commit

Permalink
🐛 fix frustration animation in session replay
Browse files Browse the repository at this point in the history
Frustration records used to reference MouseInteraction/Click records.
Since Click Actions are now generated based on mouseup events, we broke
this reference. There is no MouseInteraction/PointerUp record that
Frustration can reference.

This commit is a quick fix. It changes how we are generating
MouseInteraction/MouseUp records: instead of listening to mouseup DOM
events, we now listen to pointerup events, allowing Frustration records
to reference MouseInteraction/MouseUp records.

This works on the player side without any modification.

In the context of supporting Mobile Session Replay, we introduced
`PointerInteraction` records used by the Mobile SDKs in place of
`MouseInteraction`. In the future, it would be great to replace
`MouseInteraction` by `PointerInteraction` in the Browser SDK so we have
an uniform way to convey such interaction. This would cleanly solve the
issue since we would have `PointerInteraction/Up` records that we could
reference from `Frustration` records.
  • Loading branch information
BenoitZugmeyer committed Feb 6, 2023
1 parent a4d336c commit 92794a8
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
11 changes: 10 additions & 1 deletion packages/rum/src/domain/record/observers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,16 @@ export function initMoveObserver(cb: MousemoveCallBack): ListenerHandler {
}

const eventTypeToMouseInteraction = {
[DOM_EVENT.MOUSE_UP]: MouseInteractionType.MouseUp,
// Listen for pointerup DOM events instead of mouseup for MouseInteraction/MouseUp records. This
// allows to reference such records from Frustration records.
//
// In the context of supporting Mobile Session Replay, we introduced `PointerInteraction` records
// used by the Mobile SDKs in place of `MouseInteraction`. In the future, we should replace
// `MouseInteraction` by `PointerInteraction` in the Browser SDK so we have an uniform way to
// convey such interaction. This would cleanly solve the issue since we would have
// `PointerInteraction/Up` records that we could reference from `Frustration` records.
[DOM_EVENT.POINTER_UP]: MouseInteractionType.MouseUp,

[DOM_EVENT.MOUSE_DOWN]: MouseInteractionType.MouseDown,
[DOM_EVENT.CLICK]: MouseInteractionType.Click,
[DOM_EVENT.CONTEXT_MENU]: MouseInteractionType.ContextMenu,
Expand Down
14 changes: 7 additions & 7 deletions test/e2e/scenario/recorder/recorder.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,15 +730,15 @@ describe('recorder', () => {
expect(serverEvents.sessionReplay.length).toBe(1)
const { segment } = serverEvents.sessionReplay[0]

const clickRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.Click)
const mouseupRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.MouseUp)
const frustrationRecords = findAllFrustrationRecords(segment.data)

expect(clickRecords.length).toBe(1)
expect(clickRecords[0].id).toBeTruthy('mouse interaction record should have an id')
expect(mouseupRecords.length).toBe(1)
expect(mouseupRecords[0].id).toBeTruthy('mouse interaction record should have an id')
expect(frustrationRecords.length).toBe(1)
expect(frustrationRecords[0].data).toEqual({
frustrationTypes: [FrustrationType.DEAD_CLICK],
recordIds: [clickRecords[0].id!],
recordIds: [mouseupRecords[0].id!],
})
})

Expand All @@ -763,14 +763,14 @@ describe('recorder', () => {
expect(serverEvents.sessionReplay.length).toBe(1)
const { segment } = serverEvents.sessionReplay[0]

const clickRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.Click)
const mouseupRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.MouseUp)
const frustrationRecords = findAllFrustrationRecords(segment.data)

expect(clickRecords.length).toBe(4)
expect(mouseupRecords.length).toBe(4)
expect(frustrationRecords.length).toBe(1)
expect(frustrationRecords[0].data).toEqual({
frustrationTypes: [FrustrationType.RAGE_CLICK],
recordIds: clickRecords.map((r) => r.id!),
recordIds: mouseupRecords.map((r) => r.id!),
})
})
})
Expand Down

0 comments on commit 92794a8

Please sign in to comment.