GridStateService always clear row selections even though syncGridSelection is TRUE

[whtan98]

I'm submitting a Bug report

Your Environment

Software	Version(s)
Angular	8
Angular-Slickgrid	2.x

Context

this.gridOptions = {
      enableFiltering: true,
      enableGrouping : true,
      enableCheckboxSelector: true,
      enableCellNavigation : false,
      dataView : {syncGridSelection : true}
    };

Setting syncGridSelection to TRUE does not preserve the row selections whenever I change the column filter. See #191

Expected Behavior

It should preserve the row selections if syncGridSelection is TRUE

Possible Solution

Probably can consider changing gridState.service.ts to something like below:

  // Check whether the row selection needs to be preserved
  private needToPreserveRowSelection(): boolean {
    let preservedRowSelection = false;
    if (this._gridOptions && this._gridOptions.dataView && this._gridOptions.dataView.hasOwnProperty('syncGridSelection')) {

      const syncGridSelection = this._gridOptions.dataView.syncGridSelection;
      if (typeof syncGridSelection === 'boolean') {
        preservedRowSelection = <boolean>this._gridOptions.dataView.syncGridSelection;
      } else {
        preservedRowSelection = syncGridSelection.preserveHidden;
      }
    }
    return preservedRowSelection;
  }

  /** if we use Row Selection or the Checkbox Selector, we need to reset any selection */
  resetRowSelection() {
    if (this._gridOptions.enableRowSelection || this._gridOptions.enableCheckboxSelector) {
      // this also requires the Row Selection Model to be registered as well
      const rowSelectionExtension = this.extensionService && this.extensionService.getExtensionByName && this.extensionService.getExtensionByName(ExtensionName.rowSelection);
      if (!this.needToPreserveRowSelection() && rowSelectionExtension && rowSelectionExtension.instance) {
        this._grid.setSelectedRows([]);
      }
    }
  }

Code Sample

[ghiscoding]

The row selection is not, and will not, be part of the Grid State & Presets feature simply because there is no guarantee that the data will always be the same and the row selection is by row index and has no connection with the data itself. The Grid State/Presets is only meant for Grid Filtering, Sorting & Column Position, nothing else. I do not want to change that.

Basically if you want to keep the row selection, you'll have to do it manually, which is not that hard to do with (sgOnSelectedRowsChanged), then keep it in local storage or somewhere else.

[whtan98]

Actually I'm just trying to make use of the readily available feature in 6pac/SlickGrid as described in

https://github.com/6pac/SlickGrid/wiki/DataView#synchronizing-selection--cell-css-styles

Thanks for the feedback. Will explore further

[ghiscoding]

For any features or special code that is not directly available in Angular-Slickgrid, you can use the SlickGrid Grid Object and DataView Object as shown in this Wiki. These objects were exposed for that reason. The main difference with the core lib, is that I use only the DataView in my lib (so manipulating the data as to be with the DataView), so that is something to keep in mind. The one place I play with CSS styling is for highlighting a row, with this method, I use the row metadata as described in this SO. Kind of complicate with metadata just to highlight a row, but it works.

[timhawkins1]

I'm experiencing the same problem, and I don't understand how the suggested solution of handling (sgOnSelectedRowsChanged) can work, because when the grid is filtered or sorted, the (sgOnSelectedRowsChanged) event is triggered with rows equal to an empty array (because of the call to resetRowSelection in gridState.service.ts). If I have to keep track of the row selection manually, I'm not sure how to distinguish between the effect of resetRowSelection being called and the situation where the user intentionally clears all selected rows.

[ghiscoding]

I'm going to repeat again that it's by design, I never intended to add the Row Selection in the Grid State, it doesn't belong there and I won't add it. The reason is the same as I mentioned earlier

The row selection is not, and will not, be part of the Grid State & Presets feature simply because there is no guarantee that the data will always be the same and the row selection is by row index and has no connection with the data itself. The Grid State/Presets is only meant for Grid Filtering, Sorting & Column Position, nothing else. I do not want to change that.

[whtan98]

I'm experiencing the same problem, and I don't understand how the suggested solution of handling (sgOnSelectedRowsChanged) can work, because when the grid is filtered or sorted, the (sgOnSelectedRowsChanged) event is triggered with rows equal to an empty array (because of the call to resetRowSelection in gridState.service.ts). If I have to keep track of the row selection manually, I'm not sure how to distinguish between the effect of resetRowSelection being called and the situation where the user intentionally clears all selected rows.

@timhawkins1 Instead of using the built-in checkbox, I solved my problem using a checkbox column & process the onCellClick event & save the selection state into the row item. No more idiosyncrasy due to sorting/filtering

[timhawkins1]

@whtan98 Thanks for the help. I have it working now in the way that you described. The only thing I couldn't work out how to implement was a header checkbox, so instead I've added a Check/Uncheck All button to my page, which I think is good enough.

[whtan98]

@timhawkins1 You're welcome, I believe you can refer to example
https://ghiscoding.github.io/Angular-Slickgrid/#/headerbutton
which uses CSS to do the trick

[ghiscoding]

I'm reopening this issue as I recently came across the syncGridSelection method and I now see that I have misunderstood this issue and didn't know much about this method before playing with it. I'm currently working at doing the following 2 things

1. keep row selections after filtering/sorting (the main subject of this issue)
2. adding the row selections in the Grid State & Presets (also mentioned later in this issue).

@whtan98 @timhawkins1

I would like some feedback on the Grid State & Presets, I'm adding a new property rowSelection in the Grid State which follows the interface shown below. Note that there's a big difference between the 2 arrays (read the comment of each below). I would imagine that most people would use the dataContextIds array.

export interface CurrentRowSelection {
  /** Grid Row Indexes, based on the row position in the grid (what we see in the UI) */
  gridRowIndexes?: number[];

  /** Row Selection by the Row Data Context, in other words we select the row by the data object ID */
  dataContextIds?: number[];
}

Demo

The print screen below shows a Grid Presets with an IDs of Tasks with IDs=2 & 6 (the ID is equal to the Task number), these tasks are on the first 2 pages of this grid and the presets I used was

presets: {
  pagination: { pageNumber: 2, pageSize: 5 },
  // row selection, use only the best array for your use case and remove the other one
  // it will always start with the "dataContextIds" array, if nothing is found it will go to "gridRowIndexes"
  rowSelection: { dataContextIds: [2, 6],  /*gridRowIndexes: [2],*/ } 
},

Note on Backend Services

I don't think this would necessarily or always work with backendServiceApi (OData/GraphQL) because this row selections would typically only work with what it has loaded in the DataView since the syncGridSelection is a method from the DataView and we typically load only current page with the Backend Services, so perhaps it would re-select what is in the current page but won't select anything on other pages (since that flushes out the DataView dataset).

[ghiscoding]

Hmm I do see a big problem when using Local Grid Pagination, when calling grid.getSelectedRows() it only returns current page selection, it doesn't know the full selection. We can see this wrong behavior in the print screen I shown in previous post, the rowSelection only has 1 index in both gridRowIndexes and dataContextIds arrays, while it should have 2 since I have a selection the page 1 and page 2.

So basically this Grid State would work properly on a grid without any Pagination, but it's a big issue when using Local Grid Pagination, as shown in Example 10, and now I'm starting to question if I should continue with this and/or maybe disable the Grid State rowSelection, the Grid Presets does work though (as shown in the animated GIF), it's just the Grid State that is an issue.

Possible solution, I see this SO which is the same as my problem, but that requires more work... after some thought that probably won't work correctly either since there's no way to know if the click was to add or remove a selection.

[ghiscoding]

This was more complicated than I imagined (because of possible Local Grid Pagination) but I managed to get it all done. We now have a new Grid State property rowSelection which also work with Pagination, it keeps selected row as well with the use of syncGridSelection and the modification suggested in the original issue.

This is now in place and will be released soon.

[timhawkins1]

I appreciate the effort you've put into this, but I still can't get it to work. I've updated to the latest release of Angular-Slickgrid, but the row selections are still lost whenever I filter or sort the grid. I'm not sure what I need to do to retain them.

Can you provide an example that demonstrates this functionality working? Both grids in Example 10 retain the row selections after changing Page, but not after filtering or sorting.

[ghiscoding]

Example 10 and 16 are the main demos, if that doesn't then I give up, seriously I spent over a week on this and it's quite frustrating to get it all to work together (filter + sort + pagination), it was working at some point, it might the code related to the pagination (that was a lot of code to put in to deal with local grid pagination). What you could try is to remove the pagination and I bet it will work.

I seriously wish that I'd had more contributions on a lib that I spent few thousand of hours on. If you would take a look at, I would greatly appreciate the help. The problem, if it is with Pagination, would be in this function

[ghiscoding]

Nevermind, I found it after doing a quick search to see where the setSelectedRows([]) might be called and it's of course a simple and stupid error, on this line, the check is missing parentheses and because of that it was always going in (because of the || check) when it shouldn't

This

const isSyncGridSelectionEnabled = this.gridStateService && this.gridStateService.needToPreserveRowSelection() || false;
if (!isSyncGridSelectionEnabled && this.gridOptions.enableRowSelection || this.gridOptions.enableCheckboxSelector) {
  this.gridService.setSelectedRows([]);
}

should be this

const isSyncGridSelectionEnabled = this.gridStateService && this.gridStateService.needToPreserveRowSelection() || false;
if (!isSyncGridSelectionEnabled && (this.gridOptions.enableRowSelection || this.gridOptions.enableCheckboxSelector)) {
  this.gridService.setSelectedRows([]);
}

I'll add another Cypress E2E test to make sure that test is also covered, forgot to check that possibility.

[ghiscoding]

... and of course I found yet another problem when using Local Grid Pagination and Filters. The filtered rows don't get removed from the selection array. Yet more complex code to deal with. This is so frustrating 😠 I just shouldn't have added this Local Grid Pagination, it's causing so much complexity everywhere to deal with.

[ghiscoding]

after lot of effort, I think I finally got it all covered (works with/without Pagination too), however I had to add a new property in the Grid State to handle filtered and non-filtered row selections, the new prop is filteredDataContextIds

The output looks like this now, in most cases you'll probably want to use the new filteredDataContextIds since the other dataContextIds has the entire (non-filtered) selection.

I'll push a new version tomorrow and hopefully this is the last time I go over this thing

[ghiscoding]

@timhawkins1
Hopefully this is it, I believe I've fixed every possible scenarios and updated the Example 10 and like I wrote in previous post, you'll probably want to use the new prop filteredDataContextIds, however it's worth to know that the preset itself will use dataContextIds to reload the selection. You can update to latest version 2.17.3

[timhawkins1]

I've tried the latest version, and there still seems to be a problem. The row selection is retained correctly following filtering or sorting if enablePagination is true, but it's not retained correctly if enablePagination is false. I'm not using pagination in my grids, so the row selection isn't working correctly for me.

It's possible to reproduce the problem in Example 10 if you edit the code so that enablePagination is false in gridOptions2. After doing this, apply a filter to the second grid so that none of the selected rows are visible, and then mark one of the visible rows as selected. If you then remove the filter, only the single row that you just selected is still selected, and the rows that were previously selected are no longer selected.

[ghiscoding]

@timhawkins1
Another morning spent on this... it should be fixed now, you can test it in Example 12 where I added the row selection and some more Cypress E2E tests.

I have to say though that next time around I'd like to get a bit more help in troubleshooting and/or testing, even just adding some Cypress E2E tests would have been helpful. This is an Open Source project for a reason, I'd like to have contributions once in a while (at least that was the reason of why we decided to make it Open Source).

[timhawkins1]

I confirm that it's working perfectly now, thank you!

Point taken, I'm not very familiar with your code, but I was doing my best to be helpful by giving instructions to reproduce the problem. I'll try to make more of a contribution next time.