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

Volume slider sticky and jumps on Chrome #6989

Closed
appinica opened this issue Dec 9, 2020 · 9 comments · Fixed by #7094
Closed

Volume slider sticky and jumps on Chrome #6989

appinica opened this issue Dec 9, 2020 · 9 comments · Fixed by #7094

Comments

@appinica
Copy link

appinica commented Dec 9, 2020

Description

See https://streamable.com/hcvagg for a short video demo.

When sliding the volume control there are times it sticks and then repositions incorrectly, it only happens when sliding down to mute.

https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0

It happens for me on this start template video, I'm using Google Chrome (doesn't happen with Safari or Firefox).

On the video.js website if I select the 'fantasy' demo which uses a similar volume slider, the issue does not seem to occur. However if I use the fantasy theme on my own project the issue does occur, like with the default theme as shown in the linked video.

Only seems to happen when the cursor shows as a pointer too.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

  1. Load the codepen in Chrome
  2. Hover on volume icon to reveal slider
  3. Slide to the left to mute, slow down as you near the mute point and then continue sliding further

Results

Expected

I expect the slide circle to stop at the mute point and remain there, and then smoothly increase if I slide to the right.

Actual

As seen in video.

Error output

No errors.

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

7.10.2

browsers

Google Chrome: Version 87.0.4280.88 (Official Build) (x86_64).

OSes

macOS Catalina 10.15.5

@gkatsev
Copy link
Member

gkatsev commented Dec 9, 2020

This may be related to us tying muted and volume state together in the UI. We store the last volume value and restore it on mute/unmute and also drag to zero. It's probably a weird interaction with that.
I think that in our recent change to the slider logic, it potentially calculates things incorrect once you mouse out of the volume control itself.

@appinica
Copy link
Author

Hello, thanks for your comment.

Are you able to provide which version of video.js is running on https://videojs.com/? As mentioned previously the issue doesn't occur for me on Chrome when using the volume slider on the fantasy theme demo.

You mentioned the tying of muting and volume state together in the UI, which version of video.js was this change introduced in?

@gkatsev
Copy link
Member

gkatsev commented Dec 11, 2020

Looks like videojs.com is using v7.9.2. In 7.9.4 we merged this PR #5773, so, I assume it may be related.

@kenluck2001
Copy link

Let me know if this issue is still available @gkatsev . This can serve as a good first time ticket.

@gkatsev
Copy link
Member

gkatsev commented Jan 21, 2021

It's available, feel free to take a look, though, I'm not sure if it's a great first ticket.

@kenluck2001
Copy link

I am starting to work on it. Hopefully, I will get to understand the source code in the process.

@kenluck2001
Copy link

kenluck2001 commented Jan 22, 2021

I am starting to work on it. Hopefully, I will get to understand the source code in the process. My first investigation show that the issue is related to the slider control translating when you move the scrubber. This momentarily changes the position of the volume control thereby accepting an inadvertently clicking on the volume, thereby muting the play. I have reproduced on my system

@kenluck2001
Copy link

kenluck2001 commented Jan 23, 2021

I am seeing that there are two sliders (volume slider and playback slider). When you over controls, we have both sliders are horizontal sliders.

I am recommending two changes

  1. modify the event argument from PR improve: Better mouse position handling #5773 to accepting the current event on line 611 of src/js/utils/dom.js
    from
  const boxTarget = findPosition(event);

to

  const boxTarget = findPosition(event.currentTarget);
  1. Change the volume slider than appear on a mouseover event to be a vertical slider, so it does not conflict with the playback scrubber for space leading to unnecessary interaction effects.

The current volume slider is

<div class="vjs-volume-panel vjs-control vjs-volume-panel-horizontal"><button class="vjs-mute-control vjs-control vjs-button vjs-vol-2" type="button" title="Mute" aria-disabled="false"><span aria-hidden="true" class="vjs-icon-placeholder"></span><span class="vjs-control-text" aria-live="polite">Mute</span></button><div class="vjs-volume-control vjs-control vjs-volume-horizontal"><div tabindex="0" class="vjs-volume-bar vjs-slider-bar vjs-slider vjs-slider-horizontal" role="slider" aria-valuenow="51" aria-valuemin="0" aria-valuemax="100" aria-label="Volume Level" aria-live="polite" aria-valuetext="51%"><div class="vjs-volume-level" style="width: 51.22%;"><span class="vjs-control-text"></span></div></div></div></div>

Let me know if the proposals are reasonable and possible pointers for making the volume slider to be vertical. Once I get some feedback, I will proceed to PR @gkatsev

@rysolv-bot
Copy link

apowerful1 has contributed $25.00 to this issue on Rysolv.

The total bounty is now $25.00. Solve this issue on Rysolv to earn this bounty.

kamilbator added a commit to kamilbator/video.js that referenced this issue Feb 12, 2021
When sliding volume down to mute it sticks and then repositions incorrectly

Fixes videojs#6989
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2022
edirub pushed a commit to edirub/video.js that referenced this issue Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants