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 missing formats to color spaces #435

Closed
wants to merge 1 commit into from

Conversation

lloydk
Copy link
Collaborator

@lloydk lloydk commented Feb 14, 2024

Some color spaces couldn't be parsed because they didn't have any formats.

Added formats for:

  • a98rgb-linear
  • p3-linear
  • prophoto-linear
  • xyz-abs-d65

I used a dashed-ident for all of the spaces above similar to #430.

Some color spaces couldn't be parsed because they didn't have
any formats.
Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for colorjs ready!

Name Link
🔨 Latest commit 1e984e1
🔍 Latest deploy log https://app.netlify.com/sites/colorjs/deploys/65cc26103892380008aa4876
😎 Deploy Preview https://deploy-preview-435--colorjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@LeaVerou LeaVerou left a comment

Choose a reason for hiding this comment

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

Hey, thanks for working on this!

In general, as an API Design principle, I think boilerplate should be avoided. It makes for annoying DX, increases the chance of errors, and makes it harder to change things. Software is much better than humans at generating repetitive structures; therefore any boilerplate needed should be generated automatically by the software.

How dies this apply here? So, I think we should handle this a bit differently (which is how I thought it was handled, and maybe it was and it broke at some point): Every color space, automatically, gets a color() format supporting a dashed ident and/or ident for the color name plus <number> | <percentage> for the coords (or <percentage> | , I'm fine either way). Authors can still specify color` formats explicitly, and they will override this but we should not risk having color spaces with no formats because an author forgot to include this.

What do you think?

@lloydk
Copy link
Collaborator Author

lloydk commented Feb 14, 2024

How dies this apply here? So, I think we should handle this a bit differently (which is how I thought it was handled, and maybe it was and it broke at some point): Every color space, automatically, gets a color() format supporting a dashed ident and/or ident for the color name plus <number> | <percentage> for the coords (or <percentage> | ``, I'm fine either way). Authors can still specify color` formats explicitly, and they will override this but we should not risk having color spaces with no formats because an author forgot to include this.

That's similar to an idea I've had for a while which is the coordGrammer for most spaces could be generated from the coordinate metadata. So in addition to number or percentage angle coordinates would also be supported in the auto generated color format.

@LeaVerou
Copy link
Member

That's similar to an idea I've had for a while which is the coordGrammer for most spaces could be generated from the coordinate metadata. So in addition to number or percentage angle coordinates would also be supported in the auto generated color format.

That would not be spec compliant as it's unlikely for color() to ever support angles. I think for spaces that need angles, it makes more sense to have dedicated functions (potentially prefixed? Not sure).

@lloydk
Copy link
Collaborator Author

lloydk commented Feb 16, 2024

That's similar to an idea I've had for a while which is the coordGrammer for most spaces could be generated from the coordinate metadata. So in addition to number or percentage angle coordinates would also be supported in the auto generated color format.

That would not be spec compliant as it's unlikely for color() to ever support angles. I think for spaces that need angles, it makes more sense to have dedicated functions (potentially prefixed? Not sure).

We allow a coordGrammar to be specified for the color function (#370) and a number of color spaces that aren't a part of CSS (hpluv, hsluv, lchuv, hct) allow angle units for their angle coordinates. Should those spaces be changed to only allow number and percentage?

@LeaVerou
Copy link
Member

@lloydk I think so (together with a custom function), but we should discuss in a new issue.

@lloydk
Copy link
Collaborator Author

lloydk commented Feb 16, 2024

@lloydk I think so (together with a custom function), but we should discuss in a new issue.

Created a new issue #442

@jgerigmeyer
Copy link
Member

Closed, replaced by #439.

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.

None yet

3 participants