CheckboxSelectColumn, GroupItemMetadataProvider and DataView with grouping bugs

[igayevoy]

Some bugs that I've found:

1. When collapsing expanded group that have selected rows then the next rows are marked and styled as selected.
2. When collapsing or expanding selected group then it becomes unselected.
3. When all groups are collapsed the selection of all rows with the checkbox in header or selection of all rows in group does not marks and stylizes rows as selected.

They are reproduces on example page http://6pac.github.io/SlickGrid/examples/example-grouping-checkbox-row-select.html

[hakancunier]

Yes. I am facing same problem. +1

[ferman2147]

I couldn't reproduce the bug in Number 1 And on Number 2 rows still remain selected while the group header checkbox becomes unselected but Number 3 definitely looks like it needs a workaround...

[igayevoy]

After selecting a group:

After collapsing the selected group:

[ugurakca]

Yes exactly, captures are very clear about the issue. That's the point, collapsing and expending is broking the whole grid.

[hakancunier]

And also unless checkboxes same bug occurs as below;

[6pac]

Hi all. Will look at this on the weekend. I keep saying it, but I'm just really busy at the moment. I'm also a little surprised no-one has managed to fix it yet.

[hakancunier]

We tried many times and chased for several hours but unfortunately this is really annoying. There were some solutions but we were not sure about the regression and side effects. So we couldnt address a solution.

Also it may be a long-term bug from the original mleibman repository or a design issue.

[6pac]

These should be fixed now (latest release) 2.3.9. Just involved adding

dataView.syncGridSelection(grid, true, true);

after the grid.render(); in dataView.onRowCountChanged and dataView.onRowsChanged

EDIT: actually, it only needs to be added once during grid setup. My mistake. Will patch soon.

Check the wiki under 'Synchronizing selection & cell CSS styles' here.

I am investigating adding the functionality to select and deselect collapsed groups, but not quite there yet.

[hakancunier]

We are waiting for patch according to your edited comment. Thanks for work.

[6pac]

2.3.10 released. Still investigating 'select and deselect collapsed groups', but not quite there yet.

[hakancunier]

Just tried it but still same error occurred.

[6pac]

OK, can you get specific? It's working fine for me in Chrome. What browser are you using?

Here's the example: http://6pac.github.io/SlickGrid/examples/example-grouping-checkbox-row-select.html
Will add a link for it in the wiki example page.

[hakancunier]

Hello. Do we need to change dataview as you mentioned before or wait since "EDIT: actually, it only needs to be added once during grid setup. My mistake. Will patch soon."

[6pac]

No changes to core code. Just a one line addition to the example page itself.

When I mentioned dataView.onRowCountChanged and dataView.onRowsChanged, that was in the context of the dataview events that are handled in the example page.

[hakancunier]

Thanks. Adding dataView.onRowCountChanged and dataView.onRowsChanged seems solved the issue. But if there are many rows this really locks the screen. I think we should invoke syncselection in group toggle methods.

[6pac]

You only invoke syncGridSelections once, when you're setting up. It hooks into the dataView.onRowCountChanged and dataView.onRowsChanged events by itself. To get the behaviour I'm after I'm going to have to re-architect the core parts of the sync, hopefully it will also make things a lot faster. It's pretty messy at the moment, with three separate controls and plugins all talking to each other through the grid.

[jadjare]

I'm mid work-project at the moment and hit this issue. I needed to get the behavior fixed, so I have been digging around.

Having investigated the cause it seems to principally be down to two issues:

1. When you click the select box on a collapsed group Slick Grid does not actually select the rows in the group due to them not currently being in the dataview, thus it can't find them to select. The group gets marked as selected, but actually nothing gets selected inside the group, thus when you expand it the selection seems to disappear.

2. Slick Grid provides a Slick.Group class which holds the state of each group, e.g. whether it's collapsed, the rows contained within it and whether it is selected. My guess is that a new instance of this class is created everytime a group is programmatically retrieved and during this process the isSelected flag is not correctly restored and thus always reports as false. This is why after you expand a group and select all its members using the group's checkbox, the group's checkbox subsequently changes to unchecked when it is later collapsed.

In looking to fix the issue I decided not to fork a copy of the code, but rather patch a fix on top of the current implementation.

Below is the code that can be used to patch the issue. It's not perfect, but it seems to work and also adds one or two other benefits.

Although there is a fair bit of javascript I've tried to make it simple to use. In fact it can reduce the number of lines of code required to create a grid with grouping. Thus to use the fix you simply call it like so:

var dataView = Slick.Data.DataView.createDataViewWithGroupMetaItemProvider(
                    { inlineFilters: true, clickingRowTogglesGroup: true },
                    { checkboxSelectPlugin: myCheckboxSelector, checkboxSelect: true, paritalSelectCssClass: "partial" });

... then after you've created the Slick.Grid object and assigned the dataView to it (e.g. var grid = new Slick.Grid('#grid-container', dataView, columns, options);), you'll then need to call syncGridSelection e.g. dataView.syncGridSelection(grid, true, true) as metioned by @6pac ...

