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

Multi-select retains first item when clear_button plugin is enabled #724

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GottemHams
Copy link

When you've enabled the clear_button plugin and have a multi-select dropdown that allows you to "activate" the items (i.e. not using the no_active_items plugin), pressing the keyboard shortcut for selecting everything (Ctrl+A, Cmd+A, etc) followed by backspace results in the first item in the list being retained. The list appears empty visually but the clear button is still visible while it should only appear when there are selected items, plus the first item is not selectable anymore (due to using the default setting for hideSelected).

The problem does not occur if you actually use the clear button, or if you remove all the items one by one (by simply holding backspace for example).

This seems to be due these pieces of code (added some comments for clarification):

removeItem(item = null, silent) {
    const self = this;
    item = self.getItem(item);
    if (!item) return;
    var i, idx;
    const value = item.dataset.value;

    // Returns an index that is 1 too high, so the last item in the list is
    // not removed until the next call, which was actually intended for
    // removing the first to last item
    i = nodeIndex(item);

    // All items are visually removed...
    item.remove();
    if( item.classList.contains('active') ){
        idx = self.activeItems.indexOf(item);

        // ... but this never removes the first item from the internal list
        // due to everything being already visually cleared before it gets
        // to the appropriate index, so the `getItem()` call at the top will
        // cause it to return early
        self.activeItems.splice(idx, 1);
        removeClasses(item,'active');
    }
    // ....
}

const nodeIndex = (el, amongst) => {
  if (!el) return -1;

  // `amongst` is not set by `removeItem()` so it will simply use the
  // `nodeName`, which is 'DIV' (which the clear button also is)
  amongst = amongst || el.nodeName;
  var i = 0;
  while (el = el.previousElementSibling) {
    if (el.matches(amongst)) {
      i++;
    }
  }
  return i;
};

The clear button simply calls a clear() function instead of removing a range of items one by one, like how it works when you "activate" all the items and press backspace. Removing items one by one targets every item specifically so it's no problem either.

Since you're already in the context of removeItem(), I don't think there will be any side effects by simply having nodeIndex() target the .item class.

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

Successfully merging this pull request may close these issues.

1 participant