Bug Fixes for _fnCalculateColumnWidths

Bug Fixes for _fnCalculateColumnWidths

awelchawelch Posts: 24Questions: 1Answers: 2

This bug fix is for tables using the style table-layout: fixed. There is a problem when trying to set discreet column widths if scrollX is true. If a column has content wider than the specified width, the content's width will override the set width. You can see an example here: live.datatables.net/rumomuze/10/edit. As you can see in the example the bottom table which uses a workaround has the columns sized appropriately but columns 2 and 3 in the top table are wider than they have been set. The workaround applied to the bottom table is problematic as it involves the use of scrollXInner which has been deprecated and according to this doc and @allan's comment on this thread should not be used. Not to mention calculation of the appropriate width before initialization of the table can be messy and reduces the dynamic capabilities of the table. While developing a better fix for this I came across a couple related bugs in the _fnCalculateColumnWidths function.

Bug Fix 1

The first bug does not affect the functionality of the main bug I am addressing and I actually have not noticed a manifestation of any problems from this bug in practice (I haven't explicitly looked into it and I think I may have addressed it with some custom css a while back) but the code does not look right. In the section where tmpTable is created and modified there is the following block of code:

// When scrolling (X or Y) we want to set the width of the table as 
// appropriate. However, when not scrolling leave the table width as it
// is. This results in slightly different, but I think correct behaviour
if ( scrollX && scrollXInner ) {
    tmpTable.width( scrollXInner );
}
else if ( scrollX ) {
    tmpTable.css( 'width', 'auto' );
    tmpTable.removeAttr('width');
    
    // If there is no width attribute or style, then allow the table to
    // collapse
    if ( tmpTable.width() < tableContainer.clientWidth && tableWidthAttr ) {
        tmpTable.width( tableContainer.clientWidth );
    }
}
else if ( scrollY ) {
    tmpTable.width( tableContainer.clientWidth );
}
else if ( tableWidthAttr ) {
    tmpTable.width( tableWidthAttr );
}

The if statement that allows a scrollX enabled table to collapse does not seem to match the comment above it. It seems like it should be the following:

// If there is no width attribute or style, then allow the table to
// collapse
if (tmpTable.width() < tableContainer.clientWidth && !tableWidthAttr && !styleWidth) {
    tmpTable.width(tableContainer.clientWidth);
}

I may be misinterpreting the goal of this block of code. If by collapse it is meant that the table should be allowed to be smaller than the tableContainer and only set to the size of the container if there is a width attribute or style then alternatively I would think the code should read:

// If there is no width attribute or style, then allow the table to
// collapse
if (tmpTable.width() < tableContainer.clientWidth && (tableWidthAttr || styleWidth)) {
    tmpTable.width(tableContainer.clientWidth);
}

This, however, necessitates the use of the css width: 100% on a table when scrollX is set to true or you end up with the undesirable formatting shown here (when the page is wide enough): live.datatables.net/rumomuze/11/edit?output. I'm having a hard time wrapping my head around when it would be useful to let a table collapse smaller than the container when using scrollX. It seems to be contrary to the purpose of horizontal scrolling. If someone could provide an example that would be much appreciated and I can tailor the solution accordingly.

Bug Fix 2

The second bug fix is to the setting of the userInputs variable. This fix is necessary to fix the main issue I have described here and may fix additional issues I haven't tested for. It involves the section of code that converts any user input sizes into pixel sizes:

/* Convert any user input sizes into pixel sizes */
for ( i=0 ; i<visibleColumns.length ; i++ ) {
    column = columns[ visibleColumns[i] ];
    
    if ( column.sWidth !== null ) {
        column.sWidth = _fnConvertToWidth( column.sWidthOrig, tableContainer );
    
        userInputs = true;
    }
}

In this block of code if column.sWidth already has a value it is assigned the value of column.sWidthOrig. This seems redundant and since _fnCalculateColumnWidths always sets sWidth at some point this has the consequence that userInputs is always set to true after the first time this function is called even if there were no actual user inputs to the initialization. I believe this if statement is meant to check that column.sWidthOrig is not null:

if ( column.sWidthOrig !== null ) {
    column.sWidth = _fnConvertToWidth( column.sWidthOrig, tableContainer );
    
    userInputs = true;
}

Main Bug Fix

The final change that directly influences the behavior I initially described involves the same block of code as the first bug. If scrollX is set to true and scrollXInner is not set tmpTable has the css width: auto applied. This is problematic as the table will then ignore the specified size on any column that is not wide enough to fit the biggest content in said column. The fix I am using (along with the corrected code to set userInputs in Bug Fix 2) is to only set the width to auto if there is no user specified column sizes as follows:

else if (scrollX) {
    if (!userInputs) {
        tmpTable.css('width', 'auto');
        tmpTable.removeAttr('width');
    }

    // If there is no width attribute or style, then allow the table to
    // collapse
    if (tmpTable.width() < tableContainer.clientWidth && !tableWidthAttr && !styleWidth) {
        tmpTable.width(tableContainer.clientWidth);
    }
}

Replies

  • allanallan Posts: 49,086Questions: 1Answers: 7,177 Site admin

    This is excellent - thank you! I need to give myself a little head room to think about this a bit more and make sure that there aren't any knock on effects, so it might be a little while before I can get this integrated (probably in v2) but that looks great. I've added it to my bug tracker for v2 so I don't loose it.

    Regards,
    Allan

Sign In or Register to comment.