Mouse handler updates, add trackpad horizontal scrolling

[jdelStrother]

The main feature I wanted to add here was responding to mousewheel events, so that doing a horizontal swipe on the zoomview waveform scrolls it. It feels pretty nice on an Apple Mouse & trackpad, but would welcome any feedback how well it works on a conventional clicky scrollwheel if anyone has one.

There's also a grabbag of minor fixes & cleanup around MouseDragHandler (which would skip a drag event if you dragged back over the position you started at), and it exposes a public zoomview.scrollWaveform method to allow scrolling from an external control.

I put the mousewheel listener in MouseDragHandler because it fits pretty cleanly in there, but concede it's not actually a drag as such. Happy for other suggestions...

[tscz]

Hi @jdelStrother: Cool, supporting the mousewheel/trackpad/gestures in peaks.js would be great. I would definitely be a happy user of this feature!

I tested your branch on my local windows machine with a scrollwheel mouse and SHIFT+Scrollwheel works fine. As a user I caught myself constantly expecting that only using the Scrollwheel would control the zoomlevel though. Most of other Waveform supporting tools are doing it in such a way (i.e. Native Instrument's Machine).

I also tried the peaks.js demo on my Ipad and scrolling works fine with swiping. But also here using the two finger gesture for enabling zoom on the waveform would be a great enhancement of the user experience IMHO.

[chrisn]

Related discussion: https://github.com/bbc/peaks.js/discussions/373

[chrisn]

Thanks! I tried your branch but found there's a lot of flicker when I use the scrollwheel, caused by non-integer values being passed to WaveformZoomview._updateWaveform().

Using the shift key to allow control from mouse scrollwheel is nice. I've implemented scrollwheel support based on your code in https://github.com/bbc/peaks.js/commit/fca2787870bebe68f80eda002c52e64a1fb65a1e. It's also outside MouseDragHandler, as the scrollwheel isn't dependent on the drag functionality there.

Scroll wheel support is not enabled by default, so you'll need to either call zoomview.setWheelMode('scroll') or use an option flag (see #387):

Peaks.init({
  zoomview: {
    container: document.getElementById('zoomview-container'),
    wheelMode: 'scroll'
  },
  // etc
},
function(err, peaksInstance) {
});

This is to allow a future option to set the zoom level, as @tscz (and others) suggest - although I have a concern adding that because it can be slow for long audio files. We should discuss in a separate issue.

About the other change for missed events: is that caused by this check?https://github.com/bbc/peaks.js/blob/fca2787870bebe68f80eda002c52e64a1fb65a1e/src/mouse-drag-handler.js#L109

[jdelStrother]

Thanks! I tried your branch but found there's a lot of flicker when I use the scrollwheel, caused by non-integer values being passed to WaveformZoomview._updateWaveform().

Oh, weird. I think I only ever saw integer values MouseWheelEvent.deltaX

This is to allow a future option to set the zoom level, as @tscz (and others) suggest - although I have a concern adding that because it can be slow for long audio files. We should discuss in a separate issue.

Yeah, I'm ambivalent about using mousewheel to scroll. The zoom intervals are so large (at least with our setup with long audio) that it feels weird coupling the zoom to a 'tactile' gesture like a mousewheel.

About the other change for missed events: is that caused by this check?

Yep, both that check in drag handler, and this one in zoomview -

https://github.com/bbc/peaks.js/blob/fca2787870bebe68f80eda002c52e64a1fb65a1e/src/waveform-zoomview.js#L147

If you like I'll rebase this branch onto your recent changes & re-submit it

[chrisn]

The other thing I didn't quite follow are the changes to the MouseDragHandler callback signatures. Is it enough to simply remove those conditions? (I don't remember why I added them initially, there must have been some reason for them...)

[jdelStrother]

The other thing I didn't quite follow are the changes to the MouseDragHandler callback signatures.

Zoomview records the position of the initial mousedown just so it can calculate how far the mouse has moved in the move event, which seems like something a MouseDragHandler ought to be smart enough to provide. I don't have a strong feeling about it though, happy to lose it when I rebase my changes.

[jdelStrother]

Is it enough to simply remove those conditions? (I don't remember why I added them initially, there must have been some reason for them...)

I believe so. I think it was probably an attempt to reduce unnecessary redraws (eg if you start the drag and just move the mouse vertically), but not sure it was ever quite correct. It does seem weird that it's comparing the current position to the initial position, rather than the last-known position.

[chrisn]

I think you're right. And thanks for fixing it! Rather than rebase, I can cherry-pick your changes.

[jdelStrother]

I think all the useful bits of this have been cherry-picked, shall I close?

[chrisn]

Thanks, yes I think I've got everything.