Bug report: Issue with RowGroup integrated with Responsive and custom renderer

setwebmaster

The issue has been reproduced here

To see it, simply run the js, then play with the output size to cause responsive plugin to recompute the columns and redraw.
The columns in the rowGroups that were initially fine, will become too long for the table as each td element will have the full table width set.

The source of the problem seems to come from dataTables.rowGroup.js#L186 where each td under the group row will be given a colspan equal to this._colspan(), which corresponds to the full table width.

allan

I agree - it should be:

$( 'tr.'+this.c.className, this.s.dt.table().body() ).find('td:visible')

I've committed the fix here.

Thanks for letting me know about this one! The nightly will be up to date with the change in a few minutes.

Allan

setwebmaster

@allan correction, it doesn't seem to fix the issue for me...

As the columns in the "rowgroups" are still visible, so they all get the colspan set to the full table width....

If I understand correctly, it was added to support "multi-level" row groups. The problem here is that it seems to take for granted that the "sub-group rows" will essentially only have 1 <td> element, while it's not the case here especially when using custom startRender function and only 1 level of row group

setwebmaster

A way I see to fix it is to loop in the <td> elements and then subtract the colspan of each element to the _colspan() result, so that only the last td element gets the remainder applied... This is not efficient though so there may be a better solution, I'll await your opinion on that matter.

While at it, is there a reason to have 2 event listeners on the responsive-resize event (in the constructor)?

setwebmaster

@allan Here is a spin on a potential fix (not efficient as it loops manually over each group row and dynamically computes the colspan of each column). At least it reuses the computed this._colspan() computation result which is more efficient than current version, which recomputes it for each td element in the collection

See the overwritten _adjustColspan function in this JS Bin.

The only case not supported with this is when there are more columns defined in the row groups than in the actual table. For example if you make the table smaller than 4 columns, there will be an overflow as the columns defined span at least 4 columns.

allan

Sorry - I haven't fully understood the issue before, but I see it now. You are using a custom renderer to create a grouping row which contains td elements and needs a custom colspan.

This is actually going to be surprisingly difficult to solve properly I think. Consider for example a case where you might have three columns in your grouping header, each with a different colspan on them. The calculation for the colspan would be dependent directly on the layout. The only two ways I can see to do that would be to apply specific classes to the host row's cells and count them, or to provide a custom callback for when the colspan adjust is called. I'm currently preferring the section option...

While at it, is there a reason to have 2 event listeners on the responsive-resize event (in the constructor)?

No - that is an error - thanks! Removed the second one now.

Allan

setwebmaster

@allan what do you think about my proposal? It certainly does not cover 100% of the cases, but at least provides a partial fix to the issue.

allan

Yes, your solution is a nice workaround for this use case. I think that since you've got a solution that can work externally (overriding the prototype function for the colspan count) we'll go with that just now, and I've filed an internal bug (DD-1384 for my reference) to get that added.

This way we won't be shipping a feature that will then change (depending on when this gets implemented).

Regards,
Allan