ruhley / angular-color-picker

Vanilla AngularJS Color Picker Directive with no requirement on jQuery
http://ruhley.github.io/angular-color-picker/
MIT License
166 stars 79 forks source link

Round color picker is broken starting from 2.6.0 #127

Closed zaalbarxx closed 7 years ago

zaalbarxx commented 7 years ago

Hi! I noticed that the round color picker stopped working for me after I upgraded to v2.6.0. Color does not change at all(it stays white) after every change. I noticed that the new color calculated from tinycolor always returns rgb(255,255,255). When I changed line 511 in controller from color = tinycolor({h: this.hue, s: this.saturation, l: this.lightness}); to color = tinycolor({h: this.hue, s: this.saturation, v: 100});

it started to work again. I suppose that the round color picker should always have the lightness set to 100. Isn't that correct ? I may obviously be wrong as I don't know that much about color palettes and systems.

EDIT: it actually does not work ok since it then allows to select only the colors on the edge of the picker. The problem may be the fact that the lightness is always 100 for me. image image

ruhley commented 7 years ago

@zaalbarxx There is a new lightness option that will add a slider. If it is set to 50% then it works great for me. A lightness of 100% will always be white and a lightness of 0% will always be black.

zaalbarxx commented 7 years ago

The problem is that I don't need the lightness option. I'd just like to use the picker the same way as it was used before. So is there something I should tweak to make it work correctly ? Because I don't quite know what should I look for. Will it work if I just add the lightness: false as an option to get the previous behaviour ?

ruhley commented 7 years ago

I have fixed this by setting the default lightness to 50% for the round color picker. Released in v2.7.1

zaalbarxx commented 7 years ago

Sadly it is still does not work for me even after setting lightness: false. This is my picker options object this.colorPickerOptions = { format: 'hsv', alpha: false, lightness: false, round: true, pos: this.position, swatchOnly: true, updateBackgroundColor: false, }; Maybe it is something to do with converting from hsv to hsl, but I doubt it. After all it should work good after first color selection. If I initially pass the ngModel like hsv(0, 0%, 100%) which is white, the picker locks on lightness = 100 and never change this value. I tried to go deeper into the code, but could not figure out it after a quick look. Maybe you have any thoughts on this that would guide me for a quick PR ?

ruhley commented 7 years ago

If you want to use hsv for the initial value then it should be set to hsv(0, 0%, 50%). Otherwise you could leave it as null or undefined and it will work.

zaalbarxx commented 7 years ago

Well, it does not seem to work with either hsl or hsv as the format. I also tried to set the color: null model and it also does not pick the color correctly. Seems like the implementation was changed from 2.6.0 to 2.7.0 (using 2.7.1 now). Any clue ?

ruhley commented 7 years ago

@zaalbarxx I cannot replicate the problems you are having. Are you able to set up a https://plnkr.co/ with an example of the bug?