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

XYZ color picker confuses 0..1 and 0..100% range #2

Open
svgeesus opened this issue Jul 1, 2022 · 14 comments
Open

XYZ color picker confuses 0..1 and 0..100% range #2

svgeesus opened this issue Jul 1, 2022 · 14 comments
Labels
bug Something isn't working

Comments

@svgeesus
Copy link
Member

svgeesus commented Jul 1, 2022

The XYZ-d65 color picker has 0..100 scale but gives black for [0,0,0] and an oog color close to white for [1,1,1]; it is not possible to pick any other colors due to the slider granularity.

image

@svgeesus svgeesus added the bug Something isn't working label Jul 1, 2022
@svgeesus
Copy link
Member Author

svgeesus commented Jul 4, 2022

@LeaVerou

@LeaVerou
Copy link
Member

LeaVerou commented Jul 4, 2022

Maybe the max granularity shouldn't be 1 but .1? Take a look here and tell me how you think we should fix this generically. We can't just special case XYZ.

@svgeesus
Copy link
Member Author

svgeesus commented Jul 4, 2022

let {name, range, refRange} = meta;
	name = name || id;
	range = range || refRange || [0, 100];
	let [min, max] = range;
	let step = (max - min) / 100;
	if (step > 1) {
		step = 1;
}

Since xyzd65 and xyzd50 don't declare a range, they end up with [0, 100] and (100-0)/100 should be 1 but is, I suspect, something like 1.00000000000001 and thus greater than 1. An epsilon in there would catch this and make it more robust. Although why forcing there to be only 100 steps is beneficial could also be examined.

@svgeesus
Copy link
Member Author

svgeesus commented Jul 4, 2022

Giving the relative XYZs a reference range of [0, 1] would also work.

@svgeesus
Copy link
Member Author

(confirming this UI bug still exists)

@LeaVerou
Copy link
Member

Is this issue still present on https://apps.colorjs.io/picker/ ?
If so, please transfer to the apps repo.

@svgeesus svgeesus transferred this issue from color-js/color.js May 28, 2024
@svgeesus
Copy link
Member Author

Confirming that the issue still exists, because [0..1] and [0..100] are being confused

@LeaVerou
Copy link
Member

What should the range be for each XYZ space?

@svgeesus
Copy link
Member Author

As I said earlier:

Giving the relative XYZs a reference range of [0, 1] would also work.

@LeaVerou
Copy link
Member

@DmitrySharabin could we integrate the <color-picker> component to use here so we don't maintain similar code in two places?

@DmitrySharabin
Copy link
Member

@DmitrySharabin could we integrate the <color-picker> component to use here so we don't maintain similar code in two places?

@LeaVerou, I integrated the <color-picker> component (preview and corresponding PR). Please take a look.

@LeaVerou
Copy link
Member

Thanks @DmitrySharabin! Is this issue present in <color-picker> too? If not, we should close.

@DmitrySharabin
Copy link
Member

Is this issue present in <color-picker> too?

Unfortunately, it does. As @svgeesus mentioned, since xyzd65 and xyzd50 don't declare a range, they end up with [0, 100] (here and here). In <color-picker>, we don't fall back to [0, 100] (when calculating the default color) expecting that all the color spaces declare ranges for their coords (that make <color-picker> throw for the mentioned color spaces).

So, to solve this issue, we should probably declare ranges for those spaces. As Chris said:

Giving the relative XYZs a reference range of [0, 1] would also work.

@LeaVerou
Copy link
Member

Yeah, let's do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants