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

Is this a bug or intended behavior? #149

Closed schester44 closed 7 years ago

schester44 commented 7 years ago

I was having trouble with a form validating. I could not get it to validate to save my life ($ctrl.form.$invalid was returning true no matter what).

It turns out that the reason it was happening was because I was initializing an ngModel variable with an empty string then using it within the <color-picker ng-model="", which for some reason, caused the form to give me an error of "undefined" .. Maybe i'm not understanding the way ngForm works, maybe this could be a possible bug, i'm not sure, but I wanted to share my observations.

So...

  1. initialize an ngModel with an empty string "" (ex: this.formModel.color1 = "" )
  2. then apply that model to the color-picker <color-picker ng-model="$ctrl.formModel.color1" ...
  3. the form never validates..

I was getting this error :

{
  "$error": {
    "undefined": [
      {
        "$viewValue": "",
        "$modelValue": "",
        "$validators": {},
        "$asyncValidators": {},
        "$parsers": [],
        "$formatters": [],
        "$viewChangeListeners": [],
        "$untouched": true,
        "$touched": false,
        "$pristine": true,
        "$dirty": false,
        "$valid": false,
        "$invalid": true,
        "$error": {
          "undefined": true
        },
        "$name": "",
        "$options": {}
      }
    ]
  }

here is the code in question...inside of the array.push, changing the empty string to an rgba() value causes the form to validate no questions.

  let colors = ['bg_color', 'color_1', 'color_2', 'color_3', 'color_4']; // list of color columns from the DB

    colors.forEach((element, index) => {
      // setup the _colors model and give them some random values
      this.formModel._colors.push({name: element, value: (element === 'bg_color') ? '' : 'rgba(0,' + 20 * index + ',' + 20 * index + ', 1.0)'});

      // color picker options
      this.colorPickerOptions[index] = {
        display_name: (element === 'bg_color') ? 'Background Overlay' : element.replace('_', ' ').capitalizeFirstLetter(),
        options: {
          placeholder: 'rgba(1,2,3,0.0)',
          id: element,
          name: element,
          pos: 'bottom right',
          format: 'rgb',
          restrictToFormat: true,
          close: {
            show: true
          },
          lightness: true
        }
      };
    });
schester44 commented 7 years ago

I'm going to add another thing here to de-clutter the issues page and hopefully/maybe/possibly find a fix for some issues i'm having with the color picker..

I was setting the model value to an empty string because I wanted to display a transparent color picker (like the left-most color picker in is example http://i.imgur.com/XJDKGWh.png)

This works fine, even if I replace the empty string with say rgba(0,0,0,0.0) HOWEVER, when you open the color picker, it looks like this (http://i.imgur.com/taS9JEW.png), which is super confusing from a UI/UX perspective. It would be nice if when you open it, it looked sort've like this (http://i.imgur.com/GL2UgH7.png) but the far right slider was set to transparent. Is this at all possible to accomplish?

ruhley commented 7 years ago

Your second issue has been fixed in commit 5d86d0e8422e362855f2d753f4b53724039b1db9

ruhley commented 7 years ago

As for your first issue tinycolor doesn't like an empty string, empty object or null (as they are not valid colors).

This line calculates the validity - https://github.com/ruhley/angular-color-picker/blob/master/src/scripts/controller.js#L44 And then it is applied to angularjs a little lower - https://github.com/ruhley/angular-color-picker/blob/master/src/scripts/controller.js#L71

I think the best solution may be to add another configuration option to allow empty values to be valid.

What are your thoughts?

schester44 commented 7 years ago

I'd say empty strings as not valid would be the expected behavior though a fail safe wouldn't hurt. I haven't had a chance to check out the commit yet but I appreciate you taking the time to point out the sections of code and implementing a fix.

ruhley commented 7 years ago

I have made a new release of v3.2.0. I have also fixed the undefined problem you were having earlier.