Skip to content

Share more code between hsl and hwb parsing. #293

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

Merged
merged 1 commit into from
Nov 26, 2021
Merged

Share more code between hsl and hwb parsing. #293

merged 1 commit into from
Nov 26, 2021

Conversation

emilio
Copy link
Member

@emilio emilio commented Nov 26, 2021

No description provided.

@emilio emilio merged commit 9c38d86 into master Nov 26, 2021
@emilio emilio deleted the hwb-followup branch November 26, 2021 00:23
@GPHemsley
Copy link
Contributor

I'm not sure this was the best decision.

hsl() and hwb() are defined independently of each other in the spec. They already have different support for legacy color syntax (i.e. commas) in css-colors-4, and their definitions could further diverge in the future (e.g. relative colors in css-colors-5).

In addition, in combining the code like this, you lose the correlation between the variable names and the spec, which makes the code harder to read.

I think you should revert these changes and take a different approach:

  • The spec explicitly defines the hue processing as being identical between HSL and HWB (for now, at least), so that could still be a candidate for factoring out.
  • And HWB doesn't support comma syntax, so that code can between removed from the corresponding function.

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 this pull request may close these issues.

2 participants