xiaokaike / vue-color

:art: Vue Color Pickers for Sketch, Photoshop, Chrome & more http://vue-color.surge.sh
https://xiaokaike.github.io/vue-color/
MIT License
2.54k stars 354 forks source link

Drag saturation pointer over the very bottom make hue pointer move #113

Open Ben-Mack opened 6 years ago

Ben-Mack commented 6 years ago

Step to reproduce:

vue-color-bug View this gif in high quality: https://media.giphy.com/media/l3diFoYW8rZnkfe4E/source.mp4

Also, I think the saturation pointer should not jump to the bottom left like that.

linx4200 commented 6 years ago

Hi, this is a known issue caused by tinycolor which we use as color conversion internally.

We are looking for replacement, and open to any suggestions.

paulm17 commented 6 years ago

I tried to tackle this issue. I went through a number of color converters. Even thought about doing my own implementation but then don't want to dive into precision issues (sigh).

I settled on Qix-/color as it was the only one which gave exact results to tinycolor2. I get the same result as @Ben-Mack.

Although @linx4200 mentions this is a known issue. I couldn't find any issues, perhaps they can link to it? But more importantly. Using Spectrum which is also developed by the author of the tinycolor lib. I couldn't replicate this issue. Have a look here: http://bgrins.github.io/spectrum

Using Spectrum myself for many years. I've never run against this issue.

I can only conclude, this is down to the component itself.

Regardless, I'm going to try and solve this issue. Breaking changes may occur though.


Follow Up.

The reason why this problem occurs, is due to the Hue being a computed property and based on the return value from color.js. When the pointer goes to the bottom and if you do it slow enough, it wildly shifts to the left. I can only guess at this point, precision is lost and then the computed property kicks into effect and the hue slider shifts also.

linx4200 commented 6 years ago

@paulm17

For the first slight wrong left jump:

const original = {
   h: 130.34428204985838, s: 0.6488888888888888, v: 0.008064516129032251, a: 1, source: "hsva"
}
const color = tinycolor(original); //  {h: 120, s: 0.5098039215686274, v: 0.008, a: 1}

tinycolor converts it incorrectly.

For the final wrong left jump:

const original = {
   h: 150, s: 0.6755555555555556, v: 0, a: 1, source: "hsva"
}
const color = tinycolor(original); //  {h: 0, s: 0, v: 0, a: 1}

Weird.

paulm17 commented 6 years ago

@linx4200

The PR that I created unfortunately has issues. I mentioned them in the thread. I'm not sure if you saw the message or not.

This was back in Jun 28.

Recently, I have solved all the issues. However, the codebase is different as I have introduced other functionality that I require.

I'm more than happy to put the code on github with a demo and then you can work out the differences. As I don't think I can do a simple PR now.

Let me know what you think.

I definitely want to give back. I was previously using Spectrum with jQuery and now using Vue. This component was the closest to it. It saved me a bunch of work. So I am appreciative of that! 👍

linx4200 commented 6 years ago

@paulm17

Yes, please provide a demo. Thanks for your work in advance.

u3u commented 5 years ago

image

@linx4200 Why is there no such problem with the old version? After testing, there is no such problem in v2.4.3 and below.

And the two demos are also different versions: Old version(ok): http://vue-color.surge.sh New version(wrong): https://xiaokaike.github.io/vue-color

xchen136 commented 4 years ago

FYI to others, this is the issue related to #109 and #25 previously announced and currently stated in the mixin color.js file.

pandashuai commented 3 years ago

Still exists and has not been resolved