-
Notifications
You must be signed in to change notification settings - Fork 68
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
Enable touch events #83
base: master
Are you sure you want to change the base?
Conversation
@@ -16,6 +16,7 @@ | |||
left: 0; | |||
bottom: 45px; | |||
right: 0; | |||
touch-action: pan-x pan-y; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop browser handling these events as the code does.
@@ -45,6 +45,7 @@ | |||
|
|||
<script type="text/javascript" src="js/lib/jquery-1.9.1.min.js?v={{version}}"></script> | |||
<script type="text/javascript" src="js/lib/jquery-ui-1.10.1.custom.min.js?v={{version}}"></script> | |||
<script> if (! ("ontouchend" in document)){document.ontouchend=''}</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it turns out jquery.ui.touch-punch.min.js
is broken as it checks for this and bails out if not set. No idea why. Perhaps an old touch events thing or only certain browsers? Who knows.
new Notification('info', 'BPM addressing removed, incompatible with ' + source + ' sync mode', 8000) | ||
} | ||
self.pedalboardModified = true | ||
var source = syncMode === "link" ? "Ableton Link" : "MIDI" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems somene has different editor settings for spaces/tabs. Suggest you use editorconfig
. or prettier
Also, for some reason even though I fetch and merged your upstream changes this was left out. I triple checked it is in your master.
@@ -239,7 +239,7 @@ JqueryClass('pedalboard', { | |||
}}); | |||
|
|||
// Dragging the pedalboard move the view area | |||
self.mousedown(function (e) { | |||
self.bind('mousedown touchstart', function (e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so jquery.ui.touch-punch.min.js
is not working for the desktop - we have to do the equivalent our selvess - not hard.
Thanks! |
$('body')[0].addEventListener('gesturestart', prevent) | ||
$('body')[0].addEventListener('gesturechange', prevent) | ||
$('body')[0].addEventListener('touchmove', prevent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is right pain. Should use {passive: false} to make it actually work and then older browser will mis interpret that as another option.. Chrome automatically assume ignore, but not firefox. Both throw warnings if don;t use passive:false unless use touch-action
in css, which I did
Thanks, I plan on trying this out soon. The UI used to work with touch, at least dragging the screen around and pedal placement, but it stopped some time ago and I am hopeful that your PR gets this working again! Do you have suggestions of how to test this on my Duo? |
Hi @falkTX , @SteveALee ! I improved on this a bit - added pinch-to-zoom, merged latest master. Also applied to hotfix-1.12, which I tested with Surface 6 and my Dwarf. Didn't test master though, since I sill have 1.12 on device. |
I'm out of action for a while so from my perspective please do what ever suits you and falk. Cheers |
@ElijahLynn if you have 1.12 on your device, you can try this. There's |
thanks, I'll try it again soon when I get out my Duo (moved recently and not setup yet). |
I was getting frustrated with touch not working on my windows PC (Surface Pro 4) so investigated. It now works perfectly with finger or pen.
It took much more effort than I expected due to the code being new to me and my being rusty with jQuery (at least you use an old version :-)) Plus Touch Events have a complex history, especially with jQuery not supporting them (maybe now).
More comments in line below...