Add a getSelectionBarColor options #197
Conversation
ValentinH
changed the title
getSelectionBarColor options
ValentinH
force-pushed
the
selection-bars-colors
branch
from
484a6ec to
512c91f
Compare
|
This is very good, but I kind of want it on the other side. I want to convey that choosing a number higher than a defined amount is bad. In other words, I have a slider that goes from 1-14. Once I slide past 10, I want things from 11-14 to turn red. |
|
Also, do we want to add the ability to allow the tick values to be able to turn colors as well or always just keep them blue? |
| @@ -868,6 +870,10 @@ | |||
| updateSelectionBar: function() { | |||
| this.setDimension(this.selBar, Math.abs(this.maxH.rzsp - this.minH.rzsp) + this.handleHalfDim); | |||
| this.setPosition(this.selBar, this.range ? this.minH.rzsp + this.handleHalfDim : 0); | |||
| if(this.options.getSelectionBarColor) { | |||
|
Thanks for your feedback. Indeed, I forgot to implement colors for ticks, now it is done. About DOM manipulation and not using ng-class, what do you mean. I have tried using ng-style to apply the color but then we are dependant of angular $digest cycle and I got some weird behavior where the color was not updated. This is the same for the ticks, since they were already constructed "manually", I add a similar manual method the color them. |
|
Ok, I think maybe I'm thinking about the UX for this incorrectly, but what I'd like is really just to make the line between 11-14 red at all times. I'd like to signify that moving the slider into that range is 'bad'. Maybe 11-14 turns darker red if the slider is actually in that range. Yea, I think you might get better performance out of angular if you use it as it is intended and not use .html() to push stuff into the DOM. |
|
If you want 11-14 to always be red then you can just set the slider bar to red using css (not the selection bar). For the ticks part, it's true that it has become quite dirty but my logic was not to add any useless stuff for people not using the ticks (or the tooltip) . If I do it the angular way, I would need to add a ng-repeat for displaying the ticks but I think I'll try to do this because it would be cleaner and easier to maintain. As this is not directly to this PR, I'll do it outside of it. |
ValentinH
force-pushed
the
selection-bars-colors
branch
2 times, most recently
from
c61fbd4 to
48582a8
Compare
|
@lookfirst I have reimplemented the ticks mecanism using ng-repeat, ng-style and ng-class and it almost work. The only issue is the one I was afraid of: synchronisation. In the JSFiddle example, if you move the slider quickly the ticks sometimes don't get the correct color whereas the color specified on the object is correct. |
|
@ValentinH Looking better.... =) It still appears as though you are trying to do one thing with .css() and another with ng-style? |
|
The code looks better but the behavior is not good unfortunately. I know that doing everything with .css() and .html() is not the Angular way but at least we are sure that it works, it uses less $watch internally and we don't depend on the $digest cycle. I didn't write this library at the beginning and when I started to modify it, it was also feeling weird to me to do all that work manually but now I understand that it was not due to a lack of Angular knowledge. The codebase of this slider almost only uses Angular specific stuff to interact with the outside world. I think this is actually good because it should be easier to upgrade to Angular 2 or even reuse it with another framework. |
|
Shrug. =) Up to you man. |
|
Do you see the issue on the JSFiddle example? Here's what I get: |
|
Yup, I totally see the issue. As I said above, I think that sync issue is related to you using .css() and ngstyle. Pick one, not both. =) |
|
I just tried locally to only use ngStyle to handle colors and it's worse: sometimes the color is not updated whereas the model is. You can see on the gif below, that when I change the colored slider, then another one, the first one only gets the other color when the second is modified. This shows that there is a $digest problem...
|
|
OK there is a deeper problem about synchronisation of model update and color update. I'll look at it better tomorrow. |
|
Wow, I can't imagine how that is happening. Each slider should be separate from the other slider and there should not be shared state between them. |
|
Don't worry, they are separated but the angular $digest cycle is common to the whole application. When I stop dragging the first slider, the scope value is 9 but it is still green because the scope value was still 10 when the getSelectionBarColor was called. Then when I moved the other one, it starts a new $digest and the getSelectionBarColor of the first one is called because its scope has changed compared to the last cycle. The internal flow is quite complicated actually and it might need some.simplification. |
…n't depend on Angular synchronisation.
ValentinH
force-pushed
the
selection-bars-colors
branch
from
48582a8 to
bddea44
Compare
|
Ok now it should be fine. I haven't refactor the internal flow yet (this is not only related to this PR) but now the current values are passed to the getSelectionBarColor function so we are sure we are testing the latest one. I have updated the example to show this: http://jsfiddle.net/mtm2xhwx/ |
|
👍 nice work. |
|
I wonder if there is a way to apply css3 animations to make things a bit smoother... right now it feels like the slider just jumps between ticks... I wish it could slide more... |
|
Actually if you uncomment those lines: https://github.com/rzajac/angularjs-slider/blob/master/src/rzslider.less#L79, it might do the job. I just remember that there was some wrong stuff with it but I haven't tried it for a while. |
|
Here's an example: http://jsfiddle.net/gc3hgf0y/ . Also transition doesn't work for the ticks because they are replaced at every frame by ng-repeat... |
|
My last commit fixes the ticks issue by using a track by in ng-repeat so the dom element are just updated. |
ValentinH
force-pushed
the
selection-bars-colors
branch
from
a265278 to
012ece9
Compare
|
Yea... I just came here to tell you that... ;-) |
|
This is much cooler, but it is a little bit weird now because it follows the mouse a bit when dragging... |
|
Yes, this is why it is not added by default. But setting a very small value like 0.1s is quite cool actually. |
|
Works for me. Nice job.
|
Add a getSelectionBarColor options
Add a
getSelectionBarColoroptions to dynamically change the selection bar color. As discussed in #173.Demo: http://jsfiddle.net/mtm2xhwx/