row().remove().draw(false) causes blank table

row().remove().draw(false) causes blank table

petr24petr24 Posts: 12Questions: 2Answers: 0
edited March 2016 in Free community support

So I have a DataTable initialized with few options such as scroller.

And when the sort order is desc or asc, and I remove a row and the table is scrolled to the top of the list it works...
however if I am scrolled all the way to the bottom of the list and I delete a row, the table goes blank on draw().

I couldn't get a demo sample of my code on JS Fiddle or DataTables live.

Been playing with it for a few hours now, can't seem to find whats causing this.

but I have my init code, and row remove code

I use DT_RowId for rows, here is how I remove a row.

oTable.row("#" + contact._id).remove().draw(false);

Init Code

var oTable = $('.contacts-table').DataTable({
        scrollY: "600px",
        scrollCollapse: true,
        data: getData(oldTime),
        "sDom": "rtiS",
        "scroller": true,
        responsive: true,
        "autoWidth": true,
        "bDeferRender": true,
        "columnDefs": [
            {
                "targets": 0,
                "data": "contactPhoto",
                "render": function ( data, type, full, meta ) {
                    return (data) ? "<img src="+data+">" : "<img src='/carbon/empty-avatar.png'>";
                },
                className: "contact-avatar",
                'bSortable' : false
            },
            {
                "targets": 1,
                "data": "name",
                "order": "desc",
                "render": function ( data, type, full, meta ) {
                    return (data) ? data : "----";
                },
                className: "name"
            },
            {
                "targets": 2,
                "data": "phone",
                "render": function ( data, type, full, meta ) {
                    if (data) {
                        return (data[0] ? data[0].value : "----");
                    }else {
                        return "----";
                    }
                },
                className: "phone"
            },
            {
                "targets": 3,
                "data": "email",
                "render": function ( data, type, full, meta ) {
                    if (data) {
                        return (data[0] ? data[0].value : "----");
                    }else {
                        return "----";
                    }
                },
                className: "email"
            },
            {
                "targets": 4,
                "data": "shared",
                "render": function ( data, type, full, meta ) {
                    return "<i class='fa fa-trash-o delete-contact-table contact-delete-trash'></i>"
                },
                'bSortable' : false
            },
            {
                "targets": 5,
                "data": "shared",
                "visible": false,
                "searchable": true,
                "render": function (data, type, full, meta) {
                    if (data) {
                        return "shared";
                    }else {
                        return "";
                    }
                }
            }
        ],
        "fnCreatedRow": function (nRow, aData, iDisplayIndex) {
            $(nRow).attr('id', aData._id);
            if (aData.shared) {
                $(nRow).addClass('success');
            }
            $(nRow).addClass('contact-row');
        },
        "fnInitComplete": function(oSettings, json) {
            Session.set('templateReady', true);
        },
        "drawCallback": function () {
            console.log('DRAW ROW');
        }
    });

