Alberplz / angular2-color-picker

Angular 2 Color Picker Directive, no dependences required.
MIT License
185 stars 82 forks source link

Proposal: remove this.colorPickerChange from the directive in ngOnInit() #37

Open maartentibau opened 7 years ago

maartentibau commented 7 years ago

I don't know if it's a smart thing to do. But in my project I have a service who's setting style settings on dynamic created components. But because of the this.colorPickerChange event that's fired in the ngOnInit of the color-picker.directive.ts, the default settings are directly overwritten with the settings of the color picker directive, since I myself trigger a custom event.

changeTemplateStyle (sections: Array<string>, property: string, value: string) {
  this.templateService.updateTemplateStyle(sections, property, value);
}

Whereas I think it would be better that the color picker colors aren't being changed until the color picker itself has been clicked on and a color has been selected. By removing that line, my app now runs fine and only when I open the color picker and select a new color all my other colors are updated and as my components are dynamically created.

ngOnInit() {
  var hsva = this.service.stringToHsva(this.colorPicker);
  if (hsva == null) {
    hsva = this.service.stringToHsva(this.cpFallbackColor);
  }
  this.colorPickerChange.emit(this.service.outputFormat(hsva, this.cpOutputFormat));
}
Alberplz commented 7 years ago

If the event is only fired when the color input format doesn't match with the color output format?

I can change that line by:

let output = this.service.outputFormat(hsva, this.cpOutputFormat); if (output !== this.colorPicker) { this.colorPickerChange.emit(this.service.outputFormat(hsva, this.cpOutputFormat)); }

maartentibau commented 7 years ago

@Alberplz that's a solution for components that are dynamically created, but if you use the renderer to alter some stuff on dynamic created components, then it will still use the values created by color picker instead of the pre-set colors.

Alberplz commented 7 years ago

Then a new option to control that line could be a good solution

maartentibau commented 7 years ago

@Alberplz a new option isn't needed. I did this and that works perfectly!

ngOnInit() {
  var hsva = this.service.stringToHsva(this.colorPicker);
  if (hsva == null) {
    hsva = this.service.stringToHsva(this.cpFallbackColor);
  }

  if (this.created) {
    this.colorPickerChange.emit(this.service.outputFormat(hsva, this.cpOutputFormat));
  }
}

FYI: it's better to use let than var in typescript.

Alberplz commented 7 years ago

I have not tested with components dynamically created but it's nice!! :)

Yes....yesterday I committed that var

no-more commented 7 years ago

Hi,

I think I have the same issue, change event is triggered when my components init, and so I mark my form as dirty by error. It would be great to not trigger the event on input changes or at least not on creation.

Thanks

BrainCrumbz commented 7 years ago

any update on this? Thank you

renaud-dev commented 7 years ago

Any update on this feature ? (would be great not to have to fork). Current code on master is still : https://github.com/Alberplz/angular2-color-picker/blob/master/src/color-picker.directive.ts#L64-L71 Thank you for your great work :+1:

wartab commented 7 years ago

@renaudaste angular2-color-picker is not maintained anymore. Use this instead: https://github.com/zefoy/ngx-color-picker

It is a drop in replacement for this package and should work out of the box with bug fixes and added features. Should your use case not be covered yet in it, feel free to report it there.