refactoring: index.js #44
Conversation
| var body = document.body; | ||
|
|
||
| document.querySelector('.open-menu-btn').onclick = function (event) { | ||
| event.preventDefault(); |
There was a problem hiding this comment.
Просто из интереса: preventDefault в самом начале на что-то влияет?
There was a problem hiding this comment.
Позиция preventDefault ни на что не влияет. Было написано и так и так. Поэтому сделал как привычнее.
Сейчас, посмотрев на inner.js склоняюсь к тому, что надо в конец функции опустить.
Сам же думаю, что надо везде в начало функции переместить, так как preventDefault непосредственно влияет на событие и мы его отменяем. Об этом лучше сообщить сразу читателю. Весь остальной код внутри уже не про событие, а про действия ,которые надо выполнить при событии.
There was a problem hiding this comment.
Хотя в inner.js preventDefaule() располагается как угодно. Надо просто придумать ему место.
|
Я предлагаю довести всё до единого стиля во всех случаях употребления и тогда уже мёржить. В отличие от стилей, скрипты не будут выброшены, а скорее переписаны на основе. |
|
Все jquery объекты теперь именуются с Удалил
jquery-селекторы вынес в шапку. Повторяющиеся jquery селекторы вынес, теперь в DOM стучимся реже. Конструкции переделал в так нагляднее и вероятнее, что не попадёт в Длинные строки ограничил 120 символами по ширине. Некоторые переменные назвал по-человечески, хотя автору более понятно. Я не всё осилил =( Дублирующиеся куски вынес в функцию. Двойные кавычки, сменил на одинарные там где это возможно. Добавил точек с запятой в конец строк. Всё что можно было зачейнить зачейнил. Всё что не смог не зачейнил. В целом для рефактора, мне кажется достаточно. По правильному это надо переписать. Опять же ждём #25 |
|
Спасибо! По такому коду точно будет проще переписывать :) Ну и не стыдно за исходники сейчас. |
Изменения для #43 - привести код index.js к единому стилю