Answers

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    I'd really need a test case to be able to debug the issue.

    Does it work if you don't use Scroller?

    Allan

  • petr24petr24 Posts: 12Questions: 2Answers: 0
    edited March 2016

    Hey Allan,

    Thanks for the quick reply, I just got back to my desk, and I tried without scroller
    and the issue still exists.

    I also went ahead and created an exact replica of my code on a dummy link on our staging server. So it has 300 dummy rows, and if you hover over the row you will see
    a trash can, and if you're scrolled to the bottom and delete a row, you will see how it goes blank.

    Here is the link.
    https://staging.carboncontact.com/allan

    And when it goes blank, you can just resort or scroll up some and then back down and it resets.

    Also been trying it get it to work in a fiddle, but I figured I'll do this in the mean time.
    Ran the debugger tool, which is amazing btw, works so nice and easy.
    http://debug.datatables.net/afumus

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Ah - I see! The problem is when you are at the end of the display and delete a row. I'm really surprised that issue exists without the Scroller extension - I would have thought that the issue is related to the fact that Scroller is forcing the table's height and that needs to be updated.

    That's going to take a bit of debugging and poking around I'm afraid and I'm not entirely sure when I'll get the time to do so, but I'll try to make some time to look at it next week.

    Thanks for the test case :-)

    Allan

  • petr24petr24 Posts: 12Questions: 2Answers: 0

    Hey so I have been playing around and I think I have the culprit. Its two things that I can test that cause the issue.

    The first is if I don't have scroller enabled, but my sDom settings are "rtiS", the same issue occurs where the table goes blank, but If I remove the sDom and instead do something like this, "lengthMenu": [[10, 25, 50, -1], [10, 25, 50, "All"]], that problem goes away.

    And for my actual app, locally I put lengthMenu and removed scroller, and the problem goes away. Hopefully this helps lead us into the right direction trying to find what it is exactly about those two.

    I'll keep looking more into the detail.

    And thank you for you help man.

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    The first is if I don't have scroller enabled, but my sDom settings are "rtiS",

    That still enables Scroller. In fact the S option is legacy now - it can be dropped and just use the scroller option.

    Allan

  • petr24petr24 Posts: 12Questions: 2Answers: 0

    Hey Allan,

    Been playing around with the source code to try and see whats happening.
    Hopefully this helps narrow it down, or at least give you some hints.

    In _fnDrawCallback function inside Scroller,
    you have these 3 lines which calculate boundaries, and set redrawTop and redrawBottom, If I comment these 3 lines out, I don't have the bug of the blank table, when removing a row. I wouldn't label it as a fix by any means.

    var boundaryPx = (iScrollTop - this.s.tableTop) * this.s.boundaryScale;
    this.s.redrawTop = iScrollTop - boundaryPx;
    this.s.redrawBottom = iScrollTop + boundaryPx;
    
    
  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Can you confirm that it works if you disable Scroller completely (no S in dom and no scroller option)?

    Allan

  • petr24petr24 Posts: 12Questions: 2Answers: 0

    Yes, if scroller option is disabled and no S in dom options, it works.

  • petr24petr24 Posts: 12Questions: 2Answers: 0
    edited August 2016

    Hey Allan,

    Don't know if you had time to check out this bug out more. The "fix" that I thought I had was bs, made scrolling impossibly choppy. However I found something that I think is causing the blank table on draw('false');

    With scroller enabled, I scroll down until a few draw events have been triggered.

    In the "_fnDrawCallBack" function, whenever the table goes blank I noticed the var displayStart is 0, where normally if I am just scrolling, it continually increments. Don't know if that helps to try and narrow down the issue.

    Update --

    So if I scroll down enough to see past row 704, the table goes blank after that row consistently, btw I am using a displayBuffer: 40, if I change displayBuffer to 20, the table disappears around row 350 and down to about 530, until it renders again, the math adds up for the re-render with displayBuffer.

    The results are not consistent as to when it goes blank, here is a little screen capture I recorded, with displayBuffer: 20.

    http://recordit.co/xZOeMWcCZn

  • petr24petr24 Posts: 12Questions: 2Answers: 0

    Ah Allen,

    Finally got it! Well kinda. So after way to many hours debugging I found that when ever a draw(false) || draw('full-hold') || draw('page') takes place, scroller needs to calculate its position. It uses the function fnDisplayEnd to get the start, end, length, etc of the table. And while scrolling, if I look at the values they makes sense. However when the draw event takes place (not from scroll), the var _iDisplayStart is off, and thats what messes up the whole calculation of how many rows to render.

    Example.
    The top row in the view port lets say is 1100, and it renders 40 rows.
    The end value is going to be 1140, and that fills up the whole view port + a few extra. However with the draw event, even though the top row still is 1100, somehow the value gets messed up and starts higher than it should. So now it starts at for example purposes 1050 renders 40 rows = 1090, and because thats not enough to fill the table thats what causes the blank rows.

    And as the hackiest fix ever just to make sure my theory was right, I get the top position from the text of the table info, where it says "Showing x to x of x entries", I use the first value for the top row, make it and replace iDisplayStart with that value and it works! No more blank rows on row().remove().draw().

    I still don't know why the iDisplayStart is wrong on draw, but maybe you might. This bug also happens on the scroller example, with client 50k rows.

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Heh - clever hack. Thanks for noting that it happens with the example as well. I've got a bit of a backlog of support requests atm I'm afraid so I'm not sure when I'll get to look into this in detail, but I've got it bookmarked to make sure I do so.

    Allan

  • IneenthoIneentho Posts: 3Questions: 0Answers: 0

    Is there an issue for this somewhere that I can track? If no work has been started on this I might do some digging on this issue too.

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Is there an issue for this somewhere that I can track?

    This thread :-).

    I've not had a chance to look into it yet, so any insight you might have is welcome.

    Regards,
    Allan

  • IneenthoIneentho Posts: 3Questions: 0Answers: 0
    edited October 2016

    Ok, I see :) I just wanted to make sure I didn't miss any progress that was made in a Github issue or something similar.

    There seems to be several bugs regarding removing rows, but the one we are experiencing in this thread seems to because fnLengthOverflow https://github.com/DataTables/DataTablesSrc/blob/b05edc5fe06e6ddac4e738d8fe580227d363579c/js/core/core.support.js#L225 , which seems to be the function that moves items from the next page to the current page in paginated tables?

    If I for test purposes comment out fnLengthOverflow the $dt.row(X).remove().draw(false) call works fine in all cases I could find.

    There seems to be an unrelated issue where $dt.row(500).remove().draw().row(495).scrollTo(false) works once, but if I was in the row 500 area when calling remove the blank table bug would appear again.

    An alternative (kind of dirty) solution would perhaps be to force a recalculation when removing a row, placing a if ( forceRecalculation || (isScrollTop < .... in _fnScroll at https://github.com/DataTables/Scroller/blob/aa3ecb78d691a27444d48d6ecb29be3726ff3c8b/js/dataTables.scroller.js#L659 seems to work when I did some quick testing, but I didn't find a good condition in which to set forceRecalculation to true with (I didn't find a hook for row removal?).

    I would try creating a PR but I don't think I'll be able to provide a clean solution, but I'd be happy to do some more digging if it could help you come up with a solution.

    Update: The forceRecualculation tests fixes both the original issue and the one for draw(undefined).

  • IneenthoIneentho Posts: 3Questions: 0Answers: 0

    I just wanted to leave a quick update on what I'm doing for now:

    It seems like commenting out _fnLengthOverflow from the rows().remove() method is all that is needed to make this work. For now, I'm using $.fn.DataTable.Api.registerPlural( 'rows().remove()', 'row().remove()', function () {... to override the function, but I think something that allows the scroller plugin to disable the lengthOverflow function is needed for a real fix.

    I will use this fix for now and update again if I notice if I come closer to a real solution.

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Thanks for posting back with your findings - very interesting. The ideal case would be not to need to modify the core methods for the extension to work, but if this workaround is working for you, that's great.

    Allan

  • Oliver-GibsonOliver-Gibson Posts: 2Questions: 0Answers: 0

    Hi,

    I'm facing the same problem using scroller + client side. Are there any updates on this issue ?

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Not yet sorry. I've been focusing on other issues. I'll post back here when I go get to it.

    Allan

  • Oliver-GibsonOliver-Gibson Posts: 2Questions: 0Answers: 0
    edited May 2017

    Hi again,

    I did some testing. First off for the sake of completeness lets remind that _fnLengthOverflow is called on row remove or change in page length.

    In function _fnLengthOverflow commenting

    start -= (start % len);

    Seems to fix the issue discussed here but in return messes up the change in page length when you are at the end of the datatable (that is when you use pagination instead of scroller). Could somebody else confirm that it fixes the issue ?

    Edit:

    So here the problem is that _fnLengthOverflow does too much work. The line described above fixes _iDisplayStart but should be executed only when a page length change occures. The fix is to move that particular line to _fnLengthChange right after the _fnLengthOverflow call.

  • hassanmustafa008hassanmustafa008 Posts: 2Questions: 1Answers: 0

    Hi,

    I'm facing the same problem using scroller + client side. Are there any updates on this issue ?

  • allanallan Posts: 61,650Questions: 1Answers: 10,094 Site admin

    Not yet. As I mentioned above I'll update the thread when I've had time to look into it.

    Allan

This discussion has been closed.