Fix var function inside color functions #197
Conversation
lib/Sabberworm/CSS/Value/Color.php
Outdated
| $remainingChars = substr($sColorMode, $i + 1); | ||
| $aColor[$sColorMode[$i] . $remainingChars] = $aColor[$sColorMode[$i]]; | ||
| unset($aColor[$sColorMode[$i]]); |
There was a problem hiding this comment.
I do not really like this solution but, but it's necessary as the array keys are used in the render function below to define the color function. And because of the nature of var() it's not possible to easily tell which arguments are defined through the variable.
If it's not clear what I mean: rgba(var(--rg), 255) is valid CSS code and the variable value can contain a comma, so it's valid CSS and in the parser it's hard to tell whether it's for red and green. It's somehow possible to identify it, but this will result in quite some code to identify all combinations and I am not sure if its necessary.
There was a problem hiding this comment.
What if we parse colors that contain variables not as a Color instance but just an instance of a generic CSSFunction? I don’t see the additional value (pun intended) of using a specialized class when we can’t use it for any of the specializations we built it for…
There was a problem hiding this comment.
Good idea, I updated it as suggested.
|
This should also fix #186. |
|
@sabberworm Anything else you'd like to see from this? We'd like to include it in the v2.0 release of the AMP plugin for WordPress. |
|
@westonruter No, I just haven’t had time to do a proper review yet. I only wonder if it would be possible to solve this case generically as opposed to special-casing the color parsing. Next we’ll have someone wanting to use variables in |
I tried to fix #196. Not sure if it is good solution, but at least it works ;)