search

captbaritone / webamp

Winamp 2 reimplemented for the browser
https://webamp.org
MIT License
9.72k stars 641 forks source link

Allow a single mouse drag across the EQ to set all values #1180

Closed chriscareycode closed 1 year ago

chriscareycode commented 2 years ago

In WinAMP you could mouse down on the first EQ VerticalSlider and drag the mouse across all of the VerticalSliders to set them all in one swipe. This PR brings the same capability to WebAMP.

https://user-images.githubusercontent.com/1002638/177918378-2615b514-f093-417a-85aa-0ad902d3c3e0.mov

chriscareycode commented 2 years ago

  1. Done

  2. Done

  3. Done

chriscareycode commented 2 years ago

Related to Touch: I noticed a bug where if you touch the EQ, then the next subsequent touch on button like stop, play, will not work the first time you touch it. To repro: Touch Play. Touch Stop. Touch the EQ. Touch Play. It will not play, until you touch it a second time. I tested this on webamp.org and also seeing the bug there so it appears to be unrelated to this PR.

chriscareycode commented 2 years ago

Just came across this https://w3c.github.io/pointerevents/ This changes everything. PointerEvents gives us all the events we need for touch that do not exist in TouchEvents. I tested with VerticalSlider replacing both MouseEvent and TouchEvent with PointerEvents and everything just works without needing to have different code paths for mouse and touch. This is going to be interesting. I'm going to proceed to convert over to this.

chriscareycode commented 2 years ago

Ready for review. Github says "captbaritone requested changes" but I'm not sure how to ACK that or let it know the changes are done.

captbaritone commented 1 year ago

Looking better! One thing that's still not right though: In real Winamp, clicking outside the eq window and then dragging across the sliders does not cause the sliders to be moved. Also, clicking inside the eq window and the dragging over the preamp causes us to grab the preamp slider but we can't then glide into the band sliders.

I think I'd be happy with an abstraction that gave us a separate React component that defined an area that would capture these initial clicks. So, any click that happened in this new component or one of its children would trigger the behavior you have now, but if the click originated outside of an instance of this new component, the drags would be ignored.

chriscareycode commented 1 year ago

Getting closer. Fixed the bug where clicking outside the EQ area and dragging across the EQ will affect the sliders. Also fixed the issue that dragging across the EQ will affect the preamp. This is done with a new state "clickOriginatedInEq".

chriscareycode commented 1 year ago

Working better now. Tested with mouse, pen, and touch.

captbaritone commented 1 year ago

Thanks for all your work on this, and most of all your patience! I've been busy with other stuff so I haven't been able to muster the focus to give timely review. Sorry for that. There are a few details to address here, but rather than go back for another round, I'll go ahead and merge this and followup with the fixes when I find time. (I'll mention you so you can see, in case you are curious or have feedback for me on why you chose to do it differently.

This is such a great detail to have right on Webamp. I really appreciate you persevering and getting it added!