Unable to select multiple items with Shift + select when specifying onRowSelect

[joaocenoura]

Expected Behavior

Shift + click to select multiple items should select in between items when onRowsSelect is defined and is changing state.

Current Behavior

Shift + click to select multiple adjacent items doesn't seem to work when setting state in onRowSelect.

Steps to Reproduce

Preparation

• use this example: https://github.com/gregnb/mui-datatables/blob/4f836096a56be93ee1322f3c98cbaca485f0b064/examples/selectable-rows/index.js
• on that example, comment/remove isRowSelectable (lines [93, 98]) - optional, it just makes it easier to see the bug

Steps to reproduce the bug

But notice though, if this.setState({ rowsSelected: allRows.map(row => row.dataIndex) }); on line 63 is commented/removed, it works as intended:

CC: @patorjk as this might be related with #827

Tech	Version
Material-UI	4.7.1
MUI-datatables	2.14.0
React	16.12.0

[gabrielliwerant]

Thanks for reporting, though marking as duplicate because it's a manifestation of state retention issues, which crop up everywhere and have other issues and PRs, not that I think you should have known.

[patorjk]

@joaocenoura I agree with @gabrielliwerant, it's an existing issue due to how the table drops its internal state on re-render.

This PR fixes this issue: #1086. You can see it in action here: https://codesandbox.io/s/reverent-shtern-5ppc8 (this points to that PR with the edits you suggested in your post above).

I've been using a forked version of this lib with these changes for months in prod, so if you need a fix in the near future you can grab that PR.

[joaocenoura]

Awesome guys, thanks for the fast reply and sorry for duplication, I'm kinda new to react and this lib :) I'll use that branch in the meantime.

[joaocenoura]

In case someone bumps into this, I just npm install https://github.com/patorjk/mui-datatables.git#persistent_state --save, did a quick test and seems to be working as expected. Should I close this issue or leave it to you guys?

[gabrielliwerant]

@joaoaguiam Not sure which issue you're referring to, but all work has to through me before it can end up in this repo and there are some issues in the state refactor that I'm working through, so it is not ready yet. It is based on the changes given there, but there are other issues it does not address, and it builds on some logic that is faulty (not the fault of the PR).

[patorjk]

@gabrielliwerant Not sure I agree, as I have seen no issues with those updates. However, it is your repo, so I'll leave it to you. I might suggest adding in that PR and then doing a bigger update for v3, as this issue has been open a really long time. Also, if you're basing it off of my changes, I'd appreciate if you just committed to the branch I submitted. You do a lot of PRs where you continue work from other PRs, which denies people contribution credit for what they did. It feels a little off when you do that.

[gabrielliwerant]

@patorjk

You do a lot of PRs where you continue work from other PRs, which denies people contribution credit for what they did.

This is unfortunate and unintended, as it appears github used to give all credit for a squash to the person that opened the PR (isaacs/github#1303). Whenever possible, if I've been building off of previous work, I've been branching off the prior PR to keep the commits in the history somewhere. I've been unable to do otherwise without write access to someone else's repo. It does seem, however, that github has finally fixed this problem in a first class way, by providing attribution for each author involved in the PR, regardless of who merges it https://github.blog/changelog/2019-12-19-improved-attribution-when-squashing-commits/, so that should now fix this problem. If you prefer, I can continue to update directly in your fork, since you've been gracious enough to give me access, but either way, it does appear that github will now handle the credit appropriately.

I'm also open to providing some form of attribution, perhaps by call out, if anyone feels they've been unfairly left out here. I may put a message like that in the documentation so that people have a way to respond and we can fix that. I'm not about depriving people of credit they deserve, it was just an unfortunately consequence of squash behavior.

[joaocenoura]

@gabrielliwerant re:

Not sure which issue you're referring to

Was referring to this ticket/issue: #1173 if for the purposes of tracking should I click the button Close issue or leave it as is. It's was more of a 'process' question, didn't meant to start a discussion.
Beside, as mentioned before I'm good on my side as I'm using @patorjk fork which solved my problem. Thanks guys

[gabrielliwerant]

@joaocenoura Sure, you can close it.