-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
Add support for dynamic keyframes #1043
Conversation
@kof please review. |
onCreateRule(key: string, frames: JssStyle, options: RuleOptions): KeyframesRule | null { | ||
return keyRegExp.test(key) ? new KeyframesRule(key, frames, options) : null | ||
const plugin: Plugin = { | ||
onCreateRule(key, frames, options) { |
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.
why removing the types?
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.
ah gotcha
onChangeValue(val, prop, rule) { | ||
const {sheet} = rule.options | ||
|
||
if (rule.type !== 'style' || !sheet) { |
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.
afaik only style rule value can be changed, so the first check can be removed
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.
looks good!
…frames' into feat/add-support-for-dynamic-keyframes * origin/feat/add-support-for-dynamic-keyframes: v10.0.0-alpha.11 fix changelog issue id
What would you like to add/fix?
This fixes having a dynamic animation name.
I think we can have a better system to process the update style changes and not require this duplicate code.
Still missing some tests to make sure we don't break anything.