Support for selectableCount to limit count when using selectable: "highlight"

[harbulot]

Hi Oli,

(I wasn't sure if you'd rather discuss in an issue first or see the PR directly, but I'll try to describe the issue in the PR.)

This is a new feature request. The purpose is to separate the logic of being selectable (specifically just "highlight") from the numerical limit (max number of selectable rows).

Here are a couple of live examples:

• version 5.1 without patch,
• with patch adding support for selectableCount.

Essentially, in cases where the selection is made using a checkbox, it can still make sense to use { selectable: "highlight" }.
(I think this use-case can be a bit less confusing sometimes for the user, especially when they have other reasons to click somewhere within the row: e.g. editors or tree collapse/expand symbols to click on also within the row.)

Unless I'm mistaken, at the moment, { selectable: "highlight" } implies that there is no max number of selected rows: you can't use highlight and also have a single row/checkbox selected at most.

What I'm suggesting is an additional selectableCount parameter, which can take a numerical value to limit the count when using selectable: "highlight" (and checkboxes, typically).

When it's not specified, the current behaviour of the selectable option should still apply. (Also selectableCount: true would mean an unlimited number of rows, which is already the default with selectable: "highlight" anyway.)

I hope this makes sense. Thank you!

[olifolkerd]

Hey @harbulot

Sorry for the delay in getting back to you on this one, between the website rebuild and the 5.2 release I've rather had my hands full.

Thanks for submitting a PR, it is always great when a member of the community wants to contribute!

The highlight functionality is mainly used to highlight a row on hover and doesn't directly affect selectability in any way:

"highlight" (default) - rows have the same hover stylings as selectable rows but do not change state when clicked. This is great for when you want to show that a row is clickable but don't want it to be selectable.

Could you explain a little more about your usage case as I don't quite follow what this would be used for?

Im not averse to having a selectableCount option but i feel that it could be confusing when you can normally pass the max number of selectable rows into the selectable option. if you were to add this new option, i would suggest for sake of clarity that all selection row limits draw their value from this option. so setting a certain number of rows to selectable would mean setting the selectable prop to true and then setting a number on selectableCount

I would also be looking to change the name, as i would consider a count to be an action, in this case we are setting the maximum number of selectable rows, so the property name should reflect that.

Cheers

Oli :)

[harbulot]

Hi @olifolkerd, thank you, no worries, I'll try to explain a little better.

What I was trying to achieve was to be able to both:

• be able to click on a row without selecting it, which is what selectable:"highlight" does (or at least not setting true or a number there);
• be able to select rows (all the features coming from the selection module), but using the checkboxes only;
• limit the number of rows that can be selected at the same time.

Without this patch, setting a max number via selectable also forcibly enables row-click selection.

selectable: "highlight" does part of what I was trying to do (disabling row-click selection), but I can't also set a max number of selectable rows this way. (I still want the rows to be selectable, but via checkboxes only.)

• If you try to tick multiple checkboxes in the right-most sample on the non-patched example page, they'll just be added to the selection, without any max limit on the number of selectable rows.
• If you try to tick multiple checkboxes in the right-most sample on the patched example page, there will be at most 1 selected row (selectableCount:1).
• In both cases, clicking on the row doesn't select the row (selection via checkbox only).

The selectable option currently seems to have two purposes:

1. control the ability to select a row by clicking on it.
2. setting a max number of rows that can be selected at the same time.

I'd like to have (2) without (1), which I'd replace with checkbox-based selection only.

Please let me know if this makes more sense (or not :-) ).

---

Im not averse to having a selectableCount option but i feel that it could be confusing when you can normally pass the max number of selectable rows into the selectable option. if you were to add this new option, i would suggest for sake of clarity that all selection row limits draw their value from this option. so setting a certain number of rows to selectable would mean setting the selectable prop to true and then setting a number on selectableCount

You're right, I was just drawing the max value from selectable too as a fallback solution for backwards compatibility. I'm not sure how that would affect other users or how you'd handle that in the release cycle.

I would also be looking to change the name, as i would consider a count to be an action, in this case we are setting the maximum number of selectable rows, so the property name should reflect that.

Sure, that makes sense, maybe maxSelectedRows or maxSelection?

Thank you!

[olifolkerd]

Thanks for the more detailed response that makes sense.

From a UX perspective how would you manage the checkbox elements when the maximum selections has been reached?

In terms of name maybe something like selectableMaxRows to keep the convention of the module name first intact.

There would be no problem with the functionality changing, this would have to be included in a minor release anyway which would come with an upgrade guide explaining to users how to manage it. what i would normally do is add a deprecationCheck function in the module initialize function that looks for any old configuration of the selectable option, maps it to the new configuration and triggers a console warning "DERECATION WARNING" that explains to the user what should happen next. If you have a look at the 5.2 code when i release it this evening you will see this in a few places where the API has changed.

Cheers

Oli :)

[olifolkerd]

Hey @harbulot

Are you still interested in pursuing this, or should i close the PR?

Cheers

Oli :)

[harbulot]

Hi Oli,

Apologies for the delay! I've updated the PR, using selectableMaxRows and a deprecation warning.

I've updated the demo example too.

From a UX perspective how would you manage the checkbox elements when the maximum selections has been reached?

At the moment, it's using the behaviour that was already implemented in selectable when using numeric values (selectableRollingSelection). (I guess it could also make sense to prevent users from making further selections unless they've deselected something first.)

Please let me know if you have further comments, thank you!