bootstrap-select icon indicating copy to clipboard operation
bootstrap-select copied to clipboard

"Select All" ignores data-max-options

Open favaw opened this issue 6 years ago • 9 comments

Clicking "Select All" (available by setting data-actions-box="true") does not take the data-max-options setting into account. Tested with the latest version of bootstrap-select (v1.13.10) as of today.

See: https://plnkr.co/edit/lV6WEAZpx7V4bQMfYCyT?p=preview

Steps to reproduce:

  1. Click on the dropdown element and try to select more than three options.
  2. Works as expected. Selecting the fourth option shows a "Limit reached" message.
  3. Click on "Select All".
  4. Bypasses the limit and selects the other two options.

It's counterintuitive that you can bypass the data-max-options setting by simply clicking "Select All". Clicking "Select All" should select no more options than data-max-options.

The responsible source code can be found in bootstrap-select.js; the function's name is changeAll.

Consider the alternative implementation shown below. It, on the other hand, does consider the global data-max-options setting and those of optional optgroups:

changeAll: function (status) { 
      if (!this.multiple) return;
      if (typeof status === 'undefined') status = true;

      var element = this.$element[0],
          previousSelected = 0,
          currentSelected = 0,
          prevValue = getSelectValues(element);

      element.classList.add('bs-select-hidden');
      
      var $options = this.$element.find('option'),
          maxOptions = this.options.maxOptions;
      
      for (var i = 0, len = this.selectpicker.current.elements.length; i < len; i++) {
        var liData = this.selectpicker.current.data[i],
            option = liData.option,
            $option = $(option),
            $optgroup = $option.parent('optgroup'),
            maxOptionsGrp = $optgroup.data('maxOptions') || false;
            
        if (maxOptions !== false || maxOptionsGrp !== false) {
              var maxReached = maxOptions == $options.filter(':selected').length,
                  maxReachedGrp = maxOptionsGrp == $optgroup.find('option:selected').length;

              if ((maxOptions && maxReached && status) || (maxOptionsGrp && maxReachedGrp && status)) {
                  continue;
              }
        }

        if (option && !liData.disabled && liData.type !== 'divider') {
          if (liData.selected) previousSelected++;
          option.selected = status;
          if (status) currentSelected++;
        }
      }

      element.classList.remove('bs-select-hidden');

      if (previousSelected === currentSelected) return;

      this.setOptionStatus();

      changedArguments = [null, null, prevValue];

      this.$element
        .triggerNative('change');
    }

See: https://plnkr.co/edit/RZQ5Aig6dn7Vvjla2bqD?p=preview

The above Plunker link includes the alternative implementation. Clicking "Select All" will consider the data-max-options settings - the global one and the one within the optgroup.

favaw avatar May 13 '19 16:05 favaw

You can even significantly speed up "Deselect All" by introducing one more if statement - see below:

[...]

      for (var i = 0, len = this.selectpicker.current.elements.length; i < len; i++) {
        var liData = this.selectpicker.current.data[i],
            option = liData.option,
            $option = $(option);
            
        if (status) {
          var $optgroup = $option.parent('optgroup'),
              maxOptionsGrp = $optgroup.data('maxOptions') || false;
              
          if (maxOptions !== false || maxOptionsGrp !== false) {
            var maxReached = maxOptions == $options.filter(':selected').length,
                maxReachedGrp = maxOptionsGrp == $optgroup.find('option:selected').length;

            if ((maxOptions && maxReached) || (maxOptionsGrp && maxReachedGrp)) {
              continue;
            }
          }
        }

        if (option && !liData.disabled && liData.type !== 'divider') {
          if (liData.selected) previousSelected++;
          option.selected = status;
          if (status) currentSelected++;
        }
      }

[...]

favmd avatar May 20 '19 13:05 favmd

You can even significantly speed up "Select All" by introducing one more if statement - see below:

[...]

        if (status) {
          var $optgroup = $option.parent('optgroup'),
              maxOptionsGrp = $optgroup.data('maxOptions') || false;
              
          if (maxOptions !== false || maxOptionsGrp !== false) {
            var maxReached = maxOptions == $options.filter(':selected').length,
                maxReachedGrp = maxOptionsGrp == $optgroup.find('option:selected').length;

            if (maxOptionsGrp && maxReachedGrp) {
              continue;
            }
            
            if (maxOptions && maxReached) {
              break;
            }
          }
        }

[...]

favaw avatar May 20 '19 18:05 favaw

I'm wondering if "Select All" should simply not even be visible if maxOptions is set, since having "Select All" only select some of the options is confusing (and I'm not sure what the desired use case would be). At the same time, having it select all of the options when maxOptions is set is also confusing (and obviously undesired). So, I think the best course of action would be to only show the "Deselect All" button if maxOptions is set either globally or on an optgroup.

caseyjhol avatar Sep 24 '19 21:09 caseyjhol

IMHO having a "Select Max" button would be the best compromise. Generally speaking, a way to select the maxOptions with just one click is saving us a lot of time and has become a pretty useful feature (image a long list of entries with maxOptions around 50, you would not want to click fifty times).

Reading your roadmap (#2228), it would be great to have something like buttons: ['selectMax', 'disableAll']. And if someone does not want this button, it would be buttons: ['disableAll'].

favmd avatar Sep 30 '19 09:09 favmd

Yes, it could be a time saver, but what would be the use case for wanting to arbitrarily select the first 50 options? If there's a limit on the number of selectable options, presumably the user needs to use some discretion in the options they select. What if we implemented a "shift + click" or "drag + click" feature to allow the user to select multiple consecutive options easily?

caseyjhol avatar Oct 04 '19 17:10 caseyjhol

Usually our workflow/use case is as follows: The user opens up the dropdown, uses a filter (using the search box) to select a subset of all the entries and then clicks the "Select All" button, which, in turn, should still ensure that the maximum number of items is not exceeded (say, due to a bad filter).

The "shift + click" and/or "drag + click" idea would still be a cool feature to have, but for non-technical users a "Filter" -> "Select Max" might be a more intuitive option and easier to explain/remember. Besides that, even the "shift + click"/"drag + click" feature should ensure that maxOptions is not exceeded.

favmd avatar Oct 07 '19 08:10 favmd

To follow on to favmd's input, what if you just didn't allow select all to function if more than the max is available to be selected? Throw an error to the user saying too many options available, only x options may be selected.

RebelTank avatar Jan 07 '21 15:01 RebelTank

+1. It would appear it is a 'Select all visible' is required which is a useful feature.

AzzaAzza69 avatar Sep 26 '21 07:09 AzzaAzza69

+1 Is this issue still not resolved? Just came across it when SELECT ALL did not act as expected.

If SELECT MAX works for some that's fine and SELECT ALL is in either case not a good solution in combination with MAX OPTIONS. If a SELECT MAX works for some, then that's fine, I can't see how that could be relevant at glance, at least that would be pointless in my case (list of a hundred options where up to 3 can be selected, but no random selection would make sense).

I'd suggest that until a good approach can be found or agreed (this still being OPEN was a bit of a surprise), that you disable the SELECT ALL, but still allows the DESELECT ALL to remain - in my case that would a very relevant way to quickly remove the 3 selected items. Optimally and potentially they likely should be separated, so that DESELECT ALL can be a standalone option without the SELECT X one and vice versa.

My solution till this can be resolved (just hiding the one button CSS wise seems like a bad option as we have a limit for a reason)... so I guess a JS hack to move the Selected to the top of the list is my best option. That's also yet to become an official setting for a SELECTPICKER right? That's really beyond me, should most certainly be a simple setting.

Organizer21 avatar Feb 13 '23 21:02 Organizer21