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

Set mousemove and pointermove events during dragging and resizing to be excluding inputs #103

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

LanWei22
Copy link

@LanWei22 LanWei22 commented May 1, 2021

Right now, the mousemove and pointermove events are not excluding inputs, but we want to set the mousemove and pointerdown events to be excluding inputs when they trigger dragging element around or resizing elements but not scrolling. These mousemove or pointermove events fired after mousedown or pointerdown event, but without pointercancel event will make dragging or resizing happen.


Preview | Diff

…e excluding inputs

Right now, the mousemove and pointermove events are not excluding inputs, but we want to set the mousemove and pointerdown events to be excluding inputs when they trigger dragging element around or resizing elements but not scrolling. These mousemove or pointermove events fired after mousedown or pointerdown event, but without pointercancel event will make dragging or resizing happen.
index.bs Outdated
<a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a> events after
the <a href="https://www.w3.org/TR/pointerevents/#the-pointerdown-event">pointerdown</a> event,
which trigger dragging elements around or resizing elements but not scrolling, should be excluding
inputs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this also mention that anything with pointercancel would not be an excluding input? Just move after down is not sufficient, as that would include scrolls and fling/flick.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is clearer to add that pointercancel means scrolling happen, and it is not an excluding input. Thank you.

@LanWei22 LanWei22 requested a review from npm1 May 3, 2021 19:50
Copy link
Author

@LanWei22 LanWei22 left a comment

Choose a reason for hiding this comment

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

Nicolas, please take a look to see if it is clear right now. Thank you.

index.bs Outdated
<a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a> events after
the <a href="https://www.w3.org/TR/pointerevents/#the-pointerdown-event">pointerdown</a> event,
which trigger dragging elements around or resizing elements but not scrolling, should be excluding
inputs.
Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is clearer to add that pointercancel means scrolling happen, and it is not an excluding input. Thank you.

index.bs Outdated
The <a href="https://www.w3.org/TR/uievents/#event-type-mousemove">mousemove</a> and
<a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a>
events are also not excluding inputs.
The <a href="https://www.w3.org/TR/uievents/#event-type-mousemove">mousemove</a> events after the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I think what we want is: pointermove after pointerdown that is not followed by pointercancel, right? I.e. a way the current wording could be interpreted is that the following sequence would consist of an excluding pointermove:
pointerdown, pointermove, pointercancel...
And I think we want that pointermove to not count as an excluding input, right? So in a sense we kinda need to wait until we know that pointercancel won't be fired in order to know that pointermove was excluding. Does this match the implementation, and does it make sense to you?

Copy link
Author

Choose a reason for hiding this comment

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

I have made some changes, now pointermove events are not excluding inputs.

@LanWei22 LanWei22 requested a review from npm1 June 2, 2021 21:30
index.bs Outdated Show resolved Hide resolved
@LanWei22 LanWei22 requested a review from npm1 June 9, 2021 23:14
@npm1
Copy link
Collaborator

npm1 commented Jun 16, 2021

Feel free to ping when this is ready for review again

Copy link
Collaborator

@npm1 npm1 left a comment

Choose a reason for hiding this comment

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

I think this section is much more complex than before now and we want to be super clear about the algorithm that the user agent needs to perform in order to temporarily buffer the layout shifts after down events, as well as when to dispatch these buffered shifts and how to set the had_recent_input

index.bs Outdated
until such time as it is known that the event does not begin a fling or scroll
gesture.
The user agent buffers the layout shifts of
<a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a> events after a
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is "the layout shifts of pointmove events"? This is not super clear to me (layout shifts don't belong to inputs).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also thought we agree that we do not need to care about move events since the input exclusions can be determined solely from the downs, ups, and cancels? Did this change in the implementation?

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean we do not need to specify whether pointermove events or mousemove events are excluding inputs or not.

index.bs Outdated Show resolved Hide resolved
@LanWei22 LanWei22 requested a review from npm1 July 15, 2021 04:28
index.bs Outdated
<a href="https://www.w3.org/TR/uievents/#event-type-mousedown">mousedown</a> event until a
<a href="https://www.w3.org/TR/pointerevents/#the-pointerup-event">pointerup</a> event or a
<a href="https://www.w3.org/TR/uievents/#event-type-mouseup">mouseup</a> event are received, which turn into touch and mouse drag actions.
These <a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a> events,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still referring to move events here. Do we need to?

Copy link
Author

Choose a reason for hiding this comment

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

I deleted all the pointermove and mousemove events.

index.bs Outdated Show resolved Hide resolved
@LanWei22 LanWei22 requested a review from npm1 July 16, 2021 05:34
Copy link
Collaborator

@npm1 npm1 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for working on this

index.bs Outdated
<a href="https://www.w3.org/TR/pointerevents/#the-pointermove-event">pointermove</a>
events are also not excluding inputs.
* When a <a href="https://www.w3.org/TR/pointerevents/#the-pointerdown-event">pointerdown</a> event or
a <a href="https://www.w3.org/TR/uievents/#event-type-mousedown">mousedown</a> event is received, the
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that markdown doesn't like if you don't indent this line within the bullet point

Copy link
Author

Choose a reason for hiding this comment

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

ACK.

index.bs Outdated Show resolved Hide resolved
@LanWei22 LanWei22 requested a review from npm1 July 16, 2021 22:32
* When a <a href="https://www.w3.org/TR/pointerevents/#the-pointercancel-event">pointercancel</a> event
is received, all the accumulated layout shift score in the buffer is reported, and it is not an excluding input.
* When a <a href="https://www.w3.org/TR/pointerevents/#the-pointerup-event">pointerup</a> event or a
<a href="https://www.w3.org/TR/uievents/#event-type-mouseup">mouseup</a> event is received, the buffer is reset.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: 'buffer is reset' is a bit unclear since we didn't say what the buffer is. But in any case we need to report the accumulated layout shifts before that. Do we need to also reset when we get pointercancel?

Also, we may want to specify how we compute attribution while buffering? I actually think I found a Chrome bug, filing it.

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.

None yet

2 participants