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

fix(drag-drop): not stopping drag if page is blurred #17848

Open
wants to merge 1 commit into
base: master
from

Conversation

@crisbeto
Copy link
Member

crisbeto commented Dec 1, 2019

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 #17537.

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 #17537.
Copy link
Member

jelbourn left a 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(() => {

This comment has been minimized.

Copy link
@jelbourn

jelbourn Dec 3, 2019

Member

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.

This comment has been minimized.

Copy link
@crisbeto

crisbeto Dec 4, 2019

Author Member

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.

This comment has been minimized.

Copy link
@jelbourn

jelbourn Dec 5, 2019

Member

Do users currently have a way to cancel a drag? E.g., is I want to let people press escape to abort?

This comment has been minimized.

Copy link
@crisbeto

crisbeto Dec 5, 2019

Author Member

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.

This comment has been minimized.

Copy link
@jelbourn

jelbourn Dec 5, 2019

Member

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

This comment has been minimized.

Copy link
@crisbeto

crisbeto Dec 5, 2019

Author Member

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.

@crisbeto

This comment has been minimized.

Copy link
Member Author

crisbeto commented Dec 4, 2019

@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 blur approach here is better, because it handles the same as the API, as well as when the user gets an alert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.