Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Select All" ignores data-max-options #2277

Open
favaw opened this issue May 13, 2019 · 9 comments
Open

"Select All" ignores data-max-options #2277

favaw opened this issue May 13, 2019 · 9 comments

Comments

@favaw
Copy link

favaw commented May 13, 2019

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.

@favmd
Copy link

favmd commented May 20, 2019

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++;
        }
      }

[...]

@favaw
Copy link
Author

favaw commented May 20, 2019

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;
            }
          }
        }

[...]

@caseyjhol
Copy link
Member

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.

@favmd
Copy link

favmd commented Sep 30, 2019

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'].

@caseyjhol
Copy link
Member

caseyjhol commented Oct 4, 2019

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?

@favmd
Copy link

favmd commented Oct 7, 2019

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.

@RebelTank
Copy link

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.

@AzzaAzza69
Copy link

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

@Organizer21
Copy link

Organizer21 commented Feb 13, 2023

+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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants