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.2k stars 279 forks source link

New contextmenu sets playhead position #447

Closed rowild closed 2 years ago

rowild commented 2 years ago

The new contextmenu feature causes the playhead to be set to the right-click position all the time, too. This is probably not what a user would expect to happen.

https://user-images.githubusercontent.com/213803/160275044-9cca8ee6-ac89-4c99-bc4e-812be8f86f3a.mp4

It seems this is connected to the seek feature of the player, unfortunately I am unable to find out how to fix this behaviour.

chrisn commented 2 years ago

Good point. The place to look in the code is MouseDragHandler and how this is used within WaveformZoomView. This handles mousedown and mouseup events, where seek happens on mouseup.

I'd need to think more about what the right solution is, but here are a couple of possibilities:

  1. Change Peaks.js to only use left-click to set the playhead position, which leaves right-click free to show a context menu
  2. Change Peaks.js so that right click does not change the playhead, but only if the user has added a contextmenu handler. If there's no contextmenu handler, right-click would also cause a seek.

What do you think?

rowild commented 2 years ago

Thanks, @chrisn , for your feedback! Here are my thoughts:

I am thinking about a situation, where I would use right-click to position the playhead. ATM I believe my habits do not incorporate such a behaviour, be it in a video recording / editing software, any other DAW I use or even a notation software. I checked Audacity (I think Peaks.js or waveform-data has some scripts that are based on Audacity), which does not use right-click to position the playhead.

Personally I use right-click always then, when I want to get additional (context-sensitive) functionality or information on something that I click on. At this moment I do not want to change anything yet.

Also: Since it is now possible to drag the playhead in zoomview (thanks to a recent implementation of yours), would that also mean that it has to be draggable using right-click, when there is no context menu? It currently is: when right-clicking on the waveform, the standard context menu pops up, which can be closed using the ESC key; after that the playhead "is stuck" to the mouse. A left-click frees up the playhead again.

My guts tell me that solution 1 - set playhead position with left-click only - should be the one to strive for.

But maybe others are used to a different UX/UI experience and would contradict me. So I pass on your question to all the other readers asking for feedback: What do you think?

chrisn commented 2 years ago

Thanks. I've implemented option 1, which seems like a reasonable solution.

rowild commented 2 years ago

You are so incredibly quick! Thank you very, very much!

rowild commented 2 years ago

@chrisn Any chance to push a new version to npm?

chrisn commented 2 years ago

v1.1.0 is now available in npm.

rowild commented 2 years ago

Muchas gracias!!