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

Handle tap touch actions. #730

Merged
merged 4 commits into from
Aug 10, 2017
Merged

Handle tap touch actions. #730

merged 4 commits into from
Aug 10, 2017

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Aug 8, 2017

This specifically supports styli on iPads.

Update documentation on mapInteractor.

/* In some scenarios, we get both a tap event and then, somewhat later, a
* set of mousedown/mouseup events. Ignore the spurious down/up set if we
* just handled a tap. */
if (m_touchHandler && m_touchHandler.lastEventType === 'tap' && (new Date()).valueOf() - m_touchHandler.lastTime < 1000) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth wrapping this long line? May be we could have a test for long lines - if makes sense we could allow devs to have length of 79/80 or 120 or not :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having a hard limit on line length. It makes it a pain to put things like reference urls in comments. In this case, I'll wrap the line.

Copy link
Member

@aashish24 aashish24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great to me, and I saw the demo as well. Individual commits helped reviewing the code quickly and systematically, thanks. Just had a minor comment on long line, the docs looks great.

@manthey manthey merged commit 05c5a3b into master Aug 10, 2017
@manthey manthey deleted the tap-support branch August 10, 2017 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants