PHP-CSS-Parser icon indicating copy to clipboard operation
PHP-CSS-Parser copied to clipboard

Model named colors as `Color` instances instead of strings

Open oliverklee opened this issue 1 year ago • 3 comments

https://github.com/MyIntervals/PHP-CSS-Parser/issues/510#issuecomment-1972622917

oliverklee avatar Mar 01 '24 11:03 oliverklee

Looking at the class hierarchy, Color extends CSSFunction extends ValueList extends Value. But a named colour would be neither a CSSFunction nor a ValueList - though would be a Color. Not sure how best to reconcile this. In #353 I suggested the possibility of a Keyword/Identifier type that could represent a any sort of named value, including a named color and a border-style, with the non-specific type simply wrapping a string (of unknown meaning). Something to think about...

JakeQZ avatar Mar 03 '24 00:03 JakeQZ

Not sure how best to reconcile this.

Maybe the named colour class type would need to be quite separate in the class inheritance hierarchy, focusing more on how it is parsed than what it actually represents. If there's a need to provide common functionality between named colours and RGB colours, an interface could be provided, which both implement. That interface might be Color, with the existing Color being renamed to RgbColor.

These are just thoughts that might help with the design for this change.

JakeQZ avatar Mar 03 '24 01:03 JakeQZ

IMHO named colors should be parsed into rgb colors with an additional flag. On output, these could be rendered to names again using a reverse-lookup table. Also, we should add format options on whether to prefer outputting colors the way they were input, or as hex, as function, or as named (if possible).

sabberworm avatar Mar 03 '24 13:03 sabberworm