search

HubSpot / sortable

Drop-in script to make tables sortable
http://github.hubspot.com/sortable/docs/welcome
MIT License
1.33k stars 115 forks source link

Cannot order alphabetically #24

Closed felds closed 9 years ago

felds commented 9 years ago

Hi there!

This happens when I try to order a string list alphabetically:

screenshot 2015-08-27 02 40 08 screenshot 2015-08-27 02 40 19

It only happens on Chrome (Version 44.0.2403.157 @ Mac OS 10.10). The script works fine on Safari and Firefox.

adamschwartz commented 9 years ago

I’m fairly confident the issue is that Date.parse('1&1') returns the current date in Chrome whereas Firefox returns NaN. This is the imperfect criteria we use to decide if a column contains “date” data. We could introduce an attribute data-sortable-type which you could add to the th to force a specific data-type to Sortable, with current options numeric, alpha and date. For the time being though if you ensure that the entry in the first row of the “Host” column (call it foo) have a value such that Date.parse(foo) returns NaN, you should see this issue resolve. Hope this helps!

adamschwartz commented 9 years ago

Turns out someone already suggested this in https://github.com/HubSpot/sortable/issues/21. This should be closed as a duplicate.

adamschwartz commented 9 years ago

(I’d close it myself but no longer have admin privileges for some reason.)

@timmfin @geekjuice @TrevorBurnham @graysky a little help here?

adamschwartz commented 9 years ago

Thanks @timmfin

felds commented 9 years ago

@adamschwartz I'm working on an implementation that allows selecting the sort order via data-* attributes and also allows the user to add custom types and/or change how the types work. It has been pretty hard not to break BC and to keep the small footprint at the same time, but I think I'm getting there!

adamschwartz commented 9 years ago

@felds Thanks, cool idea. Since window.Sortable.types and window.Sortable.getColumnType are already exposed, you could probably modify getColumnType slightly and you’d be pretty close. Hope this helps.

felds commented 9 years ago

@adamschwartz I made so that each type test the data to see if it matches its definition. (https://github.com/felds/sortable/commit/f28945f010c72ad6dba413ee88d84649b24a4ddc)

It works, but it's not that easy defining new types, as it's not an object, but an array (so pushing a new type would append it to the list, after the default one). Converting the array to an obj flips the problem: we cannot trust the obj order when iterating through its keys and the default type could come first on the loop.

I think a solution could be adding a "priority" key to the type obj, converting the types obj into an array before looping and then sorting by that property. Not that elegant, but I think that would work!

Any ideas?

adamschwartz commented 9 years ago

Great idea! I think the array option is slightly better. See my couple comments, but I can’t wait to see the PR! Nice job. And thanks! :+1:

jcolemorrison commented 8 years ago

Ran into this issue (that as mentioned above only occurs in chrome) due to it thinking any number is with a string is a valid date. Has anyone found a way, other than data-sortable-types in getting around Chrome's different way of dealing with Date.parse / new Date's?