[FEATURE] Support CSS custom properties #1336
Conversation
JakeQZ
force-pushed
the
feature/css-variables
branch
2 times, most recently
from
7ab14cc
to
9a010a3
Compare
|
This uses some |
Now fixed (as a separate commit, so it can still be seen what the PHP 8 code would look like). |
Add a new `HtmlProcessor` class that can evaluate CSS custom properties (variables) after CSS which uses them has been inlined. Resolves #1276, though an additional feature to remove unused variable definitions after evaluation could be provided.
oliverklee
force-pushed
the
feature/css-variables
branch
from
e2f4e9a
to
0a82f6f
Compare
There was a problem hiding this comment.
Amazing work! I've added some comments, both regarding the general architecture as well as some nitpicks.
I've also rebased the branch and autoformatted the README. So you'll probably need to hard-reset your local branch to the remote one (or delete the local copy and then use the remote one).
| /** | ||
| * @param array<non-empty-string, string> $declarations | ||
| * | ||
| * @return array<non-empty-string, string>|false `false` is returned if no substitutions were made. |
There was a problem hiding this comment.
Would it make sense to return null instead of false? Then we can make this a nullable type instead of a union type.
| { | ||
| $substitutionsMade = false; | ||
| $result = \array_map( | ||
| function (string $propertyValue) use (&$substitutionsMade): string { |
There was a problem hiding this comment.
Using all these used array functions feels to me like we maybe could use another class structure to have more type safety and to make the code more communicative. What do you think?
There was a problem hiding this comment.
Not sure what you mean or are suggesting. The method performs a transformation on each element of an array, for which array_map does the job. The reason to keep track of whether this transformation made any changes is an optimization. PHP's copy-on-write handling of arrays helps, but if a change is made, comparing $newArray !== $oldArray means checking each element (until the difference is found). (If the array is unmodified, the test is instant, since internally they'll still be referencing the same array.)
Many of the other code examples (PHP, CSS or HTML) in the README still have only two spaces indentation. (Those that are CSS or HTML are in bullet-points, though the PHP ones are also at main-content level.) I've spotted I mistake in the sample HTML I added, which I'll fix. |
Add a new
HtmlProcessorclass that can evaluate CSS custom properties (variables) after CSS which uses them has been inlined.Resolves #1276, though an additional feature to remove unused variable definitions after evaluation could be provided.