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

Record pointerType on clicks #1129

Merged
merged 6 commits into from
Apr 12, 2023
Merged

Conversation

eoghanmurray
Copy link
Contributor

  • could be useful for displaying e.g. a circle rather than a point during replay

touchscreen laptops can have both mouse and touch within the same session

@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2023

🦋 Changeset detected

Latest commit: be85644

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
rrweb Minor
@rrweb/types Minor
rrweb-snapshot Minor
rrdom Minor
rrdom-nodejs Minor
rrweb-player Minor
@rrweb/web-extension Minor
rrvideo Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Juice10
Copy link
Contributor

Juice10 commented Mar 3, 2023

You have some typescript typing that needs changing, and then a bunch of tests will need to be updated. But this is a welcome addition to rrweb IMO!

…circle rather than a point during replay

 - We have to switch to 'onpointerdown' & 'onpointerup' in order to actually capture `e.pointerType`
 - this replaces 4 event listeners (MouseDown/MouseUp/TouchStart/TouchEnd) with 2 pointer ones which should fire in all 4 scenarios. We still output the old types according to the MouseInteractions enum
 - there is no Pointer equivalent of Click, so we leave that is, but use the last Pointer event to attach a pointerType to (only) the click event, where it is most useful
 - we can fallback to the old method for any browsers not supporting `window.PointerEvent`, in which case \`pointerType\` will be absent from all events
@eoghanmurray
Copy link
Contributor Author

@Juice10 I'm deploying the changes to a 'canary' test environment so feel free to merge in a few weeks if I haven't done so by then (will be away over Easter hols)

@eoghanmurray
Copy link
Contributor Author

@Juice10 any idea why "pointerType": "mouse" doesn't show up in regards to the can record clicks test?

  • I know the x/y values are stripped out by test/utils.ts
  • in the live-stream view, the pointerType shows up — is livestream using the same puppeteer set up as testing?

@eoghanmurray
Copy link
Contributor Author

Another observation:
Firefox 111.0.1 (64-bit) on a Ubuntu Dell M3800 laptop - with touchscreen.

A touch event is showing up as 'mouse' with this new code.
But the prior version was also incorrectly recording mouseDown and mouseUp events instead of touchStart/touchEnd events.

On the same laptop, Chromium works as expected and records these events as 'touch'.

@eoghanmurray
Copy link
Contributor Author

@Juice10 any idea why "pointerType": "mouse" doesn't show up in regards to the can record clicks test?

* I know the x/y values are stripped out by test/utils.ts

* in the live-stream view, the pointerType shows up — is livestream using the same puppeteer set up as testing?

I guess it could be because our version of Puppeteer doesn't support PointerType?

@Juice10
Copy link
Contributor

Juice10 commented Apr 5, 2023

@eoghanmurray the setup is mostly the same. Big difference is that click events in puppeteer aren't 100% the same as "real" click events.

From the puppeteer docs:

Note: The mouse events trigger synthetic MouseEvents. This means that it does not fully replicate the functionality of what a normal user would be able to do with their mouse.

For example, dragging and selecting text is not possible using page.mouse. Instead, you can use the DocumentOrShadowRoot.getSelection() functionality implemented in the platform.

https://pptr.dev/api/puppeteer.mouse

@Juice10
Copy link
Contributor

Juice10 commented Apr 7, 2023

@eoghanmurray would you like to merge this one or wait till you get the tests dialed in?

@eoghanmurray eoghanmurray merged commit 41cc822 into rrweb-io:master Apr 12, 2023
@eoghanmurray
Copy link
Contributor Author

I've gone ahead and merged; if we change up from Puppeteer in future it should be discoverable what the correct pointerType should be for the results of tests.

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.

2 participants