Как правильно навесить обработчик при помощи addEventListener?

Question

[Вячеслав Лебедев]

Добрый вечер!
В коде ниже, я делаю по клику на элемент добавление класса, но получаю вот такую ошибку:
Cannot read property 'add' of undefined
Как я понял, проблема в .bind, потому что без него, все обработчики выполняются, но не по нажатию, а сразу.
Как починить?
Спасибо!

var clickToOrder, elem, i, len, select;

clickToOrder = function(elem) {
  return elem.classList.add('active');
};

select = document.querySelectorAll('.jsCart');

for (i = 0, len = select.length; i < len; i++) {
  elem = select[i];
  elem.addEventListener('click', clickToOrder.bind(elem));
}

upd: elem в clickToOrder:

MouseEvent {dataTransfer: null, which: 1, toElement: ........

Answer 1

[Евгений Петров]

Первый аргумент в обработчике — ссылка на событие.

function clickToOrder (e) {
	e.target.classList.add('active');
	console.log(this); // а это тот же e.target — вы же зачем-то вызывали bind()
};

Обычно привязывают какой-то абстрактный объект, ведь ссылка на узел, где зарегистрирован слушатель события, и так доступна.
Пример

Comments

[Вячеслав Лебедев]

Не совсем пойму, как применить эти знания к моей проблеме :)

[Вячеслав Лебедев]

Получилось вот так:

elem.addEventListener('click', clickToOrder.bind(null, elem));

Спасибо за помощь! :)

[Евгений Петров]

Вячеслав Лебедев: Если вам не требуется передавать передавать ссылку на абстрактный объект в this, как это сделал я в примере, вам bind() вообще не нужен. То есть совсем.
Метод bind() нужен для гарантированной ссылки в this, а не для передачи аргументов. Почитайте про этот метод.

Answer 2

[Денис Малиночкин]

Ваша проблема в том, что у вас при клике в функцию clickToOrder передаётся объект "событие", а не элемент, на котором был совершен клик.

Объект событие всегда передается обработчику и содержит массу полезной информации о том где и какое событие произошло.

Например, в вашем случае такое:

MouseEvent {dataTransfer: null, which: 1, toElement: div.jsCart, fromElement: null, y: 15…}

В итоге должно получиться как-то так:

(function() {
  var selectEls = document.querySelectorAll('.jsCart');

  var clickToOrder = function(event) {
    console.log(event);
    event.target.classList.add('active');
  };

  for (var i = 0; i < selectEls.length; i++) {
    var item = selectEls[i];
    item.addEventListener('click', clickToOrder);
  }
})();

Comments

[Вячеслав Лебедев]

"Говорят, что подслушивать события в циклах плохо (привет Google)."
Это почему, не пойму?)) В чем разница Вашего подхода и моего, без дополнительной ф-ии?

[copal]

Говорят, что подслушивать события в циклах плохо (привет Google).

Да ну Вы что... А если у меня сто кнопок, мне писать сто строк одного и того же кода?

[Денис Малиночкин]

copal: так, друзья, спокойствие)))

Речь про использование addEventListener прямо в теле цикла. Паттерн Google для jscs доброжелательно просит убрать его в функцию.

[Денис Малиночкин]

Вячеслав Лебедев: copal: Просто загнул про стилистику не подумав. Убрал из ответа))

[Евгений Петров]

Денис Малиночкин: А где можно почитать про этот совет?

Единственная проблема, что может прийти мне в голову — устанавливать в качестве обработчика анонимную функцию и пытаться использовать переменные цикла в этом обработчике. Тогда да — каждый из замыкания получит одно и то же значение для таких переменных. А других ограничений не вижу.

[Вячеслав Лебедев]

Появилась еще одна загвоздка.
Имеется html:

<button class="jsCart">
  <span class="icon-2x icon-add-cart"></span>
</button>

Так вот. По нажатию к примеру в область иконки (span), target будет указывать на нее.
А хотелось бы, что указывал всегда на родителя. Т.е. на button, именно тот, кто отлавливает событие "click".
Как такого достичь?

