Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Please also have a look at our
- Partial support for CSS Color Module Level 4:
- `rgb` and `rgba`, and `hsl` and `hsla` are now aliases (#797}
- Parse color functions that use the "modern" syntax (#800)
- Render RGB functions with "modern" syntax when required (#840)
- Add a class diagram to the README (#482)
- Add more tests (#449)

Expand Down
64 changes: 64 additions & 0 deletions src/Value/Color.php
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ public function render(OutputFormat $outputFormat): string
&& $this->allComponentsAreNumbers()
) {
return $this->renderAsHex();
} elseif ($this->shouldRenderInModernSyntax()) {
return $this->renderInModernSyntax($outputFormat);
}

return parent::render($outputFormat);
Expand Down Expand Up @@ -282,4 +284,66 @@ private function renderAsHex(): string

return '#' . ($canUseShortVariant ? $result[0] . $result[2] . $result[4] : $result);
}

/**
* The "legacy" syntax does not allow RGB colors to have a mixture of `percentage`s and `number`s.
*/
private function shouldRenderInModernSyntax(): bool
{
$function = $this->getRealName();
if ($function !== 'rgb' && $function !== 'rgba') {
return false;
}

$hasPercentage = false;
$hasNumber = false;
foreach ($this->aComponents as $key => $value) {
if ($key === 'a') {
// ALpha can have units that don't match those of the RGB components in the "legacy" syntax.
// So it is not necessary to check it. It's also always last, hence `break` rather than `continue`.
break;
}
if (!$value instanceof Size) {
// Unexpected, unknown, or modified via the API
return false;
}
$unit = $value->getUnit();
// `switch` only does loose comparison
if ($unit === null) {
$hasNumber = true;
} elseif ($unit === '%') {
$hasPercentage = true;
} else {
// Invalid unit
return false;
}
}

return $hasPercentage && $hasNumber;
}

/**
* @return non-empty-string
*/
private function renderInModernSyntax(OutputFormat $outputFormat): string
{
\end($this->aComponents);
if (\key($this->aComponents) === 'a') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we don't rely on $this->aComponents having the associative array keys being in a any particular order. (This also is nothing which we currently can annotate and hence check with PHPStan.)

Instead, we can use array_key_exists or isset.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order is specific and comes from the CSS function name (e.g. rgba). There's a newer one lab in which a is not alpha. a is only alpha if it is the final element (in all cases I'm aware of).

$alpha = $this->aComponents['a'];
$componentsWithoutAlpha = \array_diff_key($this->aComponents, ['a' => 0]);
} else {
$componentsWithoutAlpha = $this->aComponents;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use unset, we also don't need the if/else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except if we want

a local variable $hasAlpha

we might still need an else clause.

}

$arguments = $outputFormat->implode(' ', $componentsWithoutAlpha);
if (isset($alpha)) {
$arguments = $outputFormat->implode(
$outputFormat->spaceBeforeListArgumentSeparator('/') . '/'
. $outputFormat->spaceAfterListArgumentSeparator('/'),
[$arguments, $alpha]
);
}

return $this->getName() . '(' . $arguments . ')';
}
}
5 changes: 0 additions & 5 deletions tests/Unit/Value/ColorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ public static function provideValidColorAndExpectedRendering(): array
'rgb(0 119 0)',
'#070',
],
// The "legacy" syntax currently used for rendering does not allow a mixture of percentages and numbers.
/*
'modern rgb with percentage R' => [
'rgb(0% 119 0)',
'rgb(0% 119 0)',
Expand All @@ -112,7 +110,6 @@ public static function provideValidColorAndExpectedRendering(): array
'rgb(0 60% 0%)',
'rgb(0 60% 0%)',
],
//*/
'modern rgb with percentage components' => [
'rgb(0% 60% 0%)',
'rgb(0%,60%,0%)',
Expand All @@ -131,7 +128,6 @@ public static function provideValidColorAndExpectedRendering(): array
'rgb(0 119 0 / 50%)',
'rgba(0,119,0,50%)',
],
/*
'modern rgba with percentage R' => [
'rgb(0% 119 0 / 0.5)',
'rgba(0% 119 0/.5)',
Expand All @@ -144,7 +140,6 @@ public static function provideValidColorAndExpectedRendering(): array
'rgb(0 119 0% / 0.5)',
'rgba(0 119 0%/.5)',
],
//*/
'modern rgba with percentage RGB' => [
'rgb(0% 60% 0% / 0.5)',
'rgba(0%,60%,0%,.5)',
Expand Down