Scoped keyframes #888
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| /* @flow */ | ||
| import warning from 'warning' | ||
| import SimpleRule from '../rules/SimpleRule' | ||
| import KeyframesRule from '../rules/KeyframesRule' | ||
| import ConditionalRule from '../rules/ConditionalRule' | ||
|
|
@@ -21,7 +22,7 @@ const classes = { | |
| /** | ||
| * Generate plugins which will register all rules. | ||
| */ | ||
| const plugins: Array<Plugin> = Object.keys(classes).map((key: string) => { | ||
| // https://jsperf.com/indexof-vs-substr-vs-regex-at-the-beginning-3 | ||
| const re = new RegExp(`^${key}`) | ||
| const RuleClass = classes[key] | ||
|
|
@@ -30,4 +31,27 @@ const plugins: $ReadOnlyArray<Plugin> = Object.keys(classes).map((key: string) = | |
| return {onCreateRule} | ||
| }) | ||
|
|
||
| // Animation name ref replacer. | ||
| plugins.push({ | ||
| onProcessStyle: (style, rule, sheet) => { | ||
| if (rule.type !== 'style' || !sheet) return style | ||
|
|
||
| // We need to support camel case here, because this plugin runs before the camelization plugin. | ||
| const prop = 'animationName' in style ? 'animationName' : 'animation-name' | ||
|
Member
There was a problem hiding this comment. I think we also need to support
Member
There was a problem hiding this comment. and also the
Member
Author
There was a problem hiding this comment. thought about it an decided to limit it for now, because it goes into difficult directions, adds more code and slows down. In the first case you would introduce the knowledge from jss-expand plugin about new syntax, in the second case, you would have to always run a regex or indexOf on the animation property. We already had to introduce camel case support in the core, which is suboptimal. In theory we could make this refs replacement a separate plugin, but then its not very logical that one can't reference keyframes from the same sheet without plugins.
Member
Author
There was a problem hiding this comment. Making it a separate plugin and running after jss-expand and jss-camel-case, would allow to just support "animation-name" property and rely on the
Member
Author
There was a problem hiding this comment. Another thought I had a while ago: what if we extract all core rules into separate packages and plugins? Most people use preset anyways. A few people can also setup by themselves all the plugins. This would give us the flexibility with above mentioned plugin.
Member
Author
There was a problem hiding this comment. Additional benefit: someone who doesn't need all the rules, doesn't have to include them. For e.g. we got a request to include @page rule and there are some more rarely used rules. Why not make them all separate packages, but included in default preset.
Member
There was a problem hiding this comment. Yeah, let's maybe make all of the rules plugins for now and then move them out maybe. We could do a This should allow us to support all of the mentioned case above without needing to introduce anything.
Member
Author
There was a problem hiding this comment. by "make all of the rules plugins" you mean just jss/src/plugins or making them separate packages each?
Member
Author
There was a problem hiding this comment. lets do this in a separate PR after the other PRs are merged, too many conflicts otherwise
Member
There was a problem hiding this comment. Okay, I think we need to apply the rule plugins last so that the keyframes name replace works on every possibility. |
||
| const ref = style[prop] | ||
| const isRef = ref && ref[0] === '$' | ||
| if (!isRef) return style | ||
|
|
||
| // We need to remove $ from $ref. | ||
| const name = ref.substr(1) | ||
|
|
||
| if (name in sheet.keyframes) { | ||
| style[prop] = sheet.keyframes[name] | ||
| } else { | ||
| warning(false, '[JSS] Referenced keyframes rule "%s" is not defined.', name) | ||
| } | ||
| return style | ||
| } | ||
| }) | ||
|
|
||
| export default plugins | ||
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.
Not a fan of this. I think we should rename all of the Rules to StylePlugin etc. and move this functionality to the KeyframesPlugin.
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.
yeah, that would be cleaner, but also more hooks and more code, not sure its worth it
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.
I think it's worth it.