-
Notifications
You must be signed in to change notification settings - Fork 133
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
demo: change removed property limitVideoWidth to videoResolutionLimit #1345
demo: change removed property limitVideoWidth to videoResolutionLimit #1345
Conversation
9b058fa
to
ec3e8ac
Compare
ec3e8ac
to
5836e55
Compare
case "videoElement": | ||
videoResolutionLimitDescMsg = "The loaded video Representation will be throttled according to the given videoElement’s dimensions."; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should have a column limit to the demo.
We have one for the source but it does not seem to be enabled for the demo code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently there is one but with the options"ignoreStrings": true
that ignores lines that contains a string
I'm ok to remove this, but it should be done in a separate PR as there will be probably a lot of changes when correcting all warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I agree
@@ -23,7 +26,7 @@ const defaultOptionsValues = { | |||
}, | |||
onCodecSwitch: "continue", | |||
}, | |||
}; | |||
} satisfies { player: IConstructorOptions, loadVideo: ILoadVideoOptions }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
The code looks good to me, but I would like to know what you think about adding a max column length to the demo for readability purposes |
…mitVideoWidth demo: change removed property limitVideoWidth to videoResolutionLimit
…mitVideoWidth demo: change removed property limitVideoWidth to videoResolutionLimit
…mitVideoWidth demo: change removed property limitVideoWidth to videoResolutionLimit
…mitVideoWidth demo: change removed property limitVideoWidth to videoResolutionLimit
Property
limitVideoWidth
has been removed in the API for v4.0.0 but the changes was not reflected in the demo.This PR change this obsolete property by the new
videoResolutionLimit
in the demo.