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

Expose some dom.js methods #2718

Closed
wants to merge 1 commit into from
Closed

Expose some dom.js methods #2718

wants to merge 1 commit into from

Conversation

nickygerritsen
Copy link
Contributor

This should fix #2683 by adding the four mentioned methods to videojs. I also added some simple tests that these methods exist and are correct aliases, but I'm not sure whether these should actually be in api.js?

@pam
Copy link

pam commented Oct 20, 2015

Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: cfe5212
Build details: https://travis-ci.org/pam/video.js/builds/86340136

(Please note that this is a fully automated comment.)

@nickygerritsen
Copy link
Contributor Author

....that Vista saucelabs thing...

@gkatsev
Copy link
Member

gkatsev commented Oct 20, 2015

Yeah, I'm trying to figure out why it's busted. Thanks for the PR.

@misteroneill
Copy link
Member

Quick note. This looks good, but I have a very similar branch (rebased on my modal-dialog branch, which is why there's no PR yet) that does this and a bunch of other things like rewriting the *Class methods to use classList where available and improve tests.

@gkatsev gkatsev mentioned this pull request Oct 26, 2015
18 tasks
@nickygerritsen
Copy link
Contributor Author

Thanks @misteroneill I like your PR even more :)

@misteroneill
Copy link
Member

I opened a PR for my branch: #2754

@gkatsev
Copy link
Member

gkatsev commented Nov 9, 2015

Closing in favor of #2754. Thanks @nickygerritsen for getting this started.

@gkatsev gkatsev closed this Nov 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.0: consider exposing some dom.js methods
4 participants