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

Add touchscreen drag/zoom support #93

Merged
merged 6 commits into from
Apr 14, 2019

Conversation

JJJollyjim
Copy link
Contributor

This merge request adds support for full interaction with the canvas through dragging and pinch-to-zoom on mobile devices. It uses the Pointer Events API (a unified touch+mouse API), which is not supported in current Safari without a polyfill (though is in the preview version).

To summarise, without a polyfill:

  • Safari loses mouse support
  • Android-Chrome gains touch support

And with a polyfill:

  • All browsers keep mouse support
  • Android-Chrome gains touch support, as do Safari-iOS, Firefox-Android, and desktop browsers.

On the assumption that dropping safari mouse support is an issue, the second commit adds a pointer event polyfill.

I have tested this change (including the polyfill) on:

  • Firefox on Linux (mouse still works)
  • Chrome on Linux (mouse still works, touch now works)
  • Safari on OSX (mouse still works)
  • Chrome on Android (touch now works)
  • Firefox on Android (touch now works)
  • Safari on iOS (touch now works)

Hopefully this commit can be reverted at some point in the future
Copy link
Member

@qu1ck qu1ck left a comment

Choose a reason for hiding this comment

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

Thanks a lot for adding touch support. Some comments below.

InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Show resolved Hide resolved
InteractiveHtmlBom/web/pep.js Show resolved Hide resolved
@qu1ck
Copy link
Member

qu1ck commented Apr 13, 2019

Hey @JJJollyjim please let me know if you will be able to address the review comments soon. I plan on releasing v2.1 soon and would like to get this in but it's ok if you don't have time right now. In that case it will go in v2.2 or whenever it is ready.
Unless I hear from you I will tag v2.1 on Sunday.

@JJJollyjim
Copy link
Contributor Author

Oh yes I'm busy tonight but will respond tomorrow, thanks for reminding me :)

@JJJollyjim
Copy link
Contributor Author

I accidentally clicked resolve on the thread about where to center the zooming and can't get it back, but I made a quick video to demo my approach since you mentioned you can't test it: https://youtu.be/g2JyAtq1eE8

In short, zooming on the other finger matches the bahaviour of the OS and doesn't seem to show any shakiness

@JJJollyjim
Copy link
Contributor Author

I'll push my changes for the style/minifciations comments when I'm back at my computer

@qu1ck
Copy link
Member

qu1ck commented Apr 14, 2019

I think this is in good shape now and I could merge now, take care of touch reset later. Unless you already have some work done?

@JJJollyjim
Copy link
Contributor Author

Just finished and pushed it :)

InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
InteractiveHtmlBom/web/render.js Outdated Show resolved Hide resolved
@qu1ck qu1ck merged commit 0423adf into openscopeproject:master Apr 14, 2019
@qu1ck
Copy link
Member

qu1ck commented Apr 14, 2019

Thanks for the contribution and patience with code review!

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