Empty values sorted incorrectly #185

Closed
enlait opened this Issue Dec 6, 2013 · 11 comments

Comments

Projects
None yet
6 participants
@enlait

enlait commented Dec 6, 2013

There's a regression bug, the issue was fixed in alphanumeric sort #136 but is present in natural sort algorithm used now. As a result, empty strings are always on top, whatever the sort direction.

The correct way would be not to do any string replacement at all if one of (or both) passed values are null/empty

@javve

This comment has been minimized.

Show comment
Hide comment
@javve

javve Dec 6, 2013

Owner

Oh, damn. I'll look into it!

On Fri, Dec 6, 2013 at 1:44 PM, enlait notifications@github.com wrote:

There's a regression bug, the issue was fixed in alphanumeric sort #136https://github.com/javve/list.js/pull/136but is present in natural sort algorithm used now. As a result, empty
strings are always on top, whatever the sort direction.

The correct way would be not to do any string replacement at all if one of
(or both) passed values are null/empty


Reply to this email directly or view it on GitHubhttps://github.com/javve/list.js/issues/185
.

Owner

javve commented Dec 6, 2013

Oh, damn. I'll look into it!

On Fri, Dec 6, 2013 at 1:44 PM, enlait notifications@github.com wrote:

There's a regression bug, the issue was fixed in alphanumeric sort #136https://github.com/javve/list.js/pull/136but is present in natural sort algorithm used now. As a result, empty
strings are always on top, whatever the sort direction.

The correct way would be not to do any string replacement at all if one of
(or both) passed values are null/empty


Reply to this email directly or view it on GitHubhttps://github.com/javve/list.js/issues/185
.

@adrien-be

This comment has been minimized.

Show comment
Hide comment
@adrien-be

adrien-be Jul 30, 2014

seems to work smoothly, see http://jsfiddle.net/qwtwL/

seems to work smoothly, see http://jsfiddle.net/qwtwL/

@rsl

This comment has been minimized.

Show comment
Hide comment
@rsl

rsl Jul 30, 2014

i'm having this problem as well: http://jsfiddle.net/7syUK/2/ definitely not working smoothly universally

rsl commented Jul 30, 2014

i'm having this problem as well: http://jsfiddle.net/7syUK/2/ definitely not working smoothly universally

@rsl

This comment has been minimized.

Show comment
Hide comment
@rsl

rsl Jul 30, 2014

http://jsfiddle.net/7syUK/6/ shows the difference between how it handles '', ' ', and ' '. i can work around via the non-breaking space but for most users how this natural sort handles empty strings is incorrect. the issue seems to be here: https://github.com/javve/list.js/blob/master/dist/list.js#L639-L640 but i find that js kinda inscrutable and have little clue how to fix [also it's a source you're pulling in from upstream so not sure how to submit a pull request on that anyhow]. hopefully my digging into this helps you be able to find the issue and a solution. let me know if i can help anymore.

rsl commented Jul 30, 2014

http://jsfiddle.net/7syUK/6/ shows the difference between how it handles '', ' ', and ' '. i can work around via the non-breaking space but for most users how this natural sort handles empty strings is incorrect. the issue seems to be here: https://github.com/javve/list.js/blob/master/dist/list.js#L639-L640 but i find that js kinda inscrutable and have little clue how to fix [also it's a source you're pulling in from upstream so not sure how to submit a pull request on that anyhow]. hopefully my digging into this helps you be able to find the issue and a solution. let me know if i can help anymore.

@adrien-be

This comment has been minimized.

Show comment
Hide comment
@adrien-be

adrien-be Jul 30, 2014

I just found out that this feature works... but only if the value in the TDs are numbers.
See in your example if you replace the PP1 & PP2 by numbers: http://jsfiddle.net/qAw2t/

I just found out that this feature works... but only if the value in the TDs are numbers.
See in your example if you replace the PP1 & PP2 by numbers: http://jsfiddle.net/qAw2t/

@rsl

This comment has been minimized.

Show comment
Hide comment
@rsl

rsl Jul 30, 2014

that makes sense because it does some kind of coercion of empty strings to 0 for comparisons. which causes isNaN comparisons to not be the same and returns undesired results.

rsl commented Jul 30, 2014

