Fix decimal and negative range #78

Open
wants to merge 4 commits into
from

Conversation

Projects
None yet
7 participants

Aeon commented Mar 12, 2013

Fix two issues

  1. knobs break with step is a decimal, due to using ~~ for rounding. Switched to using Math.floor and Math.round instead.
  2. knobs break when the range is from negative number to positive number - check for max value when it is 0 by negating it always evaluates to true, so it always gets reset to default max instead of what user specified. Replaced with check for undefined instead.

Thanks for the great library!

Aeon commented Mar 20, 2013

Hi @aterrien, anything you need me to change for this to get accepted? I think these are useful fixes.

Owner

aterrien commented Mar 20, 2013

Hi Aeon, first, thank you for this patch.
It don't work correctly for me with keyup/down on the input field.
Tested with data-step=.2 and data-value=9.5
(I think we can write a function for the parseFloat rounding)

Aeon commented Mar 27, 2013

@aterrien ok, I will try to debug and find the issue, will send you update when I figure it out.

Aeon commented Apr 3, 2013

Hi @aterrien

I fixed the keyboard navigation issue for decimal range, can you take a look now and try again?

Aeon commented May 7, 2013

hi @aterrien, any feedback?

+1 to getting this fixed

Joey11 commented Jul 12, 2013

Nice addition! One little problem, though: keyboard-input. When entering a value, the decimal sign (dot) is not allowed

Aeon commented Sep 11, 2013

@aterrien, any changes you wish me to make to get this merged?

Aeon commented Nov 5, 2013

rebased off current master to make sure it merges cleanly. Let me know if anything is an issue.

Why isn't decimal support added on master?

Aeon commented Jan 23, 2014

I don't know; I guess @aterrien hasn't had time to review it.

Hi Aeon,

How would you go about adding support for decimal input? I.e. allow the "." key

Aeon commented Jan 23, 2014

Rebased off master for clean merge.

@waissbluth: Added decimal point support in text field input in fe24551

I think there's still issues with negative ranges though, specifically the arrow key increment/decrement doesn't work quite right. I'll see if I can track it down soon.

@aterrien aterrien added a commit that referenced this pull request Jan 23, 2014

@aterrien aterrien Fix decimal values #78 @Aaon 030fe57
Owner

aterrien commented Jan 23, 2014

Merged from your patch Aeon + had some new issues, let me know if it work as expected.

Aeon commented Jan 23, 2014

@aterrien: i don't think you merged whole branch? I don't see rest of commits merged in?

Aeon commented Jan 24, 2014

@aterrien: the decimal input commit is not sufficient, you really need the other commits for the decimal range to work.

This is great but it still seems to display imprecise decimal values like 12.1000000000001 in the middle of the dial, i.e. the "decimal problem."

breem commented Jul 28, 2014

I got the latest version of jquery.knob.js and merged in the not already added changes from the four commits from Aeon. All is working except a mouseclick on the dial. This returns a "NaN" error inside the dial. Where can I find this event and correct this behaviour?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment