2

I have following data table , with individual sorting ascending and descending buttons. I am using jQuery plugin "jQuery.fn.sort" James Padolsey

Here is the working example

http://jsbin.com/alawub/2/edit

enter image description here

I want to add sorting to each Col. but its not working please review my JS code above any other alternative solution for this is welcome.

jb10210
  • 1,158
  • 5
  • 15
Wasim Shaikh
  • 6,880
  • 18
  • 61
  • 88

5 Answers5

5

You are adding the click handlers too many times:

$('th')
    .wrapInner('<span title="sort this column"/>')
    .each(function(){
        ...
        $('#accending_1,#accending_2').click(function(e){

What this will do is for every th, and there are two rows of 4 th tags, add a click handler to the elements with id accending_1 and accending_2. That will add 8 click handlers to each button!

There are numerous ways to fix this. Instead of having specific id's for each button use classes and find them relative to the header:

$('th')
    .wrapInner('<span title="sort this column"/>')
    .each(function(){

        $('.accending', this).click(

example

Note the this parameter on the last line that limits the selector to descendants of the current TH. That would still be doing searches for the top row of TH's though.

It's probably better to just search directly for the buttons and then work out what column they are in.

  $('.accending, .accending')
    .wrapInner('<span title="sort this column"/>')
    .click(function(e){
      console.log("click");
      var th = $(this).closest('th'),
        thIndex = th.index();
      table.find('td')
        .filter(function(){
          return $(this).index() === thIndex;
        })
        .sort(
          function(a, b){
            return $.text([a]) > $.text([b]);
          }, function(){
            // parentNode is the element we want to move
            return this.parentNode; 
          }
        );
    });
  $('.decending, .decending')

example

There's still a lot of duplication in the code, and html.

The accending and decending click handlers are almost the same, so lets just pass in the sort function.

  //This function returns another function which is a click handler. 
  //It takes the sort function as a parameter.
  function createClickHandler(sortFunction){
    return function(e){
      var th = $(e.target).closest('th'),
        thIndex = th.index();
      console.log(th);
      table.find('td')
        .filter(function(){
          return $(this).index() === thIndex;
        })
        .sort(sortFunction, function(){
            // parentNode is the element we want to move
            return this.parentNode; 
          }
        );
    };
  }

  $('.accending, .accending')
    .wrapInner('<span title="sort this column"/>')
    .click(createClickHandler(function(a, b){
        return $.text([a]) > $.text([b]);
      })
    );
  $('.decending, .decending')
    .wrapInner('<span title="sort this column"/>')
    .click(createClickHandler(function(a, b){
        return $.text([a]) < $.text([b]);
      })
    );

example

The HTML can also be cleaned up a bit. The tags for the buttons are all the same so lets insert them from javascript adding the click handlers to them directly instead of having to search for them. Since we're iterating over the column headers again we can clean up how we get the column number. And finally, passing two different sort functions is a bit wasteful so let's pass a direction parameter instead.

  //This function returns another function which is a click handler. 
  //It takes the sort function as a parameter.
  function createClickHandler(column, isAccending){
    return function(e){
      table.find('td')
        .filter(function(){
          return $(this).index() === column;
        })
        .sort(function(a, b){
          var order = $.text([a]) > $.text([b]);
          return isAccending ? order : !order;
        }, function(){
          // parentNode is the element we want to move
          return this.parentNode; 
        })
      ;
    };
  }

  $('#controls th').each(function(column,item){
    //Note we get the column index for for free with the each function
    $(this)
      .append($('<a title="sort this column" href="#">Ascending</a>')
        .click(createClickHandler(column, true))
      )
      .append('&nbsp;&nbsp;')
      .append($('<a title="sort this column" href="#">Decending</a>')
        .click(createClickHandler(column, false))
      )
      ;
  });

example


Note that I removed the inverse variable. It was never being used.

return $.text([a]) > $.text([b]) 
    inverse ? -1 : 1
  ;

I'm not sure what you thought this was doing but it's actually returning on the first line due to automatic semicolon insertion. It will be interpreted as:

return $.text([a]) > $.text([b]);
    inverse ? -1 : 1;

so the inverse is dead code. It's one of the javascript's bad bits and not very obvious. jsbin was warning you about missing semicolons. It's always worth fixing any errors/warnings before submitting code here.

Sam Hasler
  • 12,344
  • 10
  • 72
  • 106
  • Thanks for detailed answer and explanation , but I think there is some issue with last column, when we sort Acc. Order is not correct is [ 123, 1234, 324 ] when It should be [123, 324, 1234], What could be the reason for this specific bug? – Wasim Shaikh Mar 05 '12 at 13:14
  • @WasimShaikh, The sort function is a sorting as text not as number. – Sam Hasler Mar 05 '12 at 17:20
  • @WasimShaikh Use one of the sort functions from one of these questions: http://stackoverflow.com/questions/2802341/natural-sort-of-text-and-numbers-javascript http://stackoverflow.com/questions/2802341/natural-sort-of-text-and-numbers-javascript to assign to the `order` variable or write your own. You'll be the best judge of what sorting algorithm best suits your data. – Sam Hasler Mar 06 '12 at 10:08
0

I really love the "sorttable" script at kryogenix.org - http://www.kryogenix.org/code/browser/sorttable/

Very easy to use and setup.

Zack Macomber
  • 6,682
  • 14
  • 57
  • 104
0

I made your code work. Here is the code, also you can test it in jsbin : http://jsbin.com/alawub/15/

Javascript:

$(document).ready(function(){

  var $table = $('table');
  $table.on('click', 'th a.accending_1, th a.decending_1', function(e){
    var th = $(e.target).parent('th'),
    thIndex = th.index(),
    direction = $(e.target).attr('class').match('accending')?'A':'D', 
    sortDir = th.data('direction'+thIndex);
    if(sortDir  && sortDir == direction ) return;

      $table.find('td').filter(function(){
                return $(this).index() === thIndex;
            }).sort(function(a, b){
                if( direction == 'D' )
                   return $.text([a]) > $.text([b]) ? -1 : 1;
                else 
                   return $.text([a]) > $.text([b]) ? 1 : -1;

            }, function(){
                // parentNode is the element we want to move
                return this.parentNode; 
            });
         th.data('direction'+thIndex, direction);
    });

});

HTML: (just corrected class of A's )

    <th id="facility_header1">
        Table Header 1<br />
        <a href="#" class="accending_1">Accending</a>&nbsp;&nbsp;
        <a href="#" class="decending_1">Decending</a>
    </th>
     <th id="facility_header2">
        Table Header 2<br />
        <a href="#" class="accending_1">Accending</a>&nbsp;&nbsp;
        <a href="#" class="decending_1">Decending</a>
  </th>

Working example over jsbin: http://jsbin.com/alawub/15/

Guumaster
  • 389
  • 3
  • 10
0

I think that tablesorter offers the features you need. It certainly handles text and numeric columns. There's an issue with dynamically updating the table but there's an excellent fix here.

Beetroot-Beetroot
  • 18,022
  • 3
  • 37
  • 44
0

I could see 4 warnings in JS Bin.

Line 67: return $.text([a]) > $.text([b]) --- Missing semicolon.
Line 68: inverse ? -1 : 1 --- Expected an assignment or function call and instead saw an expression.
Line 82: return $.text([a]) < $.text([b]) --- Missing semicolon.
Line 83: inverse ? 1 : -1 ; --- Expected an assignment or function call and instead saw an expression.

Hopefully, correcting those errors will give you the expected result.

MACMAN
  • 1,883
  • 1
  • 21
  • 35