[Евгений Петров]

Вячеслав Лебедев: Если вы зарегистрировали обработчик на .jsCart без применения bind() — в обработчике this будет указывать именно на .jsCart.
Если же вам потребовалось передать в this ссылку на некий объект, то нужно организовать цикл по parentNode, пока не дойдём до нужного узла.

[Вячеслав Лебедев]

Евгений Петров: неужели все так трудно? Хотел попробовать не применять библиотеки, но похоже придется вернуться к zepto :(

[Евгений Петров]

Вячеслав Лебедев: А вы думаете, зачем нужна абстракция и тонны API для этой абстракции? Для вашей же пользы и экономии времени. Для оперирования нативным JS требуется серьёзное и глубокое понимание того, что вы делаете.

[Вячеслав Лебедев]

Евгений Петров: Вот и все:

$('.jsCart').on('click', function() {
  return $(this).addClass('active');
});

Никакого геморроя... И работает так, как я ожидаю.
Не пойму, почему ванила такая проблематичная с точки зрения использования? :(
Очень жаль.

[Вячеслав Лебедев]

Евгений Петров: вот теперь все встало на свои места :) ванила для ниндзя.

[Евгений Петров]

Вячеслав Лебедев: Только привыкайте делать нормально

$(document).on('click', '.jsCart', function () {
	$(this).addClass('active');
});

[Вячеслав Лебедев]

Евгений Петров: Почему это является нормальным. А мой подход нет?)

[Вячеслав Лебедев]

Евгений Петров: на сколько я знаю, Ваш подход более устойчив к динамическим изменениям в структуре html. Или как это по другому называется :)

[Евгений Петров]

Вячеслав Лебедев: Потому, что:
1. Вы сначала ищете все элементы, что всегда снижает производительность, а я использую фазу всплытия события.
2. Если вы добавите потом такие же элементы, для них обработчик не сработает, а в моём случае обработчик запустится для любого динамически добавленного элемента.
3. Вы должны дождаться построения DOM, а я могу объявить регистрацию сразу — ведь document существует всегда.

Всегда лучше использовать отлов в корневых элементах, что позволяет в нативном JS использовать и фазу распространения. Впрочем, это уже для ниндзя:)

[Денис Малиночкин]

Евгений Петров: Простите, но я и вас обманул и себя запутал)))

Суть моего "загона" про addEventListener сводилась к тому, что JSHint (с дуру сказал JSCS) предупреждает об наличии объявления функции в цикле (Don't make functions within a loop), что не особо хорошо. То есть требуется создавать функцию вне цикла и вызывать её.

Корректно:

(function() {
  var clickHandler = function() {
    console.log('qwe');
  };

  for (var i = 0; i < 10; i++) {
    document.addEventListener('click', clickHandler);
  }
})();

[Вячеслав Лебедев]

Денис Малиночкин: спасибо, но уже потеряло актуальность :) потому, что чувствую мне еще рановато сталкиваться с такими трудностями ванилы :)

[Вячеслав Лебедев]

Евгений Петров: спасибо за детальное разъяснение :) взял на заметку. Буду стараться пользоваться именно таким способом :)

[Евгений Петров]

Денис Малиночкин: Как мне известно, в теле инструкции действительно нельзя писать FD, поскольку инструкции не создают контекст, но присваивать функцию как значение (FE) в теле цикла никто не запрещает. С addEventListener, конечно, это не связано.

[Вячеслав Лебедев]

Евгений Петров: А что посоветуете, вот в таких ситуациях?
$('.jsCart').attr('data-item');
Есть ли способы оптимизировать такое?

[Евгений Петров]

Вячеслав Лебедев: Любая инструкция зависит от контекста, в котором написана. В отрыве от контекста тут оптимизировать просто нечего.

[Вячеслав Лебедев]

Евгений Петров: понял. Огромное спасибо за помощь! :)