2
\$\begingroup\$

I'm working on my own little library on top of jQuery to make working with tabular data easier.

Currently my script takes user clicks in a table's thead element and creates an array of all of the table cells in that column, and gives the user an animation of changing the background color of the td elements in the column that was clicked on the table. When a user clicks on a thead element, all of the td's in that column are added to a variable clickedHeader. By holding down the shift key, the user can add multiple table columns to this array.

I'm looking for a cleaner way to package this, specifically the section marked in the comments. fiddle

EDIT: For some reason the animations don't work in the fiddle, but do locally. The rest of the script works fine in the fiddle

JS

(function () {
        //all tables in this example have a thead element
        var headers = $('table').find('thead > tr th'),
            clickedHeaders = [];

        //to test what's currently in clickedHeaders
        $(document).dblclick(function () {
            console.log(clickedHeaders);
        });
        headers.click(function (e) {
            //allow only one table column to have background color at a time
            //this looks a little hooky to me
            var tds = $('td');
            var columnData = getColumnData.call(this);
            if (!e.shiftKey) {
                //if no shift, empty the array
                clickedHeaders.length = 0;
                tds.each(function () {
                    $(this).css('background-color', 'white');
                });
                //adds animation to notify user
                notifyClick(columnData.cells);
                clickedHeaders.push(columnData.returnData);
            }
            else {
                notifyClick(columnData.cells);
                clickedHeaders.push(columnData.returnData);
            }
        });
        //passed a tbody's td elements as array
        function notifyClick(data) {
            data.each(function (i) {
                $(this).delay(30 * i).animate({ backgroundColor: $(this)
                                 .css('background-color'), backgroundColor: '#ccc'
                }, 25, 'swing');
            });
        };
        //find the column that was clicked
        function getColumnData() {
            //table's selector is column headers
            var clickedIndex = $(this).index() + 1,
            tableId = $(this).closest('table').attr('id');

            //selector to get the index of a column
            var columnSelector = function (index) {
                return '#' + tableId + ' tbody tr td:nth-child(' + index + ')';
            };

            //processing will be done on the text of the td elements
            //but I still wanted to be able to keep the array of elements as well
            var columnData = $(columnSelector(clickedIndex));
            var returnData = columnData.map(function () {
                return parseInt($(this).text(), 10);
            }).get();
            return {
                returnData: returnData,
                cells: columnData
            }
        }
    })();

HTML

<table id="testTable2">
 <thead>
  <tr>
     <th>Month</th>
     <th>Savings</th>
  </tr>
 </thead>
 <tfoot>
  <tr>
     <td>Sum</td>
     <td>$180</td>
  </tr>
 </tfoot>
 <tbody>
  <tr>
     <td>11</td>
     <td>22</td>
  </tr>
  <tr>
     <td>33</td>
     <td>44</td>
  </tr>
 </tbody>
</table>

<table id="testTable">
 <thead>
  <tr>
     <th>Month</th>
     <th>Savings</th>
  </tr>
 </thead>
 <tfoot>
  <tr>
     <td>Sum</td>
     <td>$180</td>
  </tr>
 </tfoot>
 <tbody>
  <tr>
     <td>57</td>
     <td>100</td>

  </tr>
  <tr>
     <td>56</td>
     <td>80</td>
  </tr>
    <tr>
     <td>56</td>
     <td>45</td>
  </tr>
    <tr>
     <td>56</td>
     <td>30</td>
  </tr>
    <tr>
     <td>56</td>
     <td>10</td>
  </tr>
 </tbody>
</table>
\$\endgroup\$
1
\$\begingroup\$

From a once over:

  • I would call tds -> $td, it shows the reader that its the result of a td query
  • There is some duplication in your dealing with shiftKey, you could try this:

    if (!e.shiftKey) {
      //if no shift, empty the array
      clickedHeaders.length = 0;
      $td.css('background-color', 'white');
    }
    notifyClick(columnData.cells);
    clickedHeaders.push(columnData.returnData);
    
  • I am not sure how your animation can work, jQuery docs state : width, height, or left can be animated but background-color cannot be
  • I am not a big fan of closest, you might as well use parents :

    tableId = $(this).parents('table').attr('id');
    

    This should give you less potential surprises

  • Finally, I find that your code does not work well, if I keep shift-clicking the same header, I will get a duplication of the data in console.log(clickedHeaders);. You should keep a unique list of values so that the collected columns are useful/usable.
\$\endgroup\$
  • \$\begingroup\$ @konjin it worked because I forgot I had included jQueryUI (which does animate colors). I just remembered why :). +1 for keeping a unique list as well. \$\endgroup\$ – wootscootinboogie Feb 24 '14 at 16:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.