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

refactor player.dimension() #4704

Merged
merged 2 commits into from Nov 1, 2017

Conversation

Projects
None yet
3 participants
@kocoten1992
Contributor

kocoten1992 commented Oct 31, 2017

Requirements Checklist

  • refactor player.dimension()
@kocoten1992

This comment has been minimized.

Show comment
Hide comment
@kocoten1992

kocoten1992 Oct 31, 2017

Contributor

#4706 currently fail

Contributor

kocoten1992 commented Oct 31, 2017

#4706 currently fail

@@ -656,17 +656,16 @@ class Player extends Component {
if (value === '') {
// If an empty string is given, reset the dimension to be automatic
this[privDimension] = undefined;
} else {
const parsedVal = parseFloat(value);

This comment has been minimized.

@gkatsev

gkatsev Oct 31, 2017

Member

why not keep this variable? Then we won't need to parseFloat twice?

@gkatsev

gkatsev Oct 31, 2017

Member

why not keep this variable? Then we won't need to parseFloat twice?

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Oct 31, 2017

Member

I fixed master build and reran the tests here and it's passing now :)

Member

gkatsev commented Oct 31, 2017

I fixed master build and reran the tests here and it's passing now :)

@gkatsev

gkatsev approved these changes Nov 1, 2017

@gkatsev gkatsev merged commit ad1b47b into videojs:master Nov 1, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment