ruhley / angular-color-picker

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

GRB/HEX/HEX8 mode: hue position updates when selecting color from palette #88

Closed afilenko closed 8 years ago

afilenko commented 8 years ago

When selecting a color from palette in RGB mode by moving pressed mouse in circles over the palette - hue slider position also updates, which causes palette color to be changed as well. In result when user moves pressed mouse over the palette it's color is changing slightly during that operation.

This might be caused by different color formats used in colorpicker directive settings and in watchNgModel method, where, according to color value from ngModel, hue and saturation is calculated and after that huePos value is also updated. Method call sequence in this case might look as follows:

  1. onMouseMove when colorMouse is true
  2. colorChange method updates saturation and lightness
  3. watchNgModel is called and hue, saturation, lightness updated again according to HSV color calculated from current ng-model value.
  4. hueUpdate is called where huePos is updated using the value calculated in prev step
  5. huePosUpdate method called and .color-picker-slider css.top is set
  6. Color palette is changed after hue update, which was not caused by corresponding user action (using the hue slider), but after calculations made on color changed in palette.

This issue is not observed when HSL/HSV color format is set in colorpicker settings and as I mentioned above, might be caused by differences in color representation between RGB and HSL. So after trying to calc hue from RGB color by converting it to HSV in watchNgModel method some rounding errors might occur or stuff like that (I'm not experienced in color formats representation).

Also there is a bug when moving .color-picker-picker element to bottom or left (mouse x <= left border of palette or mouse y >= bottom border) : in this case hue slider jumps to bottom. This issue is reproducible for all formats.

I'd suggest to check for colorMouse flag in watchNgModel and continue method execution only if this flag is false. In my case such solution (workaround maybe) worked:

... value: function watchNgModel(newValue, oldValue) { var _this2 = this; if (this.colorMouse) { return; } ...

For bidirectional data binding when user changes color numeric values in corresponding input - everything works as it should - palette color and hue positions are updated.

ruhley commented 8 years ago

It is a rounding problem as HSL/HSV and RGB/HEX aren't really compatible. This was something that always bothered me, so thanks for finding a great solution :+1:

The data binding and events are still triggered so I am happy with implementing this change.

Fixed in commit 04c4910fd659c0ebb30bf11cb2bd3f881f32734f and released in v2.1.1

afilenko commented 8 years ago

glad, i could help. a very nice code, btw, thanks!)