feat(ripple): handle touch events #7927
Conversation
googlebot
added
the
cla: yes
| if (!this.rippleDisabled) { | ||
| this._isMousedown = true; | ||
| const isSyntheticEvent = this._lastTouchStartEvent && | ||
| Date.now() < this._lastTouchStartEvent + ignoreMouseEventsTimeout; |
There was a problem hiding this comment.
Rather than using the Date.now(), you should be able to take the Event.timeStamp. I'm not completely sure about the browser support though, you may have to double check.
There was a problem hiding this comment.
As discussed on Slack, for browser compatibility we'll be using Date.now()
| * Timeout for ignoring mouse events. Mouse events will be temporary ignored after touch | ||
| * events to avoid synthetic mouse events. | ||
| */ | ||
| const ignoreMouseEventsTimeout = 800; |
There was a problem hiding this comment.
This seems a little high. Wasn't the timeout 300ms?
There was a problem hiding this comment.
On Polymer it's 2000ms IIRC and in Materialize it's 500. On Material 1.x we used 650ms as a touch buffer for the interaction service. 800ms should be fine, and it doesn't really matter that much anyway.
There was a problem hiding this comment.
The const name here should be uppercased to match the others' naming.
| /** Whether the mouse is currently down or not. */ | ||
| private _isMousedown: boolean = false; | ||
| /** Whether the pointer is currently down or not. */ | ||
| private _isPointerDown: boolean = false; |
There was a problem hiding this comment.
The boolean typing here is redundant.
devversion
force-pushed
the
feat/ripple-touch-support
branch
from
60d6019
to
f479940
Compare
| * Timeout for ignoring mouse events. Mouse events will be temporary ignored after touch | ||
| * events to avoid synthetic mouse events. | ||
| */ | ||
| const ignoreMouseEventsTimeout = 800; |
There was a problem hiding this comment.
The const name here should be uppercased to match the others' naming.
crisbeto
added
pr: lgtm
action: merge
devversion
force-pushed
the
feat/ripple-touch-support
branch
2 times, most recently
from
fa7e647
to
62109ab
Compare
|
@devversion lint error |
|
@devversion which devices did you test on? We have pretty much everything in the office |
* Now handles touch events properly. Fixes angular#7062
|
@jelbourn I tested it on a few browsers. Mostly desktop browsers. Chrome 62 (Win), Firefox 57 (Win), Edge, IE11, Safari 11, Chrome 62 (MacOS) and Chrome 62 (Android 7.0) |
devversion
force-pushed
the
feat/ripple-touch-support
branch
from
62109ab
to
e72592d
Compare
|
I'd like to double check on a few devices before merging |
* Now handles touch events properly. Fixes angular#7062
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Note: Tested it thoroughly already and it works great. Still want to test on more devices manually to make sure it works fine!
Fixes #7062