-
Notifications
You must be signed in to change notification settings - Fork 27
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
base: main
Are you sure you want to change the base?
Conversation
…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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Feel free to ping when this is ready for review again |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
<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, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
* 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. |
There was a problem hiding this comment.
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.
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