Skip to content

♻️ [RUMF-1368] use the PointerDown event target for click actions#1731

Merged
BenoitZugmeyer merged 15 commits into
mainfrom
benoit/heatmap--get-pointerdown-target
Sep 15, 2022
Merged

♻️ [RUMF-1368] use the PointerDown event target for click actions#1731
BenoitZugmeyer merged 15 commits into
mainfrom
benoit/heatmap--get-pointerdown-target

Conversation

@BenoitZugmeyer

@BenoitZugmeyer BenoitZugmeyer commented Sep 13, 2022

Copy link
Copy Markdown
Member

Motivation

Click target inaccuracy

When the click starts (mouse/pointer down) on an element and ends (mouse/pointer up) on another element, the click event target is set to the BODY element by the browser. This is problematic, because we report clicks occuring on the BODY even if the user uses a whole other element. This impacts the RUM click action target name (@action.target.name), as well as the experimental heatmaps CSS selector generation.

Using the pointerdown event target instead improves the situation: the element used to compute the action target name and CSS selector is the element the user intended to click.

Heatmap CSS selector improvement

Some application add a temporary class name when an element is being clicked (similarly to the :active CSS pseudo-class) or focused. Those temporary class names blocks us from finding again the element when rendering the Heatmap again.

Generating the CSS selector before the application UI reacts to it should improve the situation a bit (pointerdown is the first event triggered when a click occurs).

Frustration signal follow-up

This change is also a first step in fixing one of the most common dead click false positive cases: currently, we are only watching for page activity during or following a click event. But in some cases the application updates the UI earlier (ex: on mousedown, focus...). Those kind of clicks we are incorrectly considered a click as "dead".

In a future follow-up, we'll fix this by watching page activity during or following the pointerdown event as well, to avoid marking those clicks as "dead".

Changes

Beside a bit of refactoring, the main change was to update the listenActionEvents to call a callback on pointerdown. This callback can return a onClick closure that will be called during the next click event. This allows to build the target-related properties earlier and still build the Click during the click event.

Testing

  • Local
  • Staging
  • Unit
  • End to end

Notes

This PR does not fix the issue in the recorder part: clicks event will still be reported on the body element when the element target changes between mousedown and mouseup. This should not matter too much, as the click position is also collected.

To keep backward compatibility, the event send as context in the beforeSend callback is still the click event.


I have gone over the contributing documentation.

@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/heatmap--get-pointerdown-target branch from f9048b9 to 5d3342e Compare September 13, 2022 10:26
@BenoitZugmeyer
BenoitZugmeyer marked this pull request as ready for review September 13, 2022 12:26
@BenoitZugmeyer
BenoitZugmeyer requested a review from a team as a code owner September 13, 2022 12:26
Comment thread packages/core/src/tools/contextHistory.spec.ts
Comment thread packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts Outdated
Comment thread test/e2e/scenario/rum/actions.scenario.ts Outdated
Comment thread packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts Outdated
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #1731 (5f4e7dc) into main (817ce30) will increase coverage by 0.00%.
The diff coverage is 98.48%.

@@           Coverage Diff           @@
##             main    #1731   +/-   ##
=======================================
  Coverage   90.97%   90.98%           
=======================================
  Files         128      128           
  Lines        4944     4958   +14     
  Branches     1101     1103    +2     
=======================================
+ Hits         4498     4511   +13     
- Misses        446      447    +1     
Impacted Files Coverage Δ
packages/rum-core/test/createFakeClick.ts 87.50% <ø> (ø)
...in/rumEventsCollection/action/trackClickActions.ts 98.19% <97.77%> (+0.13%) ⬆️
packages/core/src/tools/timeUtils.ts 100.00% <100.00%> (ø)
...rum-core/src/domain/contexts/foregroundContexts.ts 90.76% <100.00%> (ø)
...n/rumEventsCollection/action/listenActionEvents.ts 100.00% <100.00%> (ø)
...rumEventsCollection/resource/matchRequestTiming.ts 95.45% <100.00%> (ø)
packages/core/src/transport/batch.ts 93.02% <0.00%> (-1.17%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bcaudan bcaudan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts Outdated
@BenoitZugmeyer
BenoitZugmeyer merged commit d775438 into main Sep 15, 2022
@BenoitZugmeyer
BenoitZugmeyer deleted the benoit/heatmap--get-pointerdown-target branch September 15, 2022 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants