Codecov Report

Merging #2260 into dev will decrease coverage by 0.49%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##              dev    #2260     +/-   ##
=========================================
- Coverage   67.43%   66.94%   -0.5%     
=========================================
  Files         160      160             
  Lines        3062     3107     +45     
  Branches      841      857     +16     
=========================================
+ Hits         2065     2080     +15     
- Misses        734      756     +22     
- Partials      263      271      +8

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ded7679...918d1a9. Read the comment docs.

I found one small bug with respect to filtering. If the table was filtered and the filter criteria changed (and table is still in the filtered state), the selection wasn't cleared. I've added in one extra check for that situation.

So I think all is good now.

Good catch for the filter bug. Found one more: if the items change (for example, user deletes some items), the selection should be cleared too, so we need to add a clear in the "items" watcher. Not sure though if I have to open a new PR for that?

Also, what do you think of renaming (or add synonyms) "single" > "radio" and "multi" > "checkbox", I guess that better convey the fonction?

Yeah, open a new PR... I think we can stick with single and multi... is there any way to merge multi and range modes?

Would just watching for changes on computedItems be enough to trigger the clearSelected? The array ref is new each time sorting, pagination or filtering run.

Although changes to row details would trigger new values, and would clear the selections if people show/hide the details.

yes, tried that already and show/hide details trigger the watch, not desired

Would almost need to use a loose equals, and filter each row properties, and then compare row by row. Would add a bit of overhead though.

what do you mean with "merging multi and range modes", getting rid of the range mode?

Although people should be deleting rows from a copy, and then replace items with a new array, rather than manipulating the local items array directly.

May have to use the computedItems watcher initially, and just list a caveat that showing and hiding details will clear the selections.

Yeah, multi and range modes could be merged... I was playing with a multi-select select element and it supports shift click, and individual click (select/clear).

TBH, i mimicked the modes from datatables, I used it since a long time, and I guess that if it is good since many years, it can't really be wrong...

But for now, we could leave all three modes as is, but later consider merging range and multi into just multi

Just was thinking that multi could emulate how native select inputs work. Although maybe it is different on different OS/browsers (was playing on windows with chrome)

well, that is the way range mode works, similar to OS selection.

Personally, I would like to be able to click to select and clear individual items, and use click, followed by shift click to select and arbitrary range, although not sure about range clearing, if that would be weird for users...

multi is designed to work the way checkboxes work, click toggles (hence my suggestion of synonyms before), more intuitive than range, but less powerfull because range works like single by default, like multi when used with CTRL and add range selection with SHIFT, they really are 3 different beasts

Synonyms might be handy initially..

But I think the main thing is to deal with things like item deletion/adding and clearing the selected array

so, that is range mode

Personally, I would like to be able to click to select and clear individual items, and use click, followed by shift click to select and arbitrary range, although not sure about range clearing, if that would be weird for users...

yes bugs first, I'll have to do a couple of check with remote data too, then I'll open a new PR for bugs

What do you think about storing the actual selected items in data, and then watching it for changes to trigger the emit row-selected?

Watching the prop items might be a good start. And also could clear the selection when the provider runs... or could watch localItems for changes and trigger the clear too..

that was my first take on that thing, but that was not satisfyng because:

• it mutates data
• is cleared with remote data
• is less performant with big arrays
• maybe other things I forgot...

Although, localItems and items changes typically would be things that would cause computedzitems to change.

I do have code that I was going to use for filter data change detection that filters out the item props using reserved property names, and then does a row by row loose equals compare until the first different row is found, and trigger an event. It would only need to fun against computed items, which will be only as large as the pagination size.. it's not overly fast, as when no changes happen it has to scan all the computedItems, but may be a solution.

Only time it would have issues is if there are duplicate rows without unique values.

I'll post the code for the loose equal watcher here tomorrow.. just getting ready to head to bed for the night.

You can take a look at it and see if we can just use it on the computedItems watcher to detect when to clear the selection. We could make the check conditional too, so that it doesn't run if the table is not in selectable mode.

ok, gn (I just wake up myself)