color-js / color.js

Color conversion & manipulation library by the editors of the CSS Color specifications
https://colorjs.io
MIT License
1.92k stars 82 forks source link

Wrong serialization result for rgba/hsla spaces #296

Closed kajkal closed 5 months ago

kajkal commented 1 year ago

There is this bug:

const c = new Color("rgba(255,0,0,1)");
c.toString({format: "rgba"}); // rgba(100%, 0%, 0%), expected rgba(100%, 0%, 0%, 1)

the fix is to check format.lastAlpha flag in the serialize.js function:

let strAlpha = (format.lastAlpha || (alpha < 1 && !format.noAlpha))
    ? `${format.commas? "," : " /"} ${alpha}`
    : "";

However, is it necessary to have two flags to determine whether a given format should include alpha? To my knowledge, there are only 3 cases: 1) alpha is required (rgba, hsla) 2) alpha is optional (alpha < 1 rgb/hsl for CSS Color 4) 3) alpha is ignored (rgb_number)

So the final flag should have 3 values, this can be achieved by, for example:

interface Format {
    /**
     * true - alpha always present in serialized string
     * undefined - default, alpha present when < 1
     * false - no alpha in serialized string
     */
    withAlpha?: boolean | undefined;
    // or more descriptively by string union:
    withAlpha?: "always" | "auto" | "never";
}
{ noAlpha: true } => { withAlpha: false }
{ lastAlpha: true } => { withAlpha: true }

Is there something I don't know about or can I make PR to merge noAlpha and lastAlpha flags into one?

LeaVerou commented 1 year ago

I like this idea. I'd rename the flag to simply alpha and use the enums "always", "never", "auto".

svgeesus commented 1 year ago

alpha is required (rgba, hsla)

Alpha is never required even for rgba() and hsla(), although it used to be (CSS Color 3 and earlier). This is true of both the legacy (comma-separated) and modern syntactic forms.

LeaVerou commented 5 months ago

This has now been implemented and will be in the upcoming v0.6.0 release. You can use alpha: true to force alpha even when it's 100%, alpha: false to prevent alpha from showing up, and commas: true to force commas. These together should give you the result you were looking for, even though as @svgeesus said, the current output is perfectly valid.