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

cdkDropList broken on touch devices on touchend #14390

Closed
atscott opened this issue Dec 5, 2018 · 7 comments · Fixed by #14392
Closed

cdkDropList broken on touch devices on touchend #14390

atscott opened this issue Dec 5, 2018 · 7 comments · Fixed by #14392
Assignees
Labels
G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround

Comments

@atscott
Copy link
Contributor

atscott commented Dec 5, 2018

What is the expected behavior?

cdkDropList does not error

What is the current behavior?

The _cleanupDragArtifacts method now calls _getPointerPositionOnPage which does not handle touchend events correctly:
const point = this._isTouchEvent(event) ? event.touches[0] : event;
==> When the event is "touchend" the event.touches array is empty. Instead changedTouches has the Touch object you need.
Documentation for the touches array (key portion "touch points that are currently in contact"):
A TouchList listing all the Touch objects for touch points that are currently in contact with the touch surface, regardless of whether or not they've changed or what their target element was at touchstart time.

What are the steps to reproduce?

I don't think this is available as an npm yet, so I can't get a stackblitz.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

I believe this was caused by #14140 and affects all touch-based browsers

Is there anything else we should know?

@crisbeto crisbeto self-assigned this Dec 5, 2018
@crisbeto
Copy link
Member

crisbeto commented Dec 5, 2018

@atscott what is the case in which the event.touches would be empty?

@atscott
Copy link
Contributor Author

atscott commented Dec 5, 2018

For a cdkDropList, it happens at the very end when you drop the element. This is a touchend event, which does not have any active touch points so the event.touches array is empty. This can be reproduced in the chrome developer console.

@atscott
Copy link
Contributor Author

atscott commented Dec 5, 2018

I think this could be solved simply with:
const point = this._isTouchEvent(event) ? (event.touches[0] || changedTouches[0]) : event;

@crisbeto
Copy link
Member

crisbeto commented Dec 5, 2018

Yes, I can put in a fix for it, but first I want to understand how the issue happens. I wasn't able to reproduce it with the dev tools:

demo2

@atscott
Copy link
Contributor Author

atscott commented Dec 5, 2018

You're doing the right thing to reproduce this bug. I don't think this bug is in the 7.1.1 release (even though that was created 2 days ago and the PR was submitted 7 days ago), but will be in the next release. I don't see the bug fix for that PR in the release notes and if I inspect the source, I don't see the call to _getPointerPositionOnPage in _cleanupDragArtifacts.

@crisbeto
Copy link
Member

crisbeto commented Dec 5, 2018

Thanks, I see it when I run master.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround has pr G This is is related to a Google internal issue labels Dec 5, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 5, 2018
Fixes an error being thrown by `CdkDrag` when the `touchend` event fires.

Fixes angular#14390.
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 5, 2018
Fixes an error being thrown by `CdkDrag` when the `touchend` event fires.

Fixes angular#14390.
mmalerba pushed a commit that referenced this issue Dec 5, 2018
Fixes an error being thrown by `CdkDrag` when the `touchend` event fires.

Fixes #14390.

Marking as P2 since this will throw consistently on touch devices.
mmalerba pushed a commit that referenced this issue Dec 10, 2018
Fixes an error being thrown by `CdkDrag` when the `touchend` event fires.

Fixes #14390.

Marking as P2 since this will throw consistently on touch devices.
josephperrott pushed a commit to josephperrott/components that referenced this issue Jan 14, 2019
Fixes an error being thrown by `CdkDrag` when the `touchend` event fires.

Fixes angular#14390.

Marking as P2 since this will throw consistently on touch devices.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants