fix(module: table): Incorrect deselection in virtualized table

[rhodon-jargon]

🤔 This is a ...

• New feature
• Bug fix
• Site / documentation update
• Demo update
• Component style update
• Bundle size optimization
• Performance optimization
• Refactoring
• Code style optimization
• Test Case
• Branch merge
• Other (about what?)

🔗 Related issue link

💡 Background and solution

If you selected a row in a virtualized table with the 'radio' selection type, scrolled down enough that the original row was unloaded, and selected a new row, the old row would not be deselected.

It happened because only the loaded rows were changed. That is now fixed by always deselecting all rowdata, even if unloaded, when selecting a row in 'radio' type.

Also cleaned up the selection code a bit (though it's still a bit cluttered), e.g. removing _allRowDataCache as it was not possible for it to contain multiple rowdatas per item, meaning it was the same as _dataSourceCache.

Added some extra overloads of SetSelection, I think it would be most ideal to remove the Selection.Key property (and thus the string overload of Table.SetSelection) in the future, maybe somehow giving it access to the TItem of the row so it can call the SetSelection(TItem) overload (in addition to some other things it would make easier).

📝 Changelog

Language	Changelog
🇺🇸 English	A 'radio' Selection column will now correctly deselect all other items in a table with EnableVirtualization.
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

• Doc is updated/provided or not needed
• Demo is updated/provided or not needed
• Changelog is provided or not needed

[github-actions]

[ElderJames]

Thank you for contribution @rhodon-jargon . I think #3284 is also related the selection, I wonder if we can repair it together?

[ElderJames]

Hello @rhodon-jargon , I found the select all doesn't work now.

[rhodon-jargon]

I'm looking into it. Regarding #3284, I'm not sure if that's related, though it seems like a very weird bug. If I have time, I'll look into it as well

[rhodon-jargon]

@ElderJames the issue should now be fixed. I also discovered that I broke the tree functionality by deleting _allRowDataCache. I refactored some things, like splitting RowData into a RowData and TableDataItem (one RowData for each different row, one TableDataItem for each different item), but it should be mostly compatible with existing code. I think it's also a lot clearer now what is happening in most of the selection code.

Also I think #3284's issue was found by zxyao145, and not really a bug.

[codecov]

Codecov Report

Attention: 144 lines in your changes are missing coverage. Please review.

Comparison is base (f469aea) 45.39% compared to head (d28ada8) 43.61%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3282      +/-   ##
==========================================
- Coverage   45.39%   43.61%   -1.78%     
==========================================
  Files         560      560              
  Lines       26851    26839      -12     
  Branches      268     5453    +5185     
==========================================
- Hits        12190    11707     -483     
+ Misses      14621    14175     -446     
- Partials       40      957     +917     

Files	Coverage Δ
components/table/Column.razor	0.00% <0.00%> (ø)
components/table/ColumnBase.cs	0.00% <0.00%> (ø)
components/table/PropertyColumn.cs	0.00% <0.00%> (ø)
components/table/ActionColumn.razor	0.00% <0.00%> (ø)
components/table/Table.razor.RowData.cs	0.00% <0.00%> (ø)
components/table/TableRowWrapper.razor	0.00% <0.00%> (ø)
components/table/Selection.razor.cs	0.00% <0.00%> (ø)
components/table/Table.razor.cs	0.00% <0.00%> (ø)
components/table/Table.razor	0.00% <0.00%> (ø)
components/table/TableModels/RowData.cs	0.00% <0.00%> (ø)
... and 1 more

... and 146 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ElderJames]

Can't we merge rowdata with tabledataitem? I think they're almost one to one.

[rhodon-jargon]

No, or at least I don't see a way to do it, other than the previous way it was done. (With _allRowDataCache, a list of all RowData's for a Data, which was less efficient and in my opinion more confusing than the current approach.)
The problem is that for tables where the same Data value occurs multiple times (e.g. the circular reference example), expanding a row should only expand that specific row, while selecting a row should select all rows with the same Data.

Therefore, we should keep track of those two in a different way. You are right in that they are almost one to one, because they are in almost all cases. The only case in which they are not is when there is circularly nested data.
(Or at least when the same Data is displayed multiple times, which may also occur if for example multiple other Datas have the same Data as child. But it only forms a problem when the Data is circular, because if you haven't kept track of expansion separately then (once you expand the first few rows) the rendering will enter an infinite loop, expanding the child rows immediately)

I originally thought that _allRowDataCache was redundant for the same reason, because it was in almost all cases. In my opinion this new approach is slightly easier to understand, though maybe even more more documentation is necessary.

What I think is also better in the new approach, is that it is now impossible for only one of the rows with the same Data to be selected, while previously that had to be done 'manually' by looping through _allRowDataCache every time a row was selected. That last approach would be easy to forget or to make mistakes with in future code changes, and the fact that it's only relevant with duplicate Datas makes it so that a mistake might not be noticed right away.

[ElderJames]

Hello @rhodon-jargon , I'm reviewing this PR again and find another issue that when I select some rows, and remove then by changing SelectedRows outside, it work incorrectly and make the memory explode.

[rhodon-jargon]

@ElderJames Good find! It should be fixed now.

[ElderJames]

Thank you @rhodon-jargon! I also find the issue that selection in virtualization mode would be clear while the table fetching additional data. I think we need to keep the state inside the table and retore after reload the same data.

[ElderJames]

LGTM. Thanks!