The issue has been closed

Color conversion results in NaNs #195

lukeocodes posted onGitHub

Thanks for this library folks! I just found that there appears to be a division by zero error. A NaN can appear in the RGB array from a simple zero in the pallet. Searching past issues and all mentions of this appeared to be fixed before?

const color = Color("#0000ff");

console.log(
  color,
  color.whiten(),
  color.lighten()
);

The results:

color:

Color {model: "rgb", color: Array(3), valpha: 1}
color: (3) [0, 0, 255]
model: "rgb"
valpha: 1
__proto__: Object

color.whiten():

Color {model: "hwb", color: Array(3), valpha: 1}
color: (3) [240, NaN, 0]
model: "hwb"
valpha: 1
__proto__: Object

color.lighten():

Color {model: "hsl", color: Array(3), valpha: 1}
color: (3) [240, 100, NaN]
model: "hsl"
valpha: 1
__proto__: Object

What do you expect whiten() and lighten() to do without passing the ratio parameter?

posted by Qix- about 4 years ago

Sorry, @Qix- you're right. this is an RTFM moment.

Would it be better to assume a value or return an error as opposed to returning a result?

posted by lukeocodes about 4 years ago

There is no default value that would make sense, and checking the type of the argument would add a branch, reducing performance in tight loops (which color is frequently a part of).

As with color-convert, I'm considering two flavors of the library where there's a import Color from 'color/unchecked' for those with performance critical code. If this becomes the case for the next major release, then your first example will begin to throw errors about invalid arguments as you'd expect.

As for right now however, I haven't officially planned it quite yet.

posted by Qix- about 4 years ago

If it's okay with you I'll go ahead and close this for now - let me know if there's anything else :)

posted by Qix- about 4 years ago

No problem! Thanks for the speedy reply 👍🏻

posted by lukeocodes about 4 years ago

Fund this Issue

$0.00

Funded

Pull requests

Color conversion results in NaNs #195

Company

Community

Support

Connect