Introduce new getThemeBackgroundColor utility function#69
Conversation
Add CSS parser as a dev dependency to be able to retrieve the hexadecimal of a theme's background color.
westonruter
left a comment
There was a problem hiding this comment.
Yes, I think this would serve well to be able to compute the default text highlight color, don't you? Given a dark background color, we'd presume a light text color, and thus a darker color for highlighting the text would be required. Otherwise, given a light background color, then the text would be assumed to be dark, and the highlight color should be light. I don't know what those specific colors should be, but this seems like what is required to inform that.
| PHP; | ||
|
|
||
| $document = strtr($document, array( | ||
| '{colorMapping}' => implode("\n", $backgroundColors), |
There was a problem hiding this comment.
Idea: Why not use var_export() instead of manually constructing the array above?
There was a problem hiding this comment.
Only because I didn't know var_export() existed! TIL
| /** @var Rule $value */ | ||
| foreach ($bgColor as $value) { | ||
| if ($value->getValue() instanceof Color) { | ||
| $color = preg_replace('/background(?:-color)?:\s|;/', '', (string) $value); |
There was a problem hiding this comment.
I think it would be preferable to use \Sabberworm\CSS\Value\Color::getColor() here so that the underlying RGB colors can be returned (as an array), as opposed to a hexadecimal representation (as a string).
| $color = preg_replace('/background(?:-color)?:\s|;/', '', (string) $value); | |
| $color = $value->getValue()->getColor(); |
This will avoid the need to re-parse the hex string, and the values can be passed straight into an rgb() function.
There was a problem hiding this comment.
I like this much better. I'm returning an array of numerical RGB values now instead.
| 'tomorrow-night-bright' => array( | ||
| 0 => 255, | ||
| 1 => 255, | ||
| 2 => 255, |
There was a problem hiding this comment.
Apparently #fff is getting parser without the r g b keys. Perhaps this should get normalized for highlight.php?
There was a problem hiding this comment.
Ahh oops, it wasn't that it was being parsed incorrectly, I just gave it the wrong default values when no color was given. Technically it's transparent but I'm just assigning it white. Should we indicate transparent somehow?
There was a problem hiding this comment.
I suppose a default of white is correct, since that is the browser default. That is unless there are dark schemes that also have transparent backgrounds?
There was a problem hiding this comment.
I don't think any dark themes have a transparent background. I was only thinking about @media(prefers-color-scheme) but I think at that point, it should be up to the theme designer to get colors right.
eab8ebc to
6a348c6
Compare
96f8a44 to
38fe996
Compare
Requested in westonruter/syntax-highlighting-code-block#20, this PR introduces a new utility function named
getThemeBackgroundColorthat will return the hexadecimal of the background used in a given theme.Is this what you had in mind, @westonruter? Introducing any color operations to perform on said hexadecimal values would be beyond the scope of this project. So those calculations would be left up to the WP plugin.