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

CSS output for LCH color spaces uses NaN, producing invalid color #357

Closed
haykam821 opened this issue Sep 3, 2024 · 3 comments · May be fixed by WontonSam/style-dictionary#218
Closed
Assignees
Labels

Comments

@haykam821
Copy link

Hey! With chroma.js 3.0.0, the css method seems to produce invalid values for certain colors:

chroma('white').lch(); // [100,0,NaN]
chroma('white').css("lch"); // "lch(100% 0 NaNdeg)"

Tested in a browser, lch(100% 0 NaNdeg) is an invalid color value. I would expect the css method to always provide a valid color value, even if the color doesn't necessarily make sense in the given color space.

I believe this issue is not a duplicate of #322, as the css method previously took care when handling NaN, regardless of whether hsl included NaN in its output:

chroma('white').hsl(); // [NaN,0,1,1]
chroma('white').css("hsl"); // "hsl(0deg 0% 100%)"
@vinerz
Copy link

vinerz commented Sep 8, 2024

+1 on this issue. Having to use a few 0.003 to prevent a NaN, which obviously tints the color in an unwanted way

@vinerz
Copy link

vinerz commented Sep 8, 2024

I made a monkeypatch that gives me nausea, but it... works. At least my work can progress.

import chroma, { Color } from "chroma-js";

Color.prototype.__css = Color.prototype.css;

Color.prototype.css = function cssPatch(format: string): string {
  return this.__css(format).replaceAll("NaN", "0");
};

@gka gka self-assigned this Sep 8, 2024
@gka gka added the bug label Sep 8, 2024
@gka gka closed this as completed in 999a2b2 Sep 8, 2024
@gka
Copy link
Owner

gka commented Sep 8, 2024

thanks for reporting this. I fixed the bug in the 3.1.0 release.

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

Successfully merging a pull request may close this issue.

4 participants
@gka @vinerz @haykam821 and others