Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when calling slider.noUiSlider.get() the values returned are an array of strings not numbers #813

Closed
jdedwards3 opened this issue Aug 7, 2017 · 12 comments
Milestone

Comments

@jdedwards3
Copy link

@jdedwards3 jdedwards3 commented Aug 7, 2017

I'm not sure if this is a bug, but I am using typescript with a definition file for noUiSlider gathered from the definitely typed repository.

This definition file indicates the type of the value returned from slider.noUislider.get() is a number[]. However it is actually returning a string[].

Shouldn't it be numbers?

@leongersen
Copy link
Owner

@leongersen leongersen commented Dec 29, 2017

It's not explicit in the documentation and I'll update that, but the return type is definitely supposed to be string, as it may include formatting.

@leongersen leongersen added this to the noUiSlider 11 milestone Dec 30, 2017
@leongersen
Copy link
Owner

@leongersen leongersen commented Jan 12, 2018

Updated the docs. Thanks for reporting!

@leongersen leongersen closed this Jan 12, 2018
andy-ms added a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Jan 23, 2018
see this related issue that is now closed: 
leongersen/noUiSlider#813

When calling get type should be strings
@JuusoPalander
Copy link

@JuusoPalander JuusoPalander commented Feb 2, 2018

If get() should return a string or arrays of strings, how is this possible:
screen shot 2018-02-02 at 15 48 05

I'm using nouislider version 11.0.3 and @types/nouislider version 9.0.4 and had some tests failing because of the types change.

Just wondering if the types are correct or not?

@leongersen leongersen added Bug and removed Documentation labels Feb 2, 2018
@leongersen leongersen reopened this Feb 2, 2018
@jdedwards3
Copy link
Author

@jdedwards3 jdedwards3 commented Feb 2, 2018

It is definitely returning a string. I'm not sure how the above is possible. This is not a bug.

@leongersen
Copy link
Owner

@leongersen leongersen commented Feb 2, 2018

Are you using a format option that returns a Number?

@JuusoPalander
Copy link

@JuusoPalander JuusoPalander commented Feb 2, 2018

Yup that was the case, my bad!

@leongersen leongersen added Documentation and removed Bug labels Feb 19, 2018
@leongersen
Copy link
Owner

@leongersen leongersen commented Feb 19, 2018

I think I'll leave that like it is, but it'll have to be in the docs.

@leongersen
Copy link
Owner

@leongersen leongersen commented Apr 2, 2018

Docs updated!

@leongersen leongersen closed this Apr 2, 2018
@jdedwards3
Copy link
Author

@jdedwards3 jdedwards3 commented May 9, 2018

@leongersen Just wondering if the get type def indicates it returns strings, should the set type def accept strings as params? the definition file in the definitely typed repo version 9.0.0 shows that set accepts a number array.

Can you clarify? Thanks

@leongersen
Copy link
Owner

@leongersen leongersen commented May 13, 2018

@jimwards17 it should; it accepts strings and uses the format option to parse those to numbers.

@ivanbulanov
Copy link

@ivanbulanov ivanbulanov commented Mar 28, 2019

noUiSlider should return an original raw value instead of a formatted string because valuable information gets lost during formatting and a user can easily format the raw value. For example I use the slider to select a range of timestamps. The values are formatted as "month-day hour". The information about year is lost and I need it to rebuild the timestamp.

@leongersen
Copy link
Owner

@leongersen leongersen commented Mar 28, 2019

@ivanbulanov It will return any value your format option returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants