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

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Jun 21, 2022

Motivation

Do not consider clicks that trigger an input event as dead.

Changes

  • Do some light refactoring
  • Track input events occurring during and after every click to adjust frustration signals assigned to them

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/frustration-signals--track-input-change branch from ee96a62 to 3731db3 Compare June 21, 2022 16:32
@BenoitZugmeyer BenoitZugmeyer changed the base branch from main to benoit/frustration-signals--track-selection-change-2 June 21, 2022 16:33
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/frustration-signals--track-input-change branch 2 times, most recently from 987264e to 02a6df1 Compare June 21, 2022 16:49
@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2022

Codecov Report

Merging #1603 (c35d4aa) into main (3921772) will increase coverage by 0.02%.
The diff coverage is 97.43%.

@@            Coverage Diff             @@
##             main    #1603      +/-   ##
==========================================
+ Coverage   90.89%   90.92%   +0.02%     
==========================================
  Files         126      126              
  Lines        4603     4615      +12     
  Branches     1029     1033       +4     
==========================================
+ Hits         4184     4196      +12     
  Misses        419      419              
Impacted Files Coverage Δ
packages/rum-core/test/createFakeClick.ts 87.50% <87.50%> (+5.68%) ⬆️
...n/rumEventsCollection/action/computeFrustration.ts 100.00% <100.00%> (ø)
...n/rumEventsCollection/action/listenActionEvents.ts 100.00% <100.00%> (ø)
...in/rumEventsCollection/action/trackClickActions.ts 98.01% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Base automatically changed from benoit/frustration-signals--track-selection-change-2 to main June 22, 2022 15:20
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/frustration-signals--track-input-change branch 2 times, most recently from 133efa0 to d1e9ac4 Compare June 23, 2022 08:19
The `listenActionEvents` will track more user activities than just
selection changes. This commit does some groundwork to allow multiple
user activities. It takes the future "input" constraint into account by
using a function to get the user interaction a bit after the click
occurs.
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/frustration-signals--track-input-change branch from d1e9ac4 to ed16a61 Compare June 23, 2022 16:07
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/frustration-signals--track-input-change branch from ed16a61 to bc5024f Compare June 23, 2022 16:35
@BenoitZugmeyer BenoitZugmeyer marked this pull request as ready for review June 23, 2022 16:38
@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner June 23, 2022 16:38
Comment on lines +179 to +203
createTest('do not consider a click on a checkbox as "dead_click"')
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(html` <input type="checkbox" /> `)
.run(async ({ serverEvents }) => {
const input = await $('input')
await input.click()
await flushEvents()
const actionEvents = serverEvents.rumActions

expect(actionEvents.length).toBe(1)
expect(actionEvents[0].action.frustration!.type).toEqual([])
})

createTest('do not consider a click to change the value of a "range" input as "dead_click"')
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(html` <input type="range" /> `)
.run(async ({ serverEvents }) => {
const input = await $('input')
await input.click({ x: 10 })
await flushEvents()
const actionEvents = serverEvents.rumActions

expect(actionEvents.length).toBe(1)
expect(actionEvents[0].action.frustration!.type).toEqual([])
})
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 praise: ‏nice!

@BenoitZugmeyer BenoitZugmeyer merged commit 8c21fcb into main Jun 28, 2022
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/frustration-signals--track-input-change branch June 28, 2022 09:07
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