-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Feature/vendor specific transition #1260
Feature/vendor specific transition #1260
Conversation
@@ -15,7 +17,8 @@ class Transition { | |||
add (decl, result) { | |||
let prefix, prop | |||
let add = this.prefixes.add[decl.prop] | |||
let declPrefixes = (add && add.prefixes) || [] | |||
let vendorPrefixes = this.ruleVendorPrefixes(decl) | |||
let declPrefixes = vendorPrefixes || (add && add.prefixes) || [] |
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 do we need add
here?
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 described it here.
lib/transition.js
Outdated
@@ -33,6 +36,8 @@ class Transition { | |||
if (!prefixer || !prefixer.prefixes) continue | |||
|
|||
for (prefix of prefixer.prefixes) { | |||
if (vendorPrefixes && !vendorPrefixes.includes(prefix)) continue |
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.
We can always have array in vendorPrefixes
(by returning []
instead of false
) and about this extra check.
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 like the idea of returning false
cause here we need to check whether rule is vendor specific. If it's not - we don't have to check the prefix. By the way I just found out the check itself
!vendorPrefixes.includes(prefix)
works incorrect. Prefix might be -webkit- 2009
for example. I'll fix it.
Released in 9.7.1 |
This PR should close #1217 . The issue is about
transition
property was prefixed with all of the prefixes even though not all of them are required (like-moz-
prefixes in-webkit-
rule).