feat(pointerColor): Adding feature for dynamic pointer color based on…

[BlackIPS]

Adding feature for dynamic pointer color based on the value.

Changing the color of the pointer of a slider based on the value. Just pass a function to the 'pointerColor' option which returns a valid CSS color definition (either hex or named) and let the pointer shine in the brightest colors

Demo: http://jsfiddle.net/as1r9h44/

[codecov-io]

Current coverage is 100.00%

Merging #253 into master will not affect coverage as of 5ca704e

@@            master    #253   diff @@
======================================
  Files            1       1       
  Stmts          632     641     +9
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            632     641     +9
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of 5ca704e

Powered by Codecov. Updated on successful CI builds.

[mtucciarone]

Looks very close to what I was working towards but you did it better which is awesome to see! Thanks for taking this over BlackIPS, made props

[ValentinH]

Thanks for contributing, this looks clean. I have added a demo to your PR description.

Just a few remarks:

• to be coherent with the getSelectionBarColor and to explicit that it expects a function, I would prefer you rename the option from pointerColor to getPointerColor.
• in case of range slider, it would be cool to be able to set a different color for the min and max handles. In my opinion, a good way would be to pass a flag indicating that we are getting the min or the max value:

vm.slider = {
    minVal: 100,
    maxVal: 400,
    options: {
      floor: 0,
      ceil: 500,
      pointerColor: function(val, pointerType) { // pointerType is 'min' or 'max'
        if(pointerType === 'min')
           return MinColor(val);
        else
          return MaxColor(val);
      }
    }
  }

• you didn't add your code is the correct part of the source (link to function). Indeed, this function is responsible of updating the selection bar. The place for such a feature is in this function for the min value and in this one for the max value of range.
• last but not least, your PR is not covering the new feature with unit tests (see https://github.com/angular-slider/angularjs-slider/blob/master/CONTRIBUTING.md).

[BlackIPS]

Thanks for feedback guys!

I'd pushed my changes to my branch and updated the demo to match the new option naming and added a more "colorful" example for the range slider.

All suggestions by @ValentinH has been implemented except for unit tests. I'm sorry but I couldn't get into it right now but will do later this day.

Please have a look at http://jsfiddle.net/as1r9h44/1/ for the updated demo.

[ValentinH]

Seems good! Actually I noticed that the option definition for getSelectionBarColor is wrong: https://github.com/angular-slider/angularjs-slider/blob/master/src/rzslider.js#L55 . I'll need to change it.

[BlackIPS]

Hey @ValentinH,

I have added some unit tests and verified that they work as expected. I would be happy if you could review them. If you approve these changes provide some informations for further merging please. I would be happy if it make it in your package soon :)

All the best!

[ValentinH]

👍 Looks good to me: coverage is now 100% but can't be seen here because there are merge conflicts.
Thus, so I can merge, you just need to rebase your branch on master and squash all the commits into one (the first one which has the correct description). This is basically the point 13 in https://github.com/angular-slider/angularjs-slider/blob/master/CONTRIBUTING.md.

[BlackIPS]

Thanks for approval!

I'm done squashing my changes and perform a rebase against your latest version 2.7.1. Please check it and hopefully merge it. :)

Thanks for your help!

[ValentinH]

Merged and released under 2.8.0. Thanks for contributing ;)