Почему элемент игнорируется?

Question

[Даниил Игоревич]

var ul = document.getElementsByTagName('ul')[0];
ul.onclick = function(event){
  var target = event.target;
  if(target.tagName === 'LI'){
    if(event.ctrlKey || event.metaKey){
      target.classList.toggle('selected');
    } else if(event.shiftKey){

    } else {
      var selected = ul.getElementsByClassName('selected');
      for(var i = 0; i < selected.length; i++){
        selected[i].classList.remove('selected');
      }
    }
  }
}

<ul>
    <li>Кристофер Робин</li>
    <li>Винни-Пух</li>
    <li>Ослик Иа</li>
    <li>Мудрая Сова</li>
    <li>Кролик. Просто кролик.</li>
  </ul>

Как исправить эту проблемы? Игнорируются элементы, а нужно, чтобы у каждого элемента, у которого уже есть класс selected, он пропадал.

1:


2:

Comments

[OVK2015]

А зачем вы обработчик на 'ul' вешаете?

[Даниил Игоревич]

OVK2015: на каждый li вешать? Я повесил один на ul. Если не ошибаюсь, этот приём называется делегированием.

[OVK2015]

Тут не в названии дело. При такой реализации, событие будет возникать для обертки списка. Путаницы добавит когда ветку else if(event.shiftKey) реализовывать будете

[Даниил Игоревич]

OVK2015: а какая путаница? Я сейчас всё сделал, никакой путаницы нет, ну, разве что с выделением текста, но это обсуждается в следующей главе :)

[OVK2015]

Даниил Ораин:
Вот и я о том же. Так сказать: меньше "чернил" и авторского вдохновения потратите :)

Answer 1

[Дмитрий Беляев]

getElementsByClassName возвращает nodeList, который обновляется при изменениях в dom
Допустим у Вас выделено 3 элемента, их индексы в nodeList 0, 1 и 2
Вы перебираете их циклом for(var i = 0; i < selected.length; i++)
На первой итерации цикла Вы убираете класс у 0 элемента, при этом он так же исчезает из nodeList, который сформирован по этому классу, 1й элемент становится нулевым, 2й - первым, length становится 2
На второй итерации Вы убираете класс у элемента с индексом 1, который изначально был 2, nodeList опять обновляется
Третьей итерации уже не происходит, так как i == 2, а selected.length == 1, цикл завершается.
Оптимальнее, как в плане производительности, так и в плане правильной логики перебирать nodeList d обратном порядке, так элемент будет удаляться с конца nodeList - что быстрее, а у необработанных элементов останутся прежние индексы

for(var i = selected.length; i--; ){
        selected[i].classList.remove('selected');
}

Comments

[Даниил Игоревич]

Спасли меня! +Infinity Вам в карму!

[Дмитрий Беляев]

Даниил Ораин: обращайтесь