Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a formatHsl formatter #124

Closed
johno opened this issue May 14, 2021 · 8 comments · Fixed by #130
Closed

Add a formatHsl formatter #124

johno opened this issue May 14, 2021 · 8 comments · Fixed by #130

Comments

@johno
Copy link

johno commented May 14, 2021

Firstly, thanks for the great library!

With our usecase we want to be able to output color strings for hex, rgb, and hsl, though culori doesn't currently support hsl. For now I've just written my own stringifier but it feels like this might make sense in the library itself.

Proposed solution

Following the existing formatter API, it would accept a color or string and then return hsl/hsla based on whether there's an alpha less than 1.

culori.formatHsl('tomato')

I'm not super familiar with culori internals, but I see that hex/rgb handle the fixups from within the rgb color mode. Is there a way we can do the same thing with hsl? Is this something you would accept a PR for?

@danburzo
Copy link
Collaborator

Hi John, thanks for the kind words and for raising this issue.

The formatter API is indeed somewhat incomplete because there were a few decisions to be made and CSS Color Level 4 was still a bit in flux, so we ended up with the bare minimum. The hex variants sorted themselves out due to their inherent representation, and rgb() / rgba() follows the serialization algorithm in the CSSOM 1 spec.

CSS Color Level 4 proposes some changes, among which the fact that sRGB-derived color spaces (including hsl and hwb) should have their computed value (and in turn their serialized value) as rgb() / rgba(). Therefore we haven't got a precise blueprint to produce hsl() / hsla() strings, but I agree it's something that belongs in the library, since it's one of the few widely-supported color value syntaxes.

I propose the following serialization rules for formatHsl:

  • hue as <number>, as authored (i.e. no unwrapping of angles outside [0, 360)); a missing hue (for achromatic colors) serialized as 0.
  • saturation and lightness clamped to [0%, 100%], rounded to a sufficient but not unreadable precision, maybe two digits? (culori.round(2) is a function that can be used for this purpose).

How does it sound?

I would gladly take in a PR.

@johno
Copy link
Author

johno commented May 19, 2021

Yep, makes sense and sounds good to me. Should be able to whip together a PR in the next week or so. Thanks!

@danburzo
Copy link
Collaborator

@johno, I have a few free cycles at the moment, if you prefer I can pick this up.

@johno
Copy link
Author

johno commented Jul 23, 2021

Feel free to go for it! Been a bit busy lately so would greatly appreciate it. 🙏

@danburzo
Copy link
Collaborator

Added a first pass at the implementation in #130. Currently h, s and l are rounded to the nearest integer. Both rgb() and hsl() color notations explicitly accept float values in CSS Color 4 but I'm not sure what the support story is for floats in the hsl() notation. If it's as patchy as for rgb(), then I think it would be best to round the values in hsl as well. Otherwise, 2-digit precision floats sound better to me. Maybe there are some Web Platform Tests about this specifically...

@danburzo
Copy link
Collaborator

Added an issue on MDN's browser-compat-data repo, hopefully I can find some time in the near future to perform some tests and contribute the missing data.

@danburzo
Copy link
Collaborator

Seeing that <number> in general is well supported in the hsl() syntax, and float values in particular even more so, I've added two digits of precision to all values in the formatter and merged to master. formatHsl() will be available in the next release.

@johno
Copy link
Author

johno commented Jul 30, 2021

Thank you very much @danburzo. Your work and library are greatly appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants