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

Added vjs-waiting class on waiting and removed in all non-waiting states #1351

Closed
wants to merge 3 commits into from

Conversation

paullryan
Copy link
Contributor

This is a fix that's working for me on #1225. I needed it so that I could hide the loading video box as we're doing vine like replacements for a project. I'm happy to further improve upon this prior to merge if needed. Thanks for the great project.

@heff
Copy link
Member

heff commented Jul 26, 2014

Thanks for this! Is listening to the playing event not enough?

@paullryan
Copy link
Contributor Author

If you just listen on the playing event a little black box tends to appear at the very beginning of the loading sequence that why I made it understand waiting and seeking more like the loading spinner. The idea for me was to be able to fully hide the player until the video starts running which allows use for things like image overlay vines.

I did this based on the needs from #1225 and my question on SO at http://stackoverflow.com/questions/24290317/video-js-classes-to-hide-video-while-loading.

@heff
Copy link
Member

heff commented Jul 29, 2014

Made a pull request against your branch to show some changes I would make.
paullryan#1

Can you try them out and see they work for you?

Split up seeking and waiting styles. Updated loading spinner to use them. Verified against use case.
@paullryan
Copy link
Contributor Author

Verified against my use case and merged. Thanks.

@heff heff closed this in c5fb952 Aug 1, 2014
heff added a commit that referenced this pull request Aug 1, 2014
@heff
Copy link
Member

heff commented Aug 1, 2014

@paullryan Thanks for the help! This is merged and will go out with the next release.

@paullryan
Copy link
Contributor Author

@heff Thanks for the great tool glad to help.

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

Successfully merging this pull request may close these issues.

2 participants