-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix(FEC-10844): advanced caption settings custom caption is always marked even when choose sample #525
Conversation
…rked even when choose sample Issue: condition of || doesn't work properly with 0 as current value and always take the second option. Solution: create the defaultValues as static getter to be able to use it on static methods and on first initilize of the properties. merge the setting and default value to make sure all set correctly. add relevant tests.
src/track/text-style.js
Outdated
let value = defaultValue; | ||
if (typeof newValue !== 'undefined' && newValue !== null) { | ||
value = newValue; | ||
} | ||
return value; |
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.
can be replace with a one liner:
return (typeof newValue !== 'undefined' && newValue !== null) ? newValue : defaultValue
But its a matter of taste...
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.
So unless you wanna change this to my suggestion then it is approved
Description of the Changes
Issue: the condition of || doesn't work properly with 0 as the current value and always takes the default option.
Solution: create the default values as a static getter to be able to use it on static methods and on the first initialize of the properties.
merge the setting and default value to make sure all set correctly.
add relevant tests.
CheckLists