that makes sense because it does some kind of coercion of empty strings to 0 for comparisons. which causes isNaN comparisons to not be the same and returns undesired results.

@RiZKiT

This comment has been minimized.

Show comment
Hide comment
@RiZKiT

RiZKiT Aug 5, 2014

If there is a solution, i suggest someone should check the solution against the original unit tests in this project too: https://github.com/overset/javascript-natural-sort

RiZKiT commented Aug 5, 2014

If there is a solution, i suggest someone should check the solution against the original unit tests in this project too: https://github.com/overset/javascript-natural-sort

@adrien-be

This comment has been minimized.

Show comment
Hide comment
@adrien-be

adrien-be Aug 19, 2014

On a similar subject:

  1. when sorting cells where some cells "only have NUMBERS" & some other cells have "numbers and letters" it seems that the cells with numbers always come first.
    see http://jsfiddle.net/wvtphqas/
  2. when sorting cells where some cells "only have LETTERS " & some other cells have "numbers and letters" it seems:
    2.a if the cells with numbers & letters start by a letter: everything runs as expected, see http://jsfiddle.net/wvtphqas/
    2,b if the cells with numbers & letters start by a number: we run again in the same pb, the cells starting with a number always come first, see http://jsfiddle.net/aw21jcbg/

My understanding is that these bugs may have something in common with the "empty values sorted incorrectly" bug. Although I may be wrong.

Finally, it seems that list.js does need some clean up on those main functionnalities.
In the end, as much as I like list.js, why would one use list.js knowing there is such as silly bug ?
I will try to help out at commiting some fix.

On a similar subject:

  1. when sorting cells where some cells "only have NUMBERS" & some other cells have "numbers and letters" it seems that the cells with numbers always come first.
    see http://jsfiddle.net/wvtphqas/
  2. when sorting cells where some cells "only have LETTERS " & some other cells have "numbers and letters" it seems:
    2.a if the cells with numbers & letters start by a letter: everything runs as expected, see http://jsfiddle.net/wvtphqas/
    2,b if the cells with numbers & letters start by a number: we run again in the same pb, the cells starting with a number always come first, see http://jsfiddle.net/aw21jcbg/

My understanding is that these bugs may have something in common with the "empty values sorted incorrectly" bug. Although I may be wrong.

Finally, it seems that list.js does need some clean up on those main functionnalities.
In the end, as much as I like list.js, why would one use list.js knowing there is such as silly bug ?
I will try to help out at commiting some fix.

@adrien-be adrien-be referenced this issue in javve/natural-sort Sep 26, 2014

Open

Sorting incorrect when there is a space #7

@adrien-be

This comment has been minimized.

Show comment
Hide comment
@adrien-be

adrien-be Sep 26, 2014

As pointed out above, this issue is probably coming from the "natural sort" algorithm, https://github.com/javve/natural-sort

It is used here https://github.com/javve/list.js/blob/master/dist/list.js#L607 (line 607 to 652).

As pointed out above, this issue is probably coming from the "natural sort" algorithm, https://github.com/javve/natural-sort

It is used here https://github.com/javve/list.js/blob/master/dist/list.js#L607 (line 607 to 652).

@anonymoushenchman

This comment has been minimized.

Show comment
Hide comment
@anonymoushenchman

anonymoushenchman Nov 12, 2014

I'm all fresh to web dev, but it looks like this hasn't been fixed yet.
A workaround I found was to append a zero-width-non-joiner (‌) to the text that was being searched, e.g.:

<td class="firstname" ><c:out value="${firstname.value}" />&zwnj;</td>

I'm all fresh to web dev, but it looks like this hasn't been fixed yet.
A workaround I found was to append a zero-width-non-joiner (&zwnj;) to the text that was being searched, e.g.:

<td class="firstname" ><c:out value="${firstname.value}" />&zwnj;</td>

@javve

This comment has been minimized.

Show comment
Hide comment
@javve

javve Feb 26, 2016

Owner

I've just created an issue where I reference all sort related problems. See #385. Thanks!

Owner

javve commented Feb 26, 2016

I've just created an issue where I reference all sort related problems. See #385. Thanks!

@javve javve closed this Feb 26, 2016

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