search

bbc / peaks.js

JavaScript UI component for interacting with audio waveforms
https://waveform.prototyping.bbc.co.uk
GNU Lesser General Public License v3.0
3.16k stars 277 forks source link

Mousewheel scroll not working #454

Closed chrisn closed 1 year ago

chrisn commented 2 years ago

I've received a bug report that mousewheel scrolling isn't working in some cases. On one user's system it seems that the mousewheel changes the deltaX value rather than deltaY in wheel events. This user has MacOS (12.4), but I don't know if this issue is Mac specific. The WheelEvent values are controlled by the host OS and possibly mouse/trackpad driver settings, so I think this is browser independent. I haven't been able to reproduce this locally.

On my Windows laptop, the current behaviour is:

There is a proposed change on this branch, which implements this behaviour:

@ffxsam, @jdelStrother, @tscz, @rowild Do you see any issues making this change?

jdelStrother commented 2 years ago

Nope, sounds good. And I can confirm that shift-mousewheel on mac currently doesn't scroll the waveform, and that that branch fixes it.

chrisn commented 2 years ago

Great, thanks for testing this!

chrisn commented 2 years ago

Now fixed on master.

chrisn commented 1 year ago

I have received some more feedback that this user would prefer not to have to press shift to scroll the waveform using the mousewheel.

Peaks.js currently tries to detect a vertical trackpad gesture, to always scroll the page, rather than the waveform.

If we change this, it would mean:

Or even add a flag to switch between this and the new behaviour in https://github.com/bbc/peaks.js/issues/454#issue-1272596832. I welcome thoughts and feedback. Thanks!

jdelStrother commented 1 year ago

I don't love the idea of vertical trackpad scrolls causing the waveform to scroll horizontally, that sounds pretty weird to me. However, I don't see any distinguishing features in the wheel event to tell the difference between a mousewheel and a vertical trackpad gesture 😕

image
chrisn commented 1 year ago

Yes, I agree. What I might do is add an option for this particular user and leave the default as it currently is, so they can try it out.

ffxsam commented 1 year ago

My two cents:

Since a Peaks waveform could likely exist on a page with other content, any sort of scroll action should be combined with shift to actually scroll (or zoom) the waveform. This obviously prevents a user who's scrolling down the page from getting "trapped" in the Peaks canvas area. This is a common pattern used by other embeddable widgets that use wheel/scroll (e.g. Google Maps).

Furthermore, there should be a lock-in effect for horizontal/vertical scrolling, so if a user starts doing shift + vertical scroll and the Y delta is greater than a certain # of pixels, the user is locked into a zoom action. That way, if their gesture drifts a bit to the left or right, it won't start scrolling the waveform.

chrisn commented 1 year ago

Thanks Sam, that's helpful.

rowild commented 1 year ago

I only used peaks in the context of position:absolute & overflow:hidden web apps / pages. Using peaks' waveform scrolling with SHIFT as @ffxsam describes is a good solution for pages with other content. But if there is a chance to not have to use SHIFT on absolute positioned pages, so peaks can be used like a DAW is used, that would be most welcome.

ffxsam commented 1 year ago

Yeah, agreed. An option to set the behavior would be ideal.

chrisn commented 1 year ago

Thank you all, just need to think of a good name for the option. The default will remain as it is today.

chrisn commented 1 year ago

v2.0.4 now released, which allows the waveform to be scrolled without pressing Shift:

const view = instance.views.getView('zoomview');
view.setWheelMode('scroll', { captureVerticalScroll: true });