Possible bug: Using tables.rows() with RowGroup extension

Loren Maxwell

I'm working on a DataTables/Leaflet integration and I've noticed that when RowGroup is included then something seems to foul up the table.rows() function.

Here are two pages, the first does not have the RowGroup extension and it places markers on the Leadlet map just as I had expected:
http://ghsfha.org/ts_2.html

Here's the second page, the only difference aside from the header is that RowGroup is included:
http://ghsfha.org/ts_3.html

Now 1) the markers do not show up, 1) the console.log() point in the update function is never reach, and 3) a "TypeError: a is null" for datatables.min.js:197:57 is thrown.

When the any of the tables are redrawn it triggers the update function for that DataTable-Leaflet pair: "update_map_region_standings_GHSA_AAAAAAA_X" where "X" is the pairing:

.on("draw", () => {
    if (table_region_standings_GHSA_AAAAAAA_1_draw) {
        table_region_standings_GHSA_AAAAAAA_1_draw = false;
    } else {
        update_map_region_standings_GHSA_AAAAAAA_1(table_region_standings_GHSA_AAAAAAA_1);
    }
});

The update_map_region_standings_GHSA_AAAAAAA_1(table) function uses tables.rows() to cycle through the rows to get the coordinates and add them to Leaflet, but again, when RowGroup is included it doesn't work.

Is this a bug with RowGroup or should I be doing something different to cycle through the rows?

Loren Maxwell

I've gone back and used the non-minified version of the CDN and found probably a more helpful error:
TypeError: display is null
dataTables.rowGroup.js:249:8

Here's the specific code from dataTables.rowGroup.js, where the error is now line 12:

    /**
     * Take a rendered value from an end user and make it suitable for display
     * as a row, by wrapping it in a row, or detecting that it is a row.
     * @param [node|jQuery|string] display Display value
     * @param [string] className Class to add to the row
     * @private
     */
    _rowWrap: function ( display, className )
    {
        var row;
        
        if ( typeof display === 'object' && display.nodeName && display.nodeName.toLowerCase() === 'tr') {
            row = $(display);
        }
        else if (display instanceof $ && display.length && display[0].nodeName.toLowerCase() === 'tr') {
            row = display;
        }
        else {
            row = $('<tr/>')
                .append(
                    $('<td/>')
                        .attr( 'colspan', this._colspan() )
                        .append( display  )
                );
        }

        return row
            .addClass( this.c.className )
            .addClass( className );
    }
} );

allan

Hi,

This is something I think should be fixed in RowGroup, although I'm not immediately certain what the correct fix should be - let me explain.

You have:

                "rowGroup": {
                    "dataSrc": "sub_region"
                },

But in the data loaded:

sub_region: null

So the grouping is being done on a null value. The question then is how should that be displayed - perhaps "No group", or "null" or an empty space?

Either way, that is the root of the issue. Did you expect to be grouping on null data?

Allan

Loren Maxwell

Ah, ok.

In the database sub_region can be null since not all regions are divided into subregions.

Since I show the teams by region, using RowGroup is a convenient way to divde them up, like here:
http://ghsfha.org/w/Special:SeasonHome?view=regions&season=1978

Notice how most, but not all, of the regions have subregions. I also like that RowGroup does not insert a row if there are no subgroups.

I'm not sure what RowGroup should display by default. Off hand I would say if there are no other subgroups then it should be hidden, but if there are then a blank row should be shown.

How about an option for the user to put in, such as "onNull": "", "onNull": "[NULL]", or even "onNull": none or "onNull": hide?

Any thoughts on an interim fix?

Loren Maxwell

OK, having showered I had a slightly different though on the user option part.

I think it should look like:

"rowGroup": {
    "dataSrc": "sub_region",
    "showOnNull": true // false by default
},

OR

"rowGroup": {
    "dataSrc": "sub_region",
    "showOnNull": "No category"// false by default
},

"showOnNull" should be either a Boolean or a text value.

Loren Maxwell

OK, thinking about it some more on the drive to work . . .

The "showOnNull": false wouldn't really work because when someone sorts the table it won't be readily apparent when a row has null on the rowgroup, so I think there should just be this option:

"rowGroup": {
    "dataSrc": "sub_region",
    "showOnNull": "No category"// "NULL" by default
},

So "showOnNull" should only be a text value.

allan

Committed here. rowGroup.emptyDataLabel is the name since it will work with undefined as well as null.

Thanks for the suggestion and feedback!

Allan

Loren Maxwell

Allan, looks great and I look forward to it making it into the next release.

Another request would be for an option to simply hide the group if all the rows are the same.

Not sure that would suit everyone's purpose, but it would mine.

Something like:

"rowGroup": {
    "dataSrc": "sub_region",
    "emptyDataLabel ": "No category"// "No group" by default,
    "hideIfUniversal": true // false by default,
}

allan

Not a bad idea that. I'll wait to see if anyone else likes or suggests at as well before adding. The downside that I can see is that you would be losing context information which might be important.

Allan

Loren Maxwell

The downside that I can see is that you would be losing context information which might be important

I agree that's probably the dominate case, but not always.

In my case, no one assumes regions are subdivided, but RowGroup gives me the ability to highlight the times that they are without having to point out the time that they aren't.

At least the option would allow me the chance to decide.