search

bernii / gauge.js

100% native and cool looking JavaScript gauge
MIT License
1.42k stars 391 forks source link

add option 'limitMin' and handle max/min-hit value #110

Closed hangy233 closed 7 years ago

hangy233 commented 7 years ago

Hi,

I'm a little confused about this line in gauge.coffee. I guess this is causing issue #109. It seems increasing the maxValue when the pointer value hits the maxValue. (So the pointer never goes out of the gauge?) Assuming this is true, I made some changes to fix #109 and add options "limitMin" (Shall we call them "fixMin/Max" instead of "limit"?) Please check and let me know if I'm wrong.

Thanks

kplindegaard commented 7 years ago

I don't know the story behind the line you refer to, increasing the max value with 10% if the max value is exceeded. Be that as it may, but I am in favor of being able to not only have a limitMax but also a limitMin option as you suggest. To not mess up backwards compatibility, we should use the prefix "limit" and not "fix".

hangy233 commented 7 years ago

Totally agreed. My code basically does the same thing. I changed @value to val, because @value is the previous gauge value (this is causing the issue I referred). Increasing the max value with 10% is also potentially dangerous if there is any negative value. So I changed it to @maxValue = val + 1 Could you double check the code and merge the pull request if they looks OK to you?

kplindegaard commented 7 years ago

@johanye I started to play a bit with your fix in this JSFiddle. It includes gauge.min.js from your gh-pages branch. However, I have some difficulties getting the limitMin-option to work properly. If I enable it, the gauge will not render at all.

hangy233 commented 7 years ago

Thanks for pointing out @kplindegaard . I think this is because the flag 'max_hit', It always executes AnimationUpdater.run(); See https://github.com/bernii/gauge.js/blob/gh-pages/dist/gauge.js#L511-L533 I did it wrong in my code. Please try again at your fiddle

kplindegaard commented 7 years ago

@johanye: It's looking pretty good now, thanks for having a second go at it. I have modified the original JSFiddle a bit where the pointer now tries to exceed both the max and the min value before settling back in at the mean value 150. It looks very nice :+1:

Sorry for messing up your pull request by merging in a variety of stuff and changes. Do you mind fetching from upstream and merge recent changes into your fork before resubmitting the pull request?

hangy233 commented 7 years ago

The demo looks nice! Thanks for merging @kplindegaard!