Add vjs-scrubbing class to player while scrubbing #1741

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@jcaron23
Contributor

jcaron23 commented Dec 12, 2014

This is useful to add CSS preventing hover menus from opening while scrubbing, even if the mouse strays out of the seek bar.

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Dec 13, 2014

Member

Thank you! Are you just doing this so you can add .vjs-scrubbing styles in your app or did you have default classes in mind?

Member

mmcc commented Dec 13, 2014

Thank you! Are you just doing this so you can add .vjs-scrubbing styles in your app or did you have default classes in mind?

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Dec 16, 2014

Member

@jcaron23 just following up on this. When I test scrubbing on videojs.com and roll over the captions menu, the menu doesn't show. Is there a live example of this happening, or could we create a jsbin for it?

Member

heff commented Dec 16, 2014

@jcaron23 just following up on this. When I test scrubbing on videojs.com and roll over the captions menu, the menu doesn't show. Is there a live example of this happening, or could we create a jsbin for it?

@jcaron23

This comment has been minimized.

Show comment
Hide comment
@jcaron23

jcaron23 Dec 16, 2014

Contributor

Which browser did you test it on? Safari appears to block :hover while the mouse is down, but Firefox and IE don't. Haven't checked Chrome.

Contributor

jcaron23 commented Dec 16, 2014

Which browser did you test it on? Safari appears to block :hover while the mouse is down, but Firefox and IE don't. Haven't checked Chrome.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Dec 16, 2014

Member

Yeah, no issues on Safari or Chrome but I was able to reproduce it on Firefox. Thanks for pointing that out. Would you want to help update the existing menus to use the vis-scrubbing class?

Member

heff commented Dec 16, 2014

Yeah, no issues on Safari or Chrome but I was able to reproduce it on Firefox. Thanks for pointing that out. Would you want to help update the existing menus to use the vis-scrubbing class?

@jcaron23

This comment has been minimized.

Show comment
Hide comment
@jcaron23

jcaron23 Dec 16, 2014

Contributor

I currently use this CSS:
/* prevent menus from opening while scrubbing (FF, IE) */
.vjs-default-skin.vjs-scrubbing .vjs-menu-button:hover .vjs-menu {
display: none;
}

Not quite sure it necessarily fits in the general case.

Also, it might be better to actually have some sort of not-scrubbing class (so we can just have the :hover work only on not-scrubbing, but that probably requires quite a few more changes.

Contributor

jcaron23 commented Dec 16, 2014

I currently use this CSS:
/* prevent menus from opening while scrubbing (FF, IE) */
.vjs-default-skin.vjs-scrubbing .vjs-menu-button:hover .vjs-menu {
display: none;
}

Not quite sure it necessarily fits in the general case.

Also, it might be better to actually have some sort of not-scrubbing class (so we can just have the :hover work only on not-scrubbing, but that probably requires quite a few more changes.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Dec 16, 2014

Member

Nice, I think the first option looks good to me. Want to add it to the pull request?

Member

heff commented Dec 16, 2014

Nice, I think the first option looks good to me. Want to add it to the pull request?

@jcaron23

This comment has been minimized.

Show comment
Hide comment
@jcaron23

jcaron23 Dec 17, 2014

Contributor

Added (slightly edited) CSS rule to video-js.less

Contributor

jcaron23 commented Dec 17, 2014

Added (slightly edited) CSS rule to video-js.less

heff added a commit that referenced this pull request Dec 22, 2014

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Dec 22, 2014

Member

This has been merged into master and will go out with the next release. Thanks Jacques!

Member

heff commented Dec 22, 2014

This has been merged into master and will go out with the next release. Thanks Jacques!

@heff heff closed this Dec 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment