Manual "updates" applied to jquery.dataTables.js

Manual "updates" applied to jquery.dataTables.js

plcplc Posts: 22Questions: 4Answers: 0

Hi,

I have an ASP gridview which I've applied DataTables to - this works fine. When the user selects to 'Edit' or 'Add' a record, I flip the user into the formview and also make the gridview invisible (using an ASP gvwMyGridview.Visible = false statement).

However - this has caused problems the moment the user resizes the browser or when the gridview is redisplayed. I've applied code updates to jquery.dataTables.js which only execute certain code - if - a condition is met.

I was wondering how I'd go about submitting my changes to benefit others? They're all 'defensive' additions, i.e. test for a condition (e.g. array exists) before attempting to reference element #0 of the array.

Thanks.

Answers

  • allanallan Posts: 63,523Questions: 1Answers: 10,473 Site admin

    The DataTables source repo is available on Github. I am being rather strict about what I pull in though (to keep the library size manageable primarily!), so if you would like to ask any questions about your change please do feel free!

    Allan

  • plcplc Posts: 22Questions: 4Answers: 0

    Cheers Allan, I'm not really familar with Github but I'll certainly take a look. The 4 changes I've made to the code are all 1 line each (so 4 in total) - so they certainly won't add materially to the size of the source.

  • allanallan Posts: 63,523Questions: 1Answers: 10,473 Site admin

    I'd be happy to take just a patch or diff code here and drop the changes in myself if that suits you better.

    Thanks,
    Allan

  • plcplc Posts: 22Questions: 4Answers: 0

    Sounds good to me, Allan. I'll post the changes here in 2 mins..

  • plcplc Posts: 22Questions: 4Answers: 0
    edited May 2015

    At approx line 4168, wrap the following 'if' :-

                    // Fix for error: "Unable to get property 'style' of undefined or null reference" in IE
                    if (headerCells.length > 0) {
                        headerCells[i].style.width = column.sWidthOrig !== null && column.sWidthOrig !== '' ?
                            _fnStringToCss( column.sWidthOrig ) :
                            '';
                    }
    

    At approx line 4511, add the following 'if' :-

                // Fix for error: "Unable to get property 'aDataSort' of undefined or null reference" in IE
                if (aoColumns.length > 0) {
                    aDataSort=aoColumns[srcCol].aDataSort;
    
                    for (k = 0, kLen = aDataSort.length ; k < kLen ; k++) {
                        iCol = aDataSort[k];
                        sType=aoColumns[iCol].sType || 'string';
    
                        if (nestedSort[i]._idx === undefined) {
                            nestedSort[i]._idx = $.inArray(nestedSort[i][1], aoColumns[iCol].asSorting);
                        }
    
                        aSort.push({
                            src: srcCol,
                            col: iCol,
                            dir: nestedSort[i][1],
                            index: nestedSort[i]._idx,
                            type: sType,
                            formatter: DataTable.ext.type.order[sType + "-pre"]
                        });
                    }
                }
                // END PAL
    

    At approx line 6430, add the following 'if' :-

                        // Fix for error: "Unable to get property 'mData' of undefined or null reference" in IE
                        if (col != null) {
                            if (col.mData === i) {
                                var sort = a(cell, 'sort') || a(cell, 'order');
                                var filter = a(cell, 'filter') || a(cell, 'search');
    
                                if (sort !== null || filter !== null) {
                                    col.mData = {
                                        _: i + '.display',
                                        sort: sort !== null ? i + '.@ data-' + sort : undefined,
                                        type: sort !== null ? i + '.@ data-' + sort : undefined,
                                        filter: filter !== null ? i + '.@ data-' + filter : undefined
                                    };
    
                                    _fnColumnOptions(oSettings, i);
                                }
                            }
                        }
                        // END PAL
    

    At approx line 8285, add the following try..catch :-

                    // Fix for error: "NotFoundError" in IE
                    try {
                        if ( tr ) {
                            // insertBefore can act like appendChild if 2nd arg is null
                            tr.insertBefore( cells[ column ], cells[ insertBefore ] || null );
                        }
                    }
                    catch (err) {
                        // Don't want to do anything with the error - just want to prevent an error from
                        // bubbling up to the browser
                    }
                    // END PAL
    

    All four fixes work for me, but only the last one I wasn't completely sure of - it prevents the error from bubbling up to the browser, but I couldn't think of a way of testing that it was possible to do a tr.insertBefore before going ahead and doing it.

    Only the if and try..catch lines have been added. I had to amend a couple of other lines here in this post, but that was only because the editor seems to add a hrefs into a few places, which I didn't want.

    Many Thanks, Allan.

  • allanallan Posts: 63,523Questions: 1Answers: 10,473 Site admin

    Thanks for these!

    Lines 4168 and 4511 - Zero column tables

    Why would you have a table that has 0 columns? DataTables can't have columns dynamically added or removed at the moment, so I've assumed that there always be one or more columns and everything else is an error (like in this case - actually sometimes useful to tell people that there is a misconfiguration!).

    Line 6430 - "Unable to get property 'mData' of undefined or null reference" in IE

    Are you able to give me an example of what would cause this please (i.e. a test page showing the error)? That sounds like an important one to fix - or does this come about again from 0 column tables?

    Line 8285 - try / catch

    Could you also give me test case for this one? I'm wondering what would cause that.

    Thanks,
    Allan

  • plcplc Posts: 22Questions: 4Answers: 0

    I'll try to set something up in JSFiddle.

    They'll all likely have come to light because I remove the gridview while the formview is on display - but if the user resizes the browser at this time, DataTables expects the gridview to still be in place.

    I need the DataTables library to work whether or not there's a gridview on display.

  • plcplc Posts: 22Questions: 4Answers: 0

    Actually, Allan, I'm going to struggle to set up a test case in JSFiddle, because I'm using ASP.NET.

    The gridview is initially displayed and I invoke DataTables at this point.

    The user then clicks [Edit], the gridview object is removed from the screen altogether (using gvw.Visible = false;) - this is not the same as changing its style to display:none or visibility:hidden.

    I then enable the formview and display the editable fields.

    The user then resizes the browser - with no gridview on display - and the first error pops up. Applying my first fix allows it to get past this point, but it then errors at the second point etc.

    I realise that I'm probably talking 'edge cases' here, and that most users likely have their gridview on display at all times, but I am where I am so I currently have to apply all these fixes to each DataTables release so that my code doesn't break.

    Many Thanks.

  • allanallan Posts: 63,523Questions: 1Answers: 10,473 Site admin

    Thanks for the additional information. What does gvw.Visible do - does it remove the element from the page and then destroy it? Or is it just removed so it can be added back in later?

    The changes look absolutely fine, but I would like to understand a little bit more about them before putting them in. You are right they aren't large changes, but I'm fighting a loosing battle to keep the library under 80000 bytes in size when minified. Its slightly over at the moment in my dev version and I've got a couple of other features I want to add for the next release. So every byte counts atm!

    Allan

  • plcplc Posts: 22Questions: 4Answers: 0
    edited May 2015

    gvw.Visible = false removes the element from the page as if it wasn't there (in the rendered HTML, you won't find any references to it - hence any jQuery or javascript which references it will fail unless appropriate 'null' tests are included).

    I completely understand your comments re:size. Like I said, my changes might well be "edge case", and for that reason I'm happy to keep reapplying them to future versions, but on the flip-side I also wondered if they might help others when the gridview is removed from the page but libraries continue to reference it, expecting it still to be there.

    The final decision is of course yours, and I'll completely respect any decision not to include these changes - particularly if you're looking to keep the file size down to a minimum.

    I really appreciate you having cast your eyes over these changes, Allan.

This discussion has been closed.