Vaadin-grid: _isSelected check doesn't work for paginated displays

[stramel]

The _isSelected function uses this.selectedItems.indexOf(item) > -1 which will always be false when the items replaced during page change. This is because new objects are created and this check is checking by reference.

It would be good to do an actual object comparison here or provide a way to check a certain property for comparison.

[manolo]

IMO, grid should check in the way it's implemented. The grid uses it internally and it's not a public API. If the user wants to check if a copy of the item is selected he has access to the selectedItem array and implement it's own comparison method.
I would vote here not to complicate the element to handle this feature.

[stramel]

@manolo My want for this issue is not to make the function public. I would like for the function to change the check from a by reference to deep equality.

so instead of:

this.selectedItems && this.selectedItems.indexOf(item) > -1;

do an object equivalency check, something like https://www.npmjs.com/package/deep-eql

Without a deep object comparison this function essentially is useless when a user paginates the grid.

Maybe you could offer a way for users to opt-in to using that check since it is a heavier check?

[manolo]

I'm still not convinced that comparing all properties of two objects is what we want here.
Actually the user could have some kind of crud view bounded to the array, and need to know where the edited item is in the list, even after he modified.
Do IMHO, user is responsible to make copies of their objects, and maintain custom comparisons.

BTW, deep-eql is 417 sloc I'd rather not to introduce such complexity to our element

[tomivirkki]

provide a way to check a certain property for comparison.

We’ve previously discussed adding API for defining the path to item sub-property that, if provided, would be used for the item's identity everywhere instead of reference comparison.

Something like <vaadin-grid item-id-path=“user.id”>

This is not on the roadmap currently but would be a useful addition.

[tomivirkki]

itemIdPath was added as part of hierarchy support. The feature is already available in 4.1.0-alpha1 release.