Conversation
|
+1 for this PR, encountered the issue today! |
|
Bump |
| } | ||
|
|
||
| #css4-rgba { | ||
| background-color: rgb(242 245 249 / 45%); |
There was a problem hiding this comment.
It would be cool to have an output format option to prefer this syntax over the other. AFAICT there are three boolean options that are all independent of one another: prefer-rgb-over-rgba / rgb-alpha-separator-use-slash / rgb-alpha-use-percentage.
There was a problem hiding this comment.
It would, though can (and should) be done as a separate PR.
src/Value/Color.php
Outdated
|
|
||
| $bContainsVar = false; | ||
| $iLength = $oParserState->strlen($sColorMode); | ||
| if (strpos($sColorMode, 'rgb') !== false) { |
There was a problem hiding this comment.
rgb has to be at the start, right?
| if (strpos($sColorMode, 'rgb') !== false) { | |
| if (str_starts_with($sColorMode, 'rgb')) { |
There was a problem hiding this comment.
Though I prefer being explicit:
| if (strpos($sColorMode, 'rgb') !== false) { | |
| if ($sColorMode === 'rgb') { |
(the rgba case will be handled in the else branch anyway).
|
Hi, I've updated the PR to include the recommended stricter checks for |
|
Hi, I am using this library for the first time in https://www.drupal.org/project/ui_styles/issues/3453784. I am encountering the same issue has #357. And I confirm that the changes in this PR are fixing the bug. Thanks! What remains to be done to have it merged and shipped in a new release? |
JakeQZ
left a comment
There was a problem hiding this comment.
The code changes look good to me. However, it would be good to have a few more tests:
- to cover the various separators (space, comma, slash);
- to cover
hslwith alpha.
Also, could you add an entry to CHANGELOG.md (newest first).
In CSS Level 4
rgba()andhsla()are aliases forrgb()andhsl()which now accept the alpha argument. This PR reflects this behavior and makes the parser understand the syntax. Source: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgb()_and_rgba()Example: