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

Not sorting negative numbers correctly. Reproduced on listjs.com #562

Open
ghost opened this issue Nov 4, 2017 · 9 comments · May be fixed by #779
Open

Not sorting negative numbers correctly. Reproduced on listjs.com #562

ghost opened this issue Nov 4, 2017 · 9 comments · May be fixed by #779

Comments

@ghost
Copy link

ghost commented Nov 4, 2017

Hello, listjs is not sorting negative numbers correctly. You can reproduce the code on your own website. Take a look at: http://listjs.com/examples/add-get-remove/

Click "Age" column. As you can see it goes from -23 -> -132 -> 26.

This is wrong, since -132 is smaller than -23, it should be going -132 -> -23 -> 26, etc.

@r3faat1
Copy link

r3faat1 commented Nov 14, 2017

I've changed account. This is my account now. Issue is still present.

@r3faat1
Copy link

r3faat1 commented Nov 23, 2017

Hello, I've figured this out. I resolved the problem by creating my own sort function and telling ListJS to use that. Below is the code that works for ME, but may not for you. So modify it as necessary.

        function listJSCustomNumericSort(a, b)
        {
            var before = parseFloat(a._values.profit.replace(/[^0-9.-]/g, ""));
            var after = parseFloat(b._values.profit.replace(/[^0-9.-]/g, ""));

            if (before > after) { return 1; }
            if (before < after) { return -1; }
            else { return 0; }
        }

        tableLJS.sort('profit', { sortFunction: lisJSCustomNumericSort, order: sortOrder });

Feel free to close this out, I wasn't sure if anybody else wanted to look at it first. Thanks.

@Vemb
Copy link

Vemb commented Sep 9, 2022

@r3faat1

I am having this exact problem in 2022, and can't get your code to work. Could you help?

In the code above, sortOrder is not defined. Do you simply replace it with "asc" or "desc"? And why would you want to define this at all? The user needs to be able to choose either asc or desc, and does this not fix the sort order?

What does "return 1", "return -1" and "return 0" mean? Are you just using "return 1" for "return: true"? I have never seen "return -1" before, so don't understand this.

Finally, the values below are what I console.log out from "before" and "after" when running the code on a table with these values: [1.71, -3.95, -2.09, 3.39, 12.36, -1.28]. Any idea what's happening:

before: NaN
after: 1.71
before: -3.95
after: NaN
before: NaN
after: -3.95
before: -2.09
after: NaN
before: NaN
after: -2.09
before: 3.39
after: NaN
before: NaN
after: 3.39
before: 12.36
after: NaN
before: NaN
after: 12.36
before: -1.28
after: NaN
before: NaN
after: -1.28

Would very much appreciate some pointers. Cheers.

@BadScooter1980
Copy link

@r3faat1

I am having this exact problem in 2022, and can't get your code to work. Could you help?

In the code above, sortOrder is not defined. Do you simply replace it with "asc" or "desc"? And why would you want to define this at all? The user needs to be able to choose either asc or desc, and does this not fix the sort order?

What does "return 1", "return -1" and "return 0" mean? Are you just using "return 1" for "return: true"? I have never seen "return -1" before, so don't understand this.

Finally, the values below are what I console.log out from "before" and "after" when running the code on a table with these values: [1.71, -3.95, -2.09, 3.39, 12.36, -1.28]. Any idea what's happening:

before: NaN after: 1.71 before: -3.95 after: NaN before: NaN after: -3.95 before: -2.09 after: NaN before: NaN after: -2.09 before: 3.39 after: NaN before: NaN after: 3.39 before: 12.36 after: NaN before: NaN after: 12.36 before: -1.28 after: NaN before: NaN after: -1.28

Would very much appreciate some pointers. Cheers.

Did you manage to figure this out?

@kaylamw
Copy link

kaylamw commented Aug 4, 2023

Sorry about the late reply @Vemb, I did not see your issue until now.

  1. I can't remember what the actual values of the sortOrder was but it could be "asc" or "desc". You don't hard code it, you assign the value onClick event based on which table header the user clicked for sorting (and their order).

  2. Take a look at this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort (compareFn), it basically compares which value should go before which value. The 3 different compares are < 0, > 0, and ===.

  3. I won't be able to figure out without seeing your code. Feel free to post it if you're still having issues.

@jdevalk
Copy link

jdevalk commented Jul 5, 2024

I'm experiencing this same issue on my CMS market share report; positive values get sorted properly when in descending order (300 - 200 - etc.), but when sorting in ascending order, -0.5% comes higher than -20%, which is not what it should be.

Feel free to test it out on my site, you can see all the code there too.

@vanboom
Copy link

vanboom commented Oct 3, 2024

Here is another simple codepen which shows the sorting issue with negative numbers.
https://codepen.io/vanboom/pen/poMyayR

@vanboom
Copy link

vanboom commented Oct 3, 2024

Something like this to detect if we are comparing two numeric values maybe? I think the naturalCompare function is imported from string-natural-compare so possibly we should focus the fix there? Is List.js maintained? This issue is 7 years old?

function naturalCompare(a, b) {
  var lengthA = (a += '').length;
  var lengthB = (b += '').length;
  var aIndex = 0;
  var bIndex = 0;

  // if we are comparing two numbers
  if ( a != "" && b != "" && !isNaN(a) && !isNaN(b))
  {
    return Math.sign(parseFloat(a) - parseFloat(b));
  }

  while (aIndex < lengthA && bIndex < lengthB) {
    var charCodeA = a.charCodeAt(aIndex);
    var charCodeB = b.charCodeAt(bIndex);
....

@nwoltman
Copy link

nwoltman commented Oct 4, 2024

I've submitted a PR that I think should fix this issue: #779
I haven't tested it though, so it would be nice if someone here could try out the fix and see if it works.

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 a pull request may close this issue.

7 participants