Recent changes/improvements on my fork

[svivian]

I have made several changes to my fork of stupid-tables and attempted 2 methods of implementing #54. I'll run through the changes here, and whenever you get the chance to take a look (no rush, I know you're busy) I'd appreciate your thoughts.

I first created a branch issue54 then later split that off to sortdesc. (The links show the list of commits for each, the diffs should be fairly simple.)

---

Extracted sortDir strings to an enum structure
Instead of passing strings around, I set up an "enum" to hold them. Unfortunately, creating an object that's accessible to both the plugin and outside code means the best way I could find was $.fn.stupidtable.dir. A little ugly but that can of course be stored in a var for later use.

---

Removed unnecessary "is sorted" check
The is_sorted_array function didn't seem to save any processing. To check if the array was sorted, it sorts a copy of the array, reverses a copy of that array, then compares both to the original. But then it either reverses the original array again or sorts it again. Now that we have the sort-dir data attribute it's much simpler to just sort the array once, in the required direction.

---

New method of forcing redraw
Apparently simply reading any style property from an element forces a redraw, so I do that after running beforetablesort. Fixes #53.

---

Added 'beforesortmapcreate' callback for modifying the sort-map (issue54 branch)
I looked into the sortmap callback that you suggested. To be honest I probably misunderstood what you were suggesting, or else this is gonna overcomplicate things. The plugin change was just one line with the callback, but actually modifying the sort map array (see complex_example.html) turned out to be quite complex. You need to modify the variable in-place due to the way Javascript handles pass-by-reference, plus it requires an extra pass of the array. If you can explain in a bit more detail how you expected this feature to work I can take another crack at it.

---

Allow separate sorting function when sorting descending (sortdesc branch)
One of the ideas I suggested, and I quite like it personally :) It checks for the existence of a data-sort-desc attribute and uses that as the sorting function instead of the one from data-sort. It's quite flexible as it allows for basically any custom handling, including keeping blank (or specific) rows at the bottom.

[joequery]

GREAT SCOTT!! (hahah) It's very cool that you've put all this together

I've been reviewing your changes for the past couple of hours, and there are some issues I've come across. Your sorting functions to account for the blanks are technically incorrect, but they work due to the adjustments you've made to the plugin.

Additionally, I'm not ok with ditching map reversal entirely, but I did find a way to implement it without the is_sorted_array function.

So you had a lot of good ideas with these changes, but the execution on some was slightly off. I've made some adjustments and I'll post them here soon.

[joequery]

Here's an explanation as to why the sorting functions are incorrect

  var moveBlanks = function(a, b) {
    // move blanks to bottom
    if ( a == "" )
      return 1;
    if ( b == "" )
      return -1;
    // regular string sorting
    if ( a < b )
      return -1;
    if ( a > b )
      return 1;
    return 0;
  };
  var moveBlanksDesc = function(a, b) {
    // move blanks to bottom
    if ( a == "" )
      return -1;
    if ( b == "" )
      return 1;
    // regular string sorting
    if ( a < b )
      return -1;
    if ( a > b )
      return 1;
    return 0;
  };

So moveBlanksDesc is our descending function. From the mozilla sort documentation, returning -1 means "sort a to a lower index than b", and returning 1 means "sort a to be a higher index than b". In order to have the larger values on top of the column(equivalent to making larger values have a lower index index the array) this function should return -1 when a > b, and 1 when a < b. So technically this sort function returns the wrong values, but the adjustments you made to the sort_map function caused the result to appear correct.

[svivian]

Wow, fast reply :)
Yes I see what you mean. moveBlanksDesc ends up not being "use this function to sort the values in descending order" but rather "use this function to sort in ascending order, with the knowledge the list will get reversed". I can see that will be confusing.

We could fix this by not reversing the array if we used a custom sort-desc function (and obviously fixing my moveBlanksDesc to actually sort descending). But I'll wait for your other changes first.

I also just made another commit here with a (perhaps) better attempt at the callback thing. This time using the unsorted list of values, so you only need to change the values themselves and the sorting function puts them in the correct place.

[joequery]

So here our my changes to your sortdesc branch. Unfortunately it seems like Github isn't able to generate a diff between two branches when neither of them are master, so you'll need to view it locally

https://github.com/joequery/Stupid-Table-Plugin/tree/svivian_updates

[joequery]

I think your idea of having separate ascending/descending comparison functions completely alleviates the need for a beforesortmapcreate callback. At least for now.

[svivian]

That all looks awesome! If you need any more updates to the docs I can do that tonight, either on the svivian branch or master if you merge it back in. I like your notion of having lots of examples showing the range of things you can do with the plugin. Is it worth moving the HTML files to an examples folder?

Also, are we going to implement #52?

[joequery]

Doc updates would be awesome! You should pull request against the svivian_updates branch (and don't forget the gh-pages branch).

I do plan on having #52 implemented, but I'd like to get these commits and doc updates merged in cleanly before moving on to that. I'm guessing some of these changes will cause conflicts on your branch where you've worked on #52.

[joequery]

And +1 on the examples folder. We'll just need to make sure any references to the examples files in the README and the gh-pages index file are updated.

[svivian]

Now merged into master.