Added option for round picker

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

Added option for round picker #93

Closed zaalbarxx closed 8 years ago

zaalbarxx commented 8 years ago

Hi! I thought maybe you will be interested in something me and my friend created in our fork. Basically this is the option to create just the round color picker without lightness and alpha channels. We are using it internally in our project, but maybe you would want to pick something from there and apply it to the main project. If showHue and alpha are set to false then only round picker show up which works exacly like yours but have lightness and alpha always set to 100 and 1. I only added maybe two options and new background image with reversed color palette, also I just merged your latest changes so it is up to date. It looks like this.

ruhley commented 8 years ago

Looks great, but just a few things for this PR as it is quite a large change:

Can you submit the PR without upgrading the version and building the dist folder
I have added a hue option in v2.2.0
Make round a passable option
this.updateBackgroundColor should be this.options.updateBackgroundColor
The class color-picker-panel--round can be color-picker-panel-round and instead of adding it in the controller you can add it in the template using ng-class
this.$scope.$apply(); after the close call has already been added

Otherwise I love this feature and would be very happy to add it into the base library

ruhley commented 8 years ago

Also merging PR with lots of commits is generally considered bad practice. If you can put it all into one commit or squash the commits - https://github.com/ginatrapani/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit

zaalbarxx commented 8 years ago

Not sure if done it right, but I fixed the things you pointed out above. Take a look and tell me what do you think.

ruhley commented 8 years ago

This has been released in v2.3.0. I added some improvements like allowing the alpha control and restricting pointer positions to inside the circle. Thanks for the great PR :+1:

skalb commented 8 years ago

It looks like options.updateBackgroundColor is required to be set to true now otherwise the background color of the color-picker-sprite is never updated.

Not sure if this intended, but if so, might be useful to add to the docs.

ruhley commented 8 years ago

I also removed the requirement for updateBackgroundColor and handle that using css. Thanks for reviewing @skalb :+1:

skalb commented 8 years ago

Whoops, sorry I didn't see the latest release. Thanks for the quick response!