That's it!

The createDataViewWithGroupMetaItemProvider method will patch in a fixed version of the provider, it will also intercept your later call to syncGridSelection and automatically register the groupItemMetaDataProvider plugin and then hook into grid events to start tracking group selections.

You can deduce from the code below that it should still be possible to pass in the various setup options for the Data View and GroupItemMetadataProvider. The Data View options are passed in the first argument to the 'createDataViewWithGroupMetaItemProvider' method, whilst the GroupItemMetadataProvider options are passed as the second argument. I've added a couple of extra options, one for the DataView and one for the GroupMetaItemProvider. On the DataView you can set clickingRowTogglesGroup to determine whether or not clicking any where on the row expands/collapsed the group. On the GroupItemMetaProvider you can set paritalSelectCssClass to a css class which will then be used to represent the checkbox state when a group is partially selected.

Here's the code for the fix

Slick.Data.DataView.createDataViewWithGroupMetaItemProvider = function (options, groupItemMetadataProviderOptions) {


    //Override the group item provider so that we can correctly indicate if a group is selected or not, this fixes one of the issues reported here https://github.com/6pac/SlickGrid/issues/165

    var groupItemMetadataProviderDefaults = {
        checkboxSelect: false,
        checkboxSelectCssClass: "slick-group-select-checkbox",
        checkboxSelectPlugin: null,
        groupCssClass: "slick-group",
        groupTitleCssClass: "slick-group-title",
        totalsCssClass: "slick-group-totals",
        groupFocusable: true,
        totalsFocusable: false,
        toggleCssClass: "slick-group-toggle",
        toggleExpandedCssClass: "expanded",
        toggleCollapsedCssClass: "collapsed",
        enableExpandCollapse: true,
        groupFormatter: defaultGroupCellFormatter,
        totalsFormatter: defaultTotalsCellFormatter,
        paritalSelectCssClass: "unchecked" //New Option to allow a specific class to be set when a group is partially selected
    };

    groupItemMetadataProviderOptions = Object.assign(groupItemMetadataProviderDefaults, groupItemMetadataProviderOptions);

    function defaultTotalsCellFormatter(row, cell, value, columnDef, item) {
        return columnDef.groupTotalsFormatter && columnDef.groupTotalsFormatter(item, columnDef) || "";
    }

    //Revised definition of the default Group Cell Formatter ensuring groups are correctly marked as selected
    function defaultGroupCellFormatter(row, cell, value, columnDef, item) {

        var indentation = item.level * 15 + "px";
        var options = groupItemMetadataProviderOptions;

        return (options.checkboxSelect ? '<span class="' + options.checkboxSelectCssClass +
            ' ' + dataView.computedSelectedClass(item.groupingKey, item.count) + '"></span>' : '') +
            "<span class='" + options.toggleCssClass + " " +
            (item.collapsed ? options.toggleCollapsedCssClass : options.toggleExpandedCssClass) +
            "' style='margin-left:" + indentation + "'>" +
            "</span>" +
            "<span class='" + options.groupTitleCssClass + "' level='" + item.level + "'>" +
            item.title +
            "</span>";

    }

    //Create the group meta data provider using the specified options
    var groupItemMetadataProviderWithBugFix = new Slick.Data.GroupItemMetadataProvider(
        groupItemMetadataProviderOptions
    );

    //Create the data view using the specified options
    var defaults = {
        groupItemMetadataProvider: groupItemMetadataProviderWithBugFix,
        inlineFilters: false,
        clickingRowTogglesGroup: false
    };
    options = Object.assign(defaults, options);
    
    var dataView = new Slick.Data.DataView(options);

    //Intercept the setGrouping dataview method so we can add group selection tracking
    var baseSetGrouping = dataView.setGrouping;
    dataView.setGrouping = function setGrouping(groupingInfo) {
        this.groupIsSelected = {};
        this.groupSelectedRowCount = {};
        this.computedSelectedClass = function (groupingKey, groupRowCount) {
            if ((dataView.groupSelectedRowCount[groupingKey] || 0) === 0) return 'unchecked';
            if (dataView.groupSelectedRowCount[groupingKey] === groupRowCount) return 'checked';
            return groupItemMetadataProviderOptions.paritalSelectCssClass;
        };
        baseSetGrouping(groupingInfo);
    };

    //Intercept Sync Grid Selection so we can hook into the necessary grid events to track selection changes
    var baseSyncGridSelection = dataView.syncGridSelection;
    dataView.syncGridSelection = function (grid, preserveHidden, preserveHiddenOnSelectionChange) {

        grid.registerPlugin(options.groupItemMetadataProvider);

        var ignoreSelectedRowsChanged = false;

        //Monitor row selection so we can correctly track group checked states
        grid.onSelectedRowsChanged.subscribe(function (e, args) {
            var item = dataView.getItem(args.row);

            var rowSelectionCanBeIgnored = item instanceof Slick.Group || ignoreSelectedRowsChanged;
            if (rowSelectionCanBeIgnored) return;

            var renderIsRequired = false;
            var groups = dataView.getGroups();

            var selectedRows = [];
            $.each(grid.getSelectedRows(), function (index, row) {
                    selectedRows[grid.getDataItem(row).id] = true;
                });

            //Assess the selected state of each group
            for (var i = 0, groupCount = groups.length; i < groupCount; i++) {
                var group = groups[i];

                var impossibleForSelectionToHaveChanged = group.collapsed;
                if (impossibleForSelectionToHaveChanged) continue;

                var groupSelectedRowCount = 0;
                for (var j = 0, rowCount = group.rows.length; j < rowCount; j++) {
                    if (selectedRows[group.rows[j].id]) groupSelectedRowCount++;
                }

                var previousGroupSelectedRowCount = dataView.groupSelectedRowCount[group.groupingKey];
                dataView.groupSelectedRowCount[group.groupingKey] = groupSelectedRowCount;

                var selectionCountChanged = previousGroupSelectedRowCount !== groupSelectedRowCount;
                var allGroupRowsSelected = groupSelectedRowCount === group.count;
                var checkedStateChanged = (dataView.groupIsSelected[group.groupingKey] || false) !== allGroupRowsSelected;

                if (selectionCountChanged || checkedStateChanged) {
                    dataView.groupIsSelected[group.groupingKey] = allGroupRowsSelected;
                    renderIsRequired = true;
                }
            }

            if (renderIsRequired) {
                grid.invalidateAllRows();
                grid.render();
            }
        });

        //Override the Slick Grid behaviour when selecting items in a group, this fixes the known issue with Slick Grid whereby rows in collapsed groups are not selected/deselected (see https://github.com/6pac/SlickGrid/issues/165)
        grid.onClick.subscribe(function (e, args) {
            var $target = $(e.target);
            var item = dataView.getItem(args.row);

            //Only handle collapsed groups, expanded groups are handled by the subscription to the onSelectedRowsChanged event handle above
            if (item && item instanceof Slick.Group && item.collapsed && $target.hasClass("slick-group-select-checkbox")) {
                ignoreSelectedRowsChanged = true;

                var groupingKey = item.groupingKey;

                dataView.groupIsSelected[groupingKey] = !dataView.groupIsSelected[groupingKey];

                //Expand the group and then alter the selected state of its rows
                dataView.expandGroup(groupingKey);

                var selectRowIndicies = [];
                $.each(item.rows, function (index, row) {
                        selectRowIndicies.push(dataView.getRowById(row.id));
                    });

                if (groupItemMetadataProviderOptions.checkboxSelectPlugin && dataView.groupIsSelected[groupingKey]) {
                    groupItemMetadataProviderOptions.checkboxSelectPlugin.selectRows(selectRowIndicies);
                    dataView.groupSelectedRowCount[groupingKey] = item.count;
                } else {
                    groupItemMetadataProviderOptions.checkboxSelectPlugin.deSelectRows(selectRowIndicies);
                    dataView.groupSelectedRowCount[groupingKey] = 0;
                }

                //Return the group to its collapsed state
                dataView.collapseGroup(groupingKey);

                //Re-render the group row
                grid.invalidateRow(args.row);
                grid.render();

                ignoreSelectedRowsChanged = false;
            }
        });

        if (options.clickingRowTogglesGroup) {
            //Allow the user to click the group header, in addition to the [+] [-] box, to expand/collapse the store's grid.
            grid.onClick.subscribe(function (e, args) {
                var toggleGroup = !$(e.target).hasClass("slick-group-select-checkbox") &&
                    $(e.target).closest(".slick-row").hasClass("slick-group");
                if (toggleGroup) {
                    var item = dataView.getItem(args.row);

                    if (item.collapsed) {
                        dataView.expandGroup(item.groupingKey);
                    } else {
                        dataView.collapseGroup(item.groupingKey);
                    }
                }
            });

        }

        //Continue with setting the base sync grid selection options
        baseSyncGridSelection.call(dataView, grid, preserveHidden, preserveHiddenOnSelectionChange);

    };

    return dataView;

};

Finally apologies for the log comment entry. Perhaps should be a blog post!!

Hope this helps.

[6pac]

Thanks for feeding that back. This has turned out to be one of those can-of-worms problems, and I'm way too busy right now to take it on. I'll see if I can test this out and offer it as part of the standard repo.

[rahuledb]

Please let us know when shall this issue will be resolved?

[6pac]

Sorry @rahuledb, this is a complex issue to resolve. I'm not going to get to it soon.

[fjf2002]

Any news here? Still very annoying.

[6pac]

not yet. am a little less busy, so may have time to at least evaluate the above fix. the issue is that the problem is a result of interaction between many components, so fixing it may have inintended side effects. To fix it properly I'm going to have to map out all of the use cases, create tests for all of them, and re-architect all the components for a more sane model - working out the interactions is way too complicated. That said, the above fix may just be a patch that will do for now.