color.js
color.js copied to clipboard
Docs: Unclear how to specify both space and method in `toGamut()`
I'm currently working to add HCT support so people can use it to do tonal mappings. The color space is ported locally, but it will need to gamut map the colors in HCT to comparable results to Google.
I'm testing this on the latest release, unaltered. I noticed that the way color.js implements optional parameters seems to not work. Consider this example. Specifying the method
doesn't work as a keyword or positionally. I'm not sure how it is supposed to work.
> new Color('p3', [1, 0, 0]).toGamut('srgb', {method: 'clip'}).to('srgb').coords
[ 0.9999999999999999, 0.044569516204189065, 0.045931611140175944 ]
> new Color('p3', [1, 0, 0]).toGamut('srgb', 'clip').to('srgb').coords
[ 0.9999999999999999, 0.044569516204189065, 0.045931611140175944 ]
The function has a signature as follows:
export default function toGamut (color, { method = defaults.gamut_mapping, space = color.space } = {}) {
What am I missing? Does this convention just not work or am I somehow not using the function properly? At one point, I thought this used to work with keyword arguments.
> new Color('p3', [1, 0, 0]).toGamut({space: 'srgb', method: 'clip'}).to('srgb').coords
(3) [0.9999999999999999, 7.57826960906538e-16, 4.482525461924069e-16]
It's basically an overloading of two signatures: color.toGamut(space: string | ColorSpace)
and color.toGamut(options: object)
, i.e. if you specify any named options, the space needs to also be specified as a named option.
We need to document this better :/ In general, we need better API docs.
Yeah, I figured it out right before you posted. I just wasn't familiar with this syntax, and my intuition was wrong.
I wouldn't close it, it still indicates an issue — either a docs issue, or an API issue.
Nothing preventing us from also allowing toGamut(space: string, options: object)
. The main argument I can see against it is that we then need to implement a conflict resolution policy about what happens if you specify a color space via both arguments.
Sure. I think this isn't the first time I've hit this and had to figure out how to use it again 😅 .