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

Enable touch events #83

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enable touch events #83

wants to merge 2 commits into from

Conversation

SteveALee
Copy link

@SteveALee SteveALee commented Aug 12, 2019

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).

  • I endevoured to stick with the existing code style - eg ES5
  • I don't know what your browser support policy is. It's possible some things don't work with older browsers. I couldn't find any docs about testing.
  • I could not find any tests to check for regressions
  • Works on Firefox & Chrome on Windows 10. Did not yet try iPad as do not have a bluetooth dongle that works with the DuoX

More comments in line below...

@@ -16,6 +16,7 @@
left: 0;
bottom: 45px;
right: 0;
touch-action: pan-x pan-y;
Copy link
Author

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>
Copy link
Author

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"
Copy link
Author

@SteveALee SteveALee Aug 12, 2019

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) {
Copy link
Author

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.

@falkTX
Copy link
Member

falkTX commented Aug 12, 2019

Thanks!
There are some oddities about spaces vs tabs indeed, but I can just import the changes manually as they are not too big.

$('body')[0].addEventListener('gesturestart', prevent)
$('body')[0].addEventListener('gesturechange', prevent)
$('body')[0].addEventListener('touchmove', prevent)
Copy link
Author

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

@ElijahLynn
Copy link

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?

@xko
Copy link

xko commented Feb 11, 2023

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.
How should I proceed now? new PR in this repo from my fork or PR in @SteveALee 's fork , then let him update this one?

@SteveALee
Copy link
Author

I'm out of action for a while so from my perspective please do what ever suits you and falk. Cheers

@xko
Copy link

xko commented Feb 13, 2023

@ElijahLynn if you have 1.12 on your device, you can try this. There's mod-deploy.sh script. But, beware: it ovewrites a lot of things, something can break - I bricked my dwarf at first, had to do emergency reinstall. Then I patched my version to overwrite only the javascript files, less risky now, but still..

@ElijahLynn
Copy link

@ElijahLynn if you have 1.12 on your device, you can try this. There's mod-deploy.sh script. But, beware: it ovewrites a lot of things, something can break - I bricked my dwarf at first, had to do emergency reinstall. Then I patched my version to overwrite only the javascript files, less risky now, but still..

thanks, I'll try it again soon when I get out my Duo (moved recently and not setup yet).

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.

4 participants