fnOpen with jQuery html [with fix]

fnOpen with jQuery html [with fix]

jeronejerone Posts: 12Questions: 0Answers: 0
edited August 2010 in General
I wanted to use the fnOpen function, only with jQuery html instead of string html, when I noticed this isn't possible. So I made the following change to fnOpen (line 1651):

[code] if(sHtml.jquery){
nNewCell.appendChild(sHtml.clone()[0]);
}else{
nNewCell.innerHTML = sHtml;
}[/code]

Note: when using jQuery html the id is usually used as identifier, but this will be cloned too. It's therefor not correct to include id's in this jQuery. I solved it with: [code]$("#example").hide().children().clone().show();[/code]

Replies

  • jeronejerone Posts: 12Questions: 0Answers: 0
    I updated the fix with possibility to add normal HTML too:

    [code] if(sHtml.jquery){
    nNewCell.appendChild(sHtml.clone()[0]);
    }else if(typeof sHtml == "object"){
    nNewCell.appendChild(sHtml);
    }else{
    nNewCell.innerHTML = sHtml;
    }
    [/code]
  • jeronejerone Posts: 12Questions: 0Answers: 0
    Still like this to be implemented. I want to use jQuery as the second parameter in fnOpen().
  • allanallan Posts: 63,538Questions: 1Answers: 10,476 Site admin
    I think that sounds reasonable - in future I want to support jQuery objects for all API methods, but this one certainly makes sense just now. I'll add it in for the next release. One thing though - why the clone and not just insert the object?

    Allan
  • jeronejerone Posts: 12Questions: 0Answers: 0
    Great! Cloning was needed in my situation.
  • allanallan Posts: 63,538Questions: 1Answers: 10,476 Site admin
    Okay cool. I'll drop that in the implementation I'll commit then, and if needed you could just pass the clone result in as one of the arguments.

    Regards,
    Allan
  • jeronejerone Posts: 12Questions: 0Answers: 0
    You're right, the cloned result could be passed in as the argument.
    Hope to see this feature implemented in the next release.
  • timtuckertimtucker Posts: 48Questions: 0Answers: 0
    Just tried adding a jQuery object via fnOpen, but I'm finding that it doesn't work if I'm working with a jQuery object that represents more than 1 node in the dom.

    Here's what I'm trying to do:
    [code]
    // Open the details row
    dt.fnOpen(nTr, 'Test: ', "details_row");

    // Cache the contents of the details row (will preserve form values)
    var cached_content = $(nTr).next().find("td").contents().remove();
    dt.fnClose(nTr);

    // Reopen the details row with the cached content
    dt.fnOpen(nTr, cached_content, "details_row");
    [/code]

    Here's the error I get:
    [code]
    Could not convert JavaScript argument arg 0 [nsIDOMHTMLTableCellElement.appendChild]
    [/code]

    Looking at the source and doing a little bit of testing, it looks like you could support this type of use by changing:
    [code]
    if( mHtml.jquery !== undefined || typeof mHtml === "object" )
    {
    nNewCell.appendChild( mHtml );
    }
    else
    {
    nNewCell.innerHTML = mHtml;
    }
    [/code]

    to the simpler form:
    [code]
    $(nNewCell).html( mHtml );
    [/code]

    Alternately, you could cut the whole creation process down to:
    [code]
    var nNewCell = $('', {
    "class" : sClass,
    "colspan" : _fnVisbleColumns( oSettings )
    }).html(mHtml);
    var nNewRow = $('').append(nNewCell)[0];
    [/code]
  • timtuckertimtucker Posts: 48Questions: 0Answers: 0
    Doing a little testing with jsPerf it looks like the jQuery-based method is quite a bit slower (even if it is a little simpler in syntax):
    http://jsperf.com/jquery-append-row

    Another alternative to the current approach that's slightly faster for basic strings (only 1 condition vs 2), but allows for more flexibility for jQuery objects:
    [code]
    if (typeof mHtml === "string")
    {
    nNewCell.innerHTML = mHtml;
    }
    else
    {
    $(nNewCell).html(mHtml);
    }
    [/code]
  • allanallan Posts: 63,538Questions: 1Answers: 10,476 Site admin
    I think this is a perfectly sound suggestion - thank you for making it and taking the time to set up the performance tests :-). I've just committed the change and it will be in the 1.9 beta 2 release, which should be available in the next day or two.

    Regards,
    Allan
This discussion has been closed.