fix(cdk/drag-drop): not stopping drag if page is blurred #17848
Conversation
crisbeto
added
P3
googlebot
added
the
cla: yes
There was a problem hiding this comment.
Would it also make sense to use the Page Visibility API and abort dragging on that as well? (maybe a follow up)
| // If the page is blurred while dragging (e.g. there was an `alert` or the browser window was | ||
| // minimized) we won't get a mouseup/touchend so we need to use a different event to stop the | ||
| // drag sequence. Use the last known location to figure out where the element should be dropped. | ||
| this._blurSubscription = this._dragDropRegistry.pageBlurred.subscribe(() => { |
There was a problem hiding this comment.
I would have thought that this would be treated as cancelling the drag sequence instead of completing it, since the dragged item might be somewhere the user doesn't want to drop.
There was a problem hiding this comment.
We could do that, but it won't be very smooth, because the list could've been shuffled already and we'd have to snap it back into place. Also it would be inconsistent, because we'd have to avoid emitting the dropped event, even though we've emitted the start, move etc. events.
There was a problem hiding this comment.
Do users currently have a way to cancel a drag? E.g., is I want to let people press escape to abort?
There was a problem hiding this comment.
They don't. I've had it as a low-priority task on my list to add an API for it, but I haven't had the time for it.
There was a problem hiding this comment.
I think I would want to collect more data on this before committing to the behavior (drop vs. abort)- maybe trying it out with a few people and seeing what they expect to happen
There was a problem hiding this comment.
Do we have people that are willing to try it out? This is a bit of an edge case to begin with so I don't know how much time we want to spend on it.
|
@jelbourn as I understand it, the page visibility API only emits when the user switches to a different tab or they minimize the window. I think the |
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
b4dacbe to
c0a1b15
Compare
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
c0a1b15 to
cc54183
Compare
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
cc54183 to
1437e12
Compare
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
1437e12 to
a7a25cd
Compare
|
@jelbourn I think we kind of forgot about this PR. I don't think it's a big enough issue that we'd have to poll the community about it so can we keep it moving? |
|
I still think that the page blurring should be treated as an abort rather than a drop |
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
a7a25cd to
698107f
Compare
crisbeto
changed the title
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
698107f to
cae87f9
Compare
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
cae87f9 to
5cd1b28
Compare
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
5cd1b28 to
3d53e08
Compare
Currently the only way to stop a drag sequence is via a `mouseup`/`touchend` event or by destroying the instance, however if the page loses focus while dragging the events won't be dispatched anymore and user will have to click again to stop dragging. These changes add some extra code that listens for `blur` events on the `window` and stops dragging. Fixes angular#17537.
crisbeto
force-pushed
the
17537/drag-drop-window-blur
branch
from
3d53e08 to
cd3b833
Compare
andrewseguin
removed
the
cla: yes
andrewseguin
removed
the
P3
Currently the only way to stop a drag sequence is via a
mouseup/touchendevent or by destroying the instance, however if the page loses focus while dragging the events won't be dispatched anymore and user will have to click again to stop dragging. These changes add some extra code that listens forblurevents on thewindowand stops dragging.Fixes #17537.