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

It should be converted to number first before sort #38

Merged
merged 4 commits into from
Nov 23, 2017
Merged

It should be converted to number first before sort #38

merged 4 commits into from
Nov 23, 2017

Conversation

donghopark
Copy link
Contributor

No description provided.

@donghopark
Copy link
Contributor Author

Hi. Ashish. Your plugin is really nice. Thanks a lot. I'm using this plugin and found one bug. When I set thresholds with various values, it just didn't recognize well. Please see my pull request.

@ashish-chopra
Copy link
Owner

Thanks @donghopark for using it. I am glad that you found it useful.
I saw your modifications in the pull request. But can you please explain and write some cases which you tried so that i can reproduce the issue at my side?

Thanks
Ashish

@donghopark
Copy link
Contributor Author

You may try with below configuration

$scope.gaugeThreshold = {
'0': {color: '#228B22'},
'8': {color: "#AABC22"},
'21': {color: '#FFF54C'},
'31': {color: '#DF9111'},
'41': {color: '#FF0000'},
}

$scope.value = 22;

It should be third option to be selected, but once it go over 8, it always select the second option.

@ashish-chopra
Copy link
Owner

#46

@ashish-chopra
Copy link
Owner

Hi @donghopark

I tested your code, its working fine, with one simple change that you need to push in your pull request, so that build will pass.

change

.sort((a,b) => {Number(a) > Number(b)})

to

.sort(function(a,b) {return Number(a) > Number(b);}

@donghopark
Copy link
Contributor Author

Re-commited as you recommend

@ashish-chopra ashish-chopra merged commit 46a171b into ashish-chopra:master Nov 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants