[getAll] Support precision, closes #542

[DmitrySharabin]

Ensure that it’s something you had in mind, though. My approach is relatively straightforward. We should probably have an optional options parameter with precision as a property.

[netlify]

❌ Deploy Preview for colorjs failed. Why did it fail? →

Name	Link
🔨 Latest commit	aa6db63
🔍 Latest deploy log	https://app.netlify.com/sites/colorjs/deploys/6669a09a10e10d000804bed9

[LeaVerou]

I don't think we want a separate parameter here, we'd want to make the second param an options object with space and precision as keys. Specifying a color space (either as a string or ColorSpace object should still work.

[DmitrySharabin]

I don't think we want a separate parameter here, we'd want to make the second param an options object with space and precision as keys. Specifying a color space (either as a string or ColorSpace object should still work.

Agreed. Done. Could you please have another look?

[LeaVerou]

Please note that I said:

Specifying a color space (either as a string or ColorSpace object should still work.

Using space as an options param should only be needed when we’re also specifying precision.

[MysteryBlokHed]

Something like this in toGamut.js could be used maybe?

color.js/src/toGamut.js

Lines 69 to 71 in 3f97a65

	if (util.isString(arguments[1])) {
	space = arguments[1];
	}

Where a destructured object is used as a parameter, but then arguments is used to check if something else was passed. So if it's a string or instance of a color space, then space is just set to arguments[1].

[DmitrySharabin]

Something like this in toGamut.js could be used maybe?

color.js/src/toGamut.js

Lines 69 to 71 in 3f97a65

	if (util.isString(arguments[1])) {
	space = arguments[1];
	}

Where a destructured object is used as a parameter, but then arguments is used to check if something else was passed. So if it's a string or instance of a color space, then space is just set to arguments[1].

I was thinking of something like this, too. Thank you so much for the hint! 🙏

[MysteryBlokHed]

It doesn’t seem to me like TypeScript is properly understanding the type here. Maybe making a @typedef for the function’s options would be best? I can add a code suggestion for that you agree

[LeaVerou]

Yes I think a typedef would make sense here. Though I thought we had found a way to do overloads?

[MysteryBlokHed]

Overloads would work too, but I still don’t think it’s a bad idea to define the options outside of the function’s JSDoc just for readability. We can have one overload with string | ColorSpace and another with the new type (eg. GetAllOptions).

[DmitrySharabin]

@MysteryBlokHed, could you please check if I get the idea with overloads right?

[MysteryBlokHed]

Everything typing-related LGTM!

[LeaVerou]

@DmitrySharabin do you have access to edit the release notes for v0.6.0 to mention this? (make sure not to accidentally publish!)

[DmitrySharabin]

@DmitrySharabin do you have access to edit the release notes for v0.6.0 to mention this? (make sure not to accidentally publish!)

It looks like I don't — I don't see any drafts of release notes.