stimulus-checkbox-select-all icon indicating copy to clipboard operation
stimulus-checkbox-select-all copied to clipboard

Add disable indeterminate logic

Open lopez-marc opened this issue 2 years ago • 2 comments

Proposal: Add 'disableIndeterminate' logic

Background

The current Component allows for three states for the "checkbox all": checked, unchecked, and indeterminate. However, some users may want to disallow the indeterminate state, and there's currently no built-in way to do this.

Proposal

To address this need, I propose adding a new boolean value called disableIndeterminate. By default, this property would be set to false, allowing for the usual behavior of the Component.

However, if the user sets disableIndeterminate to true, like follows: data-checkbox-select-all-disable-indeterminate-value="true" the "checkbox all" will no longer have the indeterminate state.


I hope it's okay that I'm submitting a proposal via this pull request. I recently extended the behavior of your component for a project I'm working on, and I believe that my changes could be useful for other users.

I'm looking forward to hearing your thoughts on my proposal.

Best regards,

Marc López

lopez-marc avatar Mar 31 '23 10:03 lopez-marc

Hi! thanks for the PR, the code looks good!

But I'm wondering why we would like to do that? In which scenario the indeterminate state should be disable?

guillaumebriday avatar Dec 22 '23 06:12 guillaumebriday

Hi @guillaumebriday!

First and foremost, I want to thank you for taking the time to review my pull request.

Here a brief summary why I proposed this change:

Current Behavior: As it stands, when the check all checkbox is indeterminate, clicking on it initially clears all selections.

https://github.com/stimulus-components/stimulus-checkbox-select-all/assets/78852823/0835745a-979e-4587-9e65-62a3b9bdb440

Proposed Change: By disabling indeterminate state, immediately selects all checkboxes.

https://github.com/stimulus-components/stimulus-checkbox-select-all/assets/78852823/9a390b75-1b1d-4bb0-9ca7-8b55a846de5c

Reasons for the Change:

  • It my be preferred that the first click selects all checkboxes, rather than clearing them.
  • Depending on the wording of the label, may cause confusion.
  • If there is not styling provided for the indeterminate state, that was our case, it was even more confusing. When we first implemented your component we thought it was a bug.

In our app I extended the component to allow the indeterminate state to be disabled in certain cases by passing the value, and I thought it might be useful to others. But I understand if you don't find it generic enough to be included in the component.

Thank you once again for your time and consideration.

lopez-marc avatar Dec 22 '23 17:12 lopez-marc