Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a jQuery function which adds a tag to the first row of a table. I previously tried using append, however that didn't work. I now have the following solution which is very slow and it gives this warning:

A script on this page is causing Internet Explorer to run slowly

Function:

jQuery.fn.fixGridView = function () {
    "use strict";
   // var start = +new Date();  // log start timestamp 
    if (jQuery(this).is('table') && this.find('thead').length === 0) {

        var theadv = "<thead><tr>" + this.find('tbody tr:first').html(); +"</tr></thead>";

        this.find('tbody tr:first').remove();
        var htmlv = this.html();
        this.html(theadv + htmlv);
    }
    //var end = +new Date();  // log end timestamp
   // var diff = end - start;
   // alert(diff);
    return this;
};

I have to use IE8, it is client's requirement. I have also created a jsfiddle.

share|improve this question
add comment

1 Answer 1

up vote 3 down vote accepted

As far as I understand, you just want to wrap the first as <thead> and the rest in <tbody>. Why it's slow:

  • html() is one possible cause. That's because you're reconstructing the DOM rather than reusing what's there already.

  • find() is a descendant lookup function. It looks for elements deep into the tree. If you know that the element is a few levels down, then use multiple children() calls instead which only searches one level deep.

  • Just to add, attribute selectors like [id*="gvCategories"] are very slow. Better to use multiple classes instead, like class="MainContent gvCategories". That way, you can do $('.gvCategories'), which is faster.

The fastest and most painless fix is to just write the HTML with the first row in a <thead>. Plain and simple.

If you really want to go with JS, then here's my take on it and the performance test to prove it

jQuery.fn.fixGridView = function () {
    "use strict";

    // A jQuery object may be more than one element. Operate on each  using each()
    // Additionally, return the result to allow chainability.
    return this.each(function () {
        // Cache the element to avoid doing DOM refetch
        var element = $(this);
        if (!element.is('table')) return;  

        // No need for tbody, since the browser automatically adds it
        // http://stackoverflow.com/a/938143/575527

        // Now we avoid using find() since it looks for all descendants.
        // Instead, we use children() to limit our search to 2 levels of children
        var rows = element.children('tbody').children('tr');
        var thead = element.children('thead').length;

        // Get the first row, wrap it in <thead>, and prepend it to the table
        if(!thead) rows.eq(0).wrap('<thead/>').parent().prependTo(element);


    });

};

$(document).ready(function () {
    $('.gvCategories').fixGridView();
});
share|improve this answer
    
Thank you so much @Joseph the Dreamer.. This is very fast now, however i still get the error saying "A script on this page is causing Internet Explorer to run slowly". I am using JQuery table-sorter, can it be the reason to make my page slow? I have only 50-55 records in my table.. –  user1181942 May 2 at 6:51
    
@user1181942 Can't really tell and I don't have much experience debugging in IE8. You could try profiling your code in Chrome or Firefox with their dev tools to see which code is taking long to execute. Then fix that part of the code, and then try again on IE8. –  Joseph the Dreamer May 2 at 6:54
    
..Yes..I will try that.. Nyways thanks.. You made my code super-fast.. –  user1181942 May 2 at 13:27
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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