[Table] Fix deselect all logic

[vividh]

• PR has tests / docs demo, and is linted.
• Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
• Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Aware of #3980 and #4588. Since #2251 talks about implementing a completely new Table component set, and no clear ETA of a fix for Table in 0.16.0, this could fix major woes with the existing Table component and hopefully solve many users' issues.

• Removes logic that selects last row when deselecting all rows (Was this intentional?)
• Fixes logic where clicking on any row (when all rows are selected) selects the last row (related to first issue)
• Fixes [Table] controlled table bug #5192. Also fixes Datatable selectall problem #5234.

Side Note: selectedRows should be maintained by Table, and should be passed as prop to TableBody. Benefits of doing this:

• allItemsSelected checkbox in TableHeader can be updated even if all rows are selected without using the selectAll checkbox
• the dirty length check in bd8f70d won't be required since TableBody won't be able to mutate selectedRows internally.

[oliviertassinari]

The Table component was partially / fully migrated on the next branch.
We are now focusing on releasing out the next branch.
We plan to stop the support of the migrated components.
I'm gonna close this PR. Thanks for your time.
Would be great if you check out the new component. Any PR is welcomed.

[NZhuravlev]

Why is that closed? Today is 13th December and the bug is still there. What is the downside of merging this PR?

[oliviertassinari]

What is the downside of merging this PR?

@NZhuravlev I don't have much context on this component. That was simply too time-consuming for me to review it. It would be much better with unit tests to make sure about the expected behavior.

Anyone from @callemall/material-ui wants to have a look at it? Two people interested in the same change has a good noise vs signal rate :).

[oliviertassinari]

Digging into that issue, it seems that the following piece of logic is coming out of nowhere.

      this.setState({
        selectedRows: this.state.selectedRows.length > 0 ?
          [this.state.selectedRows[this.state.selectedRows.length - 1]] : [],
      });

However, I'm wondering, why not changing that logic for the following one: #4588? @umidbekkarimov any light 💡 on why you closed your PR? Thanks

[avocadowastaken]

@oliviertassinari not sure why (yeah i should have leave a note) but i think there was another problems with state management, so I decided to use my own implementation for checkboxes with more control instead of native one

[ovaldi]

@oliviertassinari I have the same issue, and I have created a PR for this, can you please merge this PR, thank you!!

#5829

[vividh]

Glad to see this finally get fixed, although would have preferred for this PR to be merged since it fixes some other issues through e169858.

Is there some Open Source Guideline to merge the latest PR for the same issue, or the fix in e169858 just wasn't required?

[oliviertassinari]

I though both PR are fixing the same issue. I took the one that looks simpler.
What's the extra logic for?

[vividh]

The other change is for

Fixes logic where clicking on any row (when all rows are selected) selects the last row (related to first issue)

With this change, clicking on a row when all rows are selected (using select all), that row gets de-selected, which I believe is the expected behaviour. I'm not sure if merging #5829 fixed this issue as well, as I am away from a laptop. If it did, feel free to skip over this. Thanks!

[oliviertassinari]

@vividh That's not the current behavior, but I agree. That should be the default behavior.
Trying your PR, that doesn't seem to implement that behavior. Feel free to submit a new PR with the corresponding tests if you figure a solution. Thanks :).