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

loading spinner should be removed from DOM and not just set to display:none #518

Closed
tim-peterson opened this issue May 19, 2013 · 5 comments

Comments

@tim-peterson
Copy link

CSS transform has a tendency to max out the CPU even when the .vjs-loading-spinner is reset to display:none.

A better solution for loading spinner is to remove it from the DOM parent.removeChild(child) and then add it back parent.appendChild(child). I guess this could mean adding the following methods to video-js.js: this.add() and this.remove() to replace the cases where the loading spinner makes use of this.show() and this.hide().

Note: in the video-js.css file, .vjs-loading-spinner would be changed to display:inline-block .

@heff
Copy link
Member

heff commented Mar 4, 2014

Long term I'm thinking the right way to make the spinner work is to have it react to player css classes. e.g. when the player is waiting it should add the vjs-waiting class. Same with vjs-seeking. And then the spinner (including transforms) would be limited to only the times when the player has those classes.

@tim-peterson
Copy link
Author

@heff agree adding and removing CSS classes is the ways to go.

@mmcc
Copy link
Member

mmcc commented May 23, 2014

+1 - Just an internal reminder here :)

@heff heff closed this as completed in c5fb952 Aug 1, 2014
@heff
Copy link
Member

heff commented Aug 1, 2014

Thanks for your help on this Tim. #1351 addresses this if you're interested in reviewing it. It will go out with the next release.

@tim-peterson
Copy link
Author

Thanks @heff looks great! I agree className switching is the answer.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants