IgniteUI / igniteui-angular

Ignite UI for Angular is a complete library of Angular-native, Material-based Angular UI components with the fastest grids and charts, Pivot Grid, Dock Manager, Hierarchical Grid, and more.
https://www.infragistics.com/products/ignite-ui-angular
Other
570 stars 160 forks source link

Grid Row selection removes parts of selection not found in current data) when [de-]selecting a new row #13380

Closed damyanpetev closed 5 days ago

damyanpetev commented 1 year ago

Description

When the data of the Grid is changes (consider all the remote use cases that leverage that mechanic), selection generally remains unchanged. However if the new data set does not contain some of the selected rows (by primary key), those are removed seemingly as the new selection is calculated. So something is not right

Steps to reproduce

  1. Open https://stackblitz.com/edit/knucth-ekywge (forked from @pmoleri's sample)
  2. Select rows 1 and 7
  3. Click the Refresh Data button
  4. Inspect the current selection (7 should still be in grid.selectedRows)
  5. Select row 3
  6. Inspect the current selection

Result

Starting with selection of [1,7] with new data: image after selecting row 3 we end up with [1,3] and 7 is removed (presumably since it's no longer in the current grid data): image The event itself also doesn't include this as removed, so it's not like it was deselected for some reason, more like it's gone inexplicably image

Expected result

If the selection of the grid is maintained through data changes (presumably for remote scenarios?), then parts of it shouldn't be dropped on the next selection operation by the user or should be cleared immediately with the data change, otherwise selectedRows looks out-of-sync/broken. The Row Selection (Grid feature) spec is quite outdated, so can't really count on it to dictate that behavior even if it actually did, so some digging to identify the reasoning of maintaining the selection through data changes might be needed. Almost certain some remote features that purely override strategies leave the grid blissfully unaware of the remote use so that probably justifies the keeping of selected rows until manually cleared. We could always consider enhancements if possible, but at least the state as it is now should be consistent.

Attachments

Attach a sample if available, and screenshots, if applicable.

github-actions[bot] commented 10 months ago

There has been no recent activity and this issue has been marked inactive.

github-actions[bot] commented 7 months ago

There has been no recent activity and this issue has been marked inactive.

github-actions[bot] commented 4 months ago

There has been no recent activity and this issue has been marked inactive.

github-actions[bot] commented 2 months ago

There has been no recent activity and this issue has been marked inactive.

github-actions[bot] commented 3 weeks ago

There has been no recent activity and this issue has been marked inactive.

ddaribo commented 1 week ago

Looks like this has been indirectly addressed with the changes introduced by this fix. This is no longer reproduced and the data record with ID ‘7’ remains in the selection after changing the data. Here is the same sample with 18.x. Selection is also correctly persisted on switching data chunks in a remote virtualization scenario (sample)

fixed

damyanpetev commented 5 days ago

Looks like this has been indirectly addressed with the changes introduced by this fix. This is no longer reproduced and the data record with ID ‘7’ remains in the selection after changing the data. Here is the same sample with 18.x. Selection is also correctly persisted on switching data chunks in a remote virtualization scenario (sample) [....]

Indeed, the issue seems fixed after #14257. Basically, the removal of get_all_data(true).filter step did it, though that should've been a hint something more than the sort order changed :) Which leads me to my usual comment - guess what's missing? Yup, the test.

Anyway, while I was testing I landed on the broken Selection dev demo (and a few others due to the same remote service) and started cleaning up so I'll PR that along with a test and this can be closed.