Хабр Курсы для всех
РЕКЛАМА
Практикум, Хекслет, SkyPro, авторские курсы — собрали всех и попросили скидки. Осталось выбрать!
Хабр доступен 24/7 благодаря поддержке друзей
Комментарии 17
Ну, коль уж вы хотели конструктивной критики:
— не стоит вызывать getBoundingClientRect() для одного и того же элемента подряд строка за строкой — это добавляет нагрузку, лучше вынести в отдельную переменную первый запуск, а затем уже обращаться к свойствам;
— обращаться к свойствам объекта можно не только через квадратные скобки, но и через точку;
— document.getElementById('title') сработает быстрее, чем document.querySelector("#title") (так для всех обращений по id) (про window.title молчу, т.к. лучше про него забыть));
— для элементов, которые не меняются и к которым часто происходит обращение, лучше завести отдельный объект типа кэша, чтобы каждый раз через селекторы не обращаться (или в вашем случае можно «забиндить» слушателей событий на this, чтобы обращаться к тем же самым созданным div'ам);
— стили лучше менять с помощью классов — браузер в таком случае применяет сразу весь набор новых стилей, а не строчка за строчкой, каждый раз перерисовывая элемент;
— код лучше разнести по разным файлам, в зависимости от назначения;
— стили так же лучше вынести туда, где они должны быть, то есть в css-файлы;
— если появление дополнительных ненужных отступов заставляет вас писать в строку элементы html, то можно убрать шрифт для всей страницы (font-size:0px;), а включить его там, где он непосредственно нужен, чтобы красивее отобразить html-код;
— честно говоря, не понимаю, для чего вам end.js, лучше его удалить, а так же убрать из манифеста возможность редактировать страницу любого сайта — это вызовет больше доверия у потенциальных пользователей;
— структура рода [«Мои записки»,0] ведет к ненужной жесткости кода — вам потом понадобиться добавить еще какое-то свойство, придется расширять массив и менять функцию-обработчик, я бы советовал заменить на объект;
— очень много дублирования кода;
— на каждое нажатие клавиши вы запускаете перебор по всем div'ам — это ужасно — если вы хотите найти следующий (предыдущий) элемент за родителем абзаца, который находится в фокусе, пользуйтесь nextElementSibling /previousElementSibling, с проверкой на существование;
— вы сравниваете e.keyCode и e.which на одно и то же значение, используйте любой из них (лучше keyCode) и пользуйтесь своим switch по-нормальному на здоровье (если вдруг так уж случилось, что они у вас работают по-разному, то можно воспользоваться конструкциями вида key=e.keyCode||e.which);
— вместо одного универсального слушателя лучше вешать на каждый элемент свой и отдельно повесить на нажатие по контекстному меню на скрытие;
— и к детям лучше все-таки обращаться через children (childNodes).
Честно скажу, код особо не читал, написал только то, что сразу бросилось в глаза, поэтому есть ли в логике самого расширения какие-то ошибки и недочеты, не могу сказать (код отвлекает). Для чего используется focus на элементах p тоже не знаю, но, думаю, лучше обойтись без него.
Основным итогом будет следующий: не используйте querySelector как панацею, ищите более быстрые альтернативы, применяйте файлы html, js и css для тех целей, для которых они предназначаются, в манифесте должно быть перечислено только то, что используется (файлы, разрешения).
— не стоит вызывать getBoundingClientRect() для одного и того же элемента подряд строка за строкой — это добавляет нагрузку, лучше вынести в отдельную переменную первый запуск, а затем уже обращаться к свойствам;
— обращаться к свойствам объекта можно не только через квадратные скобки, но и через точку;
— document.getElementById('title') сработает быстрее, чем document.querySelector("#title") (так для всех обращений по id) (про window.title молчу, т.к. лучше про него забыть));
— для элементов, которые не меняются и к которым часто происходит обращение, лучше завести отдельный объект типа кэша, чтобы каждый раз через селекторы не обращаться (или в вашем случае можно «забиндить» слушателей событий на this, чтобы обращаться к тем же самым созданным div'ам);
— стили лучше менять с помощью классов — браузер в таком случае применяет сразу весь набор новых стилей, а не строчка за строчкой, каждый раз перерисовывая элемент;
— код лучше разнести по разным файлам, в зависимости от назначения;
— стили так же лучше вынести туда, где они должны быть, то есть в css-файлы;
— если появление дополнительных ненужных отступов заставляет вас писать в строку элементы html, то можно убрать шрифт для всей страницы (font-size:0px;), а включить его там, где он непосредственно нужен, чтобы красивее отобразить html-код;
— честно говоря, не понимаю, для чего вам end.js, лучше его удалить, а так же убрать из манифеста возможность редактировать страницу любого сайта — это вызовет больше доверия у потенциальных пользователей;
— структура рода [«Мои записки»,0] ведет к ненужной жесткости кода — вам потом понадобиться добавить еще какое-то свойство, придется расширять массив и менять функцию-обработчик, я бы советовал заменить на объект;
— очень много дублирования кода;
— на каждое нажатие клавиши вы запускаете перебор по всем div'ам — это ужасно — если вы хотите найти следующий (предыдущий) элемент за родителем абзаца, который находится в фокусе, пользуйтесь nextElementSibling /previousElementSibling, с проверкой на существование;
— вы сравниваете e.keyCode и e.which на одно и то же значение, используйте любой из них (лучше keyCode) и пользуйтесь своим switch по-нормальному на здоровье (если вдруг так уж случилось, что они у вас работают по-разному, то можно воспользоваться конструкциями вида key=e.keyCode||e.which);
— вместо одного универсального слушателя лучше вешать на каждый элемент свой и отдельно повесить на нажатие по контекстному меню на скрытие;
— и к детям лучше все-таки обращаться через children (childNodes).
Честно скажу, код особо не читал, написал только то, что сразу бросилось в глаза, поэтому есть ли в логике самого расширения какие-то ошибки и недочеты, не могу сказать (код отвлекает). Для чего используется focus на элементах p тоже не знаю, но, думаю, лучше обойтись без него.
Основным итогом будет следующий: не используйте querySelector как панацею, ищите более быстрые альтернативы, применяйте файлы html, js и css для тех целей, для которых они предназначаются, в манифесте должно быть перечислено только то, что используется (файлы, разрешения).
Спасибо большое за комментарий.
и еще раз большое спасибо за такой развернутый комментарий.
— не стоит вызывать getBoundingClientRect() для одного и того же элемента подряд строка за строкой — это добавляет нагрузку, лучше вынести в отдельную переменную первый запуск, а затем уже обращаться к свойствам;В классе я так и действовал, нашел лишь одно место где обращался дважды к getBoundingClientRect().
— обращаться к свойствам объекта можно не только через квадратные скобки, но и через точку;Спасибо, что напомнили. Первый мой язык для веба был php. Он показывал ошибку при обращении в stdClass() к элементу имеющему имя на кириллице.Это привело к тому, что я привык в php обращаться к свойствам через скобки. И сказалось на мой стиль программирования в JS.
Как сейчас состоит дело со свойствами на кирилице
— document.getElementById('title') сработает быстрее, чем document.querySelector("#title") (так для всех обращений по id) (про window.title молчу, т.к. лучше про него забыть));Я отказался работать с getElementById() в тот момент когда понял что он выдергивает только одно значение. В теле страницы могут быть несколько элементов с одним id, при парсинге товарных агрегаторов я раз и навсегда отказался от getElementById(), и перешел на querySelector() и querySelectorAll()
— для элементов, которые не меняются и к которым часто происходит обращение, лучше завести отдельный объект типа кэша, чтобы каждый раз через селекторы не обращаться (или в вашем случае можно «забиндить» слушателей событий на this, чтобы обращаться к тем же самым созданным div'ам);Я не совсем понял о чем тут речь.
— стили лучше менять с помощью классов — браузер в таком случае применяет сразу весь набор новых стилей, а не строчка за строчкой, каждый раз перерисовывая элемент;Так я итак использую классы. Не везде конечно, но там где мне нужно точно переопределить css к элементу. Так как обращение в ccs к id(#) важнее чем обращение к классу элемента(.)
— код лучше разнести по разным файлам, в зависимости от назначения;p.p.p.s. Основная задача топика — написать быстро расширение без использования фреймворков. Смысл быстро написать и запустить
— стили так же лучше вынести туда, где они должны быть, то есть в css-файлы;
— честно говоря, не понимаю, для чего вам end.js, лучше его удалить, а так же убрать из манифеста возможность редактировать страницу любого сайта — это вызовет больше доверия у потенциальных пользователей;
— структура рода [«Мои записки»,0] ведет к ненужной жесткости кода — вам потом понадобиться добавить еще какое-то свойство, придется расширять массив и менять функцию-обработчик, я бы советовал заменить на объект;
— очень много дублирования кода;
— вы сравниваете e.keyCode и e.which на одно и то же значение, используйте любой из них (лучше keyCode) и пользуйтесь своим switch по-нормальному на здоровье (если вдруг так уж случилось, что они у вас работают по-разному, то можно воспользоваться конструкциями вида key=e.keyCode||e.which);
— если появление дополнительных ненужных отступов заставляет вас писать в строку элементы html, то можно убрать шрифт для всей страницы (font-size:0px;), а включить его там, где он непосредственно нужен, чтобы красивее отобразить html-код;За этот трюк большое спасибо!!!
— на каждое нажатие клавиши вы запускаете перебор по всем div'ам — это ужасно — если вы хотите найти следующий (предыдущий) элемент за родителем абзаца, который находится в фокусе, пользуйтесь nextElementSibling /previousElementSibling, с проверкой на существование;nextElementSibling /previousElementSibling взял на заметку. Просто забыл про существование это навигации.
— вместо одного универсального слушателя лучше вешать на каждый элемент свой и отдельно повесить на нажатие по контекстному меню на скрытие;Вот здесь Вы не правы. Я создал массив папок из 200 элементов. Потом повесил на каждый элемент по обработчику события. В итоге Страница с действующим классом, начала подвисать при event'ах, а у меня очень даже нормальный ноутбук. И учитывая, что на странице может быть не один экземпляр класса мне пришлось переписать логику так чтобы event'ов было не так много.
и еще раз большое спасибо за такой развернутый комментарий.
Я отказался работать с getElementById() в тот момент когда понял что он выдергивает только одно значение. В теле страницы могут быть несколько элементов с одним id
Могут, но не должны, id есть уникальный идентификатор элемента, наличие нескольких — ошибка семантики. Для выбора нескольких элементов есть более другие методы, содержащие elements в названии (getelementsbyclassname() например).
В классе я так и действовал, нашел лишь одно место где обращался дважды к getBoundingClientRect().
Я имел в виду:
this.links.contextDiv.style.left = (p.getBoundingClientRect()[«left»]+ 50) + «px»;и
this.links.contextDiv.style.top = (p.getBoundingClientRect()[«top»]+ 10) + «px»;
var leftCR = a.getBoundingClientRect()[«left»];
var topCR = a.getBoundingClientRect()[«top»];
var widthCR = a.getBoundingClientRect()[«width»];
var bottomCR = a.getBoundingClientRect()[«bottom»];
Я отказался работать с getElementById() в тот момент когда понял что он выдергивает только одно значение. В теле страницы могут быть несколько элементов с одним id, при парсинге товарных агрегаторов я раз и навсегда отказался от getElementById(), и перешел на querySelector() и querySelectorAll()Просто оставлю это здесь. Но, в конце концов, это ваш продукт, поэтому вам принимать окончательное решение в плане выбора способа, мы лишь советуем.
По поводу отдельного объекта для кэша — здесь все зависит от логики работы вашего расширения — но в виду того, что элементы на странице не могут возникать из ниоткуда, а создаются непосредственно js-скриптом, то можно ссылки на них помещать в отдельный объект, чтобы через него быстрее обращаться к элементам, даже посредством того же перебора. Хранение не сильно нагрузит память, т.к. в объекте или массиве будут храниться ссылки на объекты.
По поводу «биндинга» — Привязка контекста и карринг: «bind».
Вот здесь Вы не правы. Я создал массив папок из 200 элементов. Потом повесил на каждый элемент по обработчику события. В итоге Страница с действующим классом, начала подвисать при event'ах, а у меня очень даже нормальный ноутбук. И учитывая, что на странице может быть не один экземпляр класса мне пришлось переписать логику так чтобы event'ов было не так много.
Здесь я имел в виду контекстное меню.
Напишите, пожалуйста, пример того, как вы вешали слушателей (или дайте ссылку на него) — интересно взглянуть, т.к. у самого есть проект с достаточно большим числом объектов на странице, на каждом из которых висит отдельный слушатель, и пока тормозов и подвисаний замечено не было.
Я отказался работать с getElementById() в тот момент когда понял что он выдергивает только одно значение. В теле страницы могут быть несколько элементов с одним id, при парсинге товарных агрегаторов я раз и навсегда отказался от getElementById(), и перешел на querySelector() и querySelectorAll()
Просто оставлю это здесь. Но, в конце концов, это ваш продукт, поэтому вам принимать окончательное решение в плане выбора способа, мы лишь советуем.
Насчет скорости доступа я согласен. Быстрее обращение к id. Но дело не в скорости, а в том я для себя решил работать в js c css селекторами. Возможно в каком то другом проекте и потребуется использовать именно getElementById, но пока querySelector.
Тесты
Плюс по скринам видно, что миллион обращение к id и к селектору примерно выполняются за 100 и 300 секунд. Для меня не критично в данном проекте ускоряться за счет id.
Вот здесь Вы не правы. Я создал массив папок из 200 элементов. Потом повесил на каждый элемент по обработчику события. В итоге Страница с действующим классом, начала подвисать при event'ах, а у меня очень даже нормальный ноутбук. И учитывая, что на странице может быть не один экземпляр класса мне пришлось переписать логику так чтобы event'ов было не так много.
Здесь я имел в виду контекстное меню.
Напишите, пожалуйста, пример того, как вы вешали слушателей (или дайте ссылку на него) — интересно взглянуть, т.к. у самого есть проект с достаточно большим числом объектов на странице, на каждом из которых висит отдельный слушатель, и пока тормозов и подвисаний замечено не было.
Вначале я начал писать этот класс и вешать непосредственно на элементы «p» все события. К сожалению первоначальный код не сохранился.
И еще раз спасибо за критику.
Пример создания одного extension chrome
тогда уж или «Пример создания одного chrome extension» или «Пример создания одного chrome-расширения» или «Пример создания одного расширения для chrome».
CamelCase в css. Блин, чёт криповенько!
Получился не проект, а сильносвязанный блоб.
- Класс-портянку рассортировать, ибо SOLID;
- Мух от котлет отделить. HTML, CSS и JS должны быть в отдельных файлах;
- Менять CSS стили во время выполнения нехорошо и неинформативно.
- Соглашения о наименованиях и стиле кода существуют не просто так;
- CSS без препроцессора? В 2017 году?
Взаимодействия с WebSQL или IndexedDB тоже не видно. Это что только интерфейс?
Извините, но что-то очень захотелось ответить: http://www.yegor256.com/2017/03/28/solid.html
Да, да, веселый, ведь он настолько не типизирован, что объект может иметь свойство в виде самого себя ) Причем при обращении к этому свойству мы реально изменяем сам объект.
Вот только типизация тут ни при чём. В типизированных языках такое тоже возможно.
Вот это
— зло.
Можете объяснить, зачем Вам кириллица в именах?
Вот так, что без этого совсем никуда…
Если это про «национальную гордость великороссов», то представьте, что вам достался на поддержку китайский проект, а они гордыми оказались…
Про
Javascript изучаете, изучите азы английского заодно. Не трудно и пригодится много где еще.
Общее впечатление от Вашего подхода — нет структуры, каша какая-то.
Уж простите за такую критику.
$obj->Русские_буквы
— зло.
Можете объяснить, зачем Вам кириллица в именах?
Вот так, что без этого совсем никуда…
Если это про «национальную гордость великороссов», то представьте, что вам достался на поддержку китайский проект, а они гордыми оказались…
Про
«svoistvo»вообще промолчу… Чем «property» не угодило? Английского не знаете?
Javascript изучаете, изучите азы английского заодно. Не трудно и пригодится много где еще.
Общее впечатление от Вашего подхода — нет структуры, каша какая-то.
Уж простите за такую критику.
YuryZakharov, спасибо за комментарий.
Вот смотрите, допустим вы парсите какой нибудь сайт, в его коде элементы различаются по dataset свойствам (к примеру data-auth=«Тиничев Валерий»). Вот именно при обработке таких блоков и возникают не англоязычные имена свойств.
Или другой пример.
Вы обрабатываете текст на поиск повторяющихся значений. Намного проще давать key именно часть текста не зависимо от языка. Это лишь при обработке данных.
property — всем угодило. Но для быстроты и наглядности использовал именно русское слово на латинице.
p.p.p.s. Основная задача топика — написать быстро расширение без использования фреймворков. Быстро написать в течении дня, максимум двух. И быстро запустить.
Наоборот спасибо Вам за критику, если человек не критиковать он остановиться в развитии. Ведь сам себя любой может простить и понять. )))
Вот это
$obj->Русские_буквы
— зло.
Можете объяснить, зачем Вам кириллица в именах?
Вот так, что без этого совсем никуда…
Если это про «национальную гордость великороссов», то представьте, что вам достался на поддержку китайский проект, а они гордыми оказались…
Вот смотрите, допустим вы парсите какой нибудь сайт, в его коде элементы различаются по dataset свойствам (к примеру data-auth=«Тиничев Валерий»). Вот именно при обработке таких блоков и возникают не англоязычные имена свойств.
Или другой пример.
Вы обрабатываете текст на поиск повторяющихся значений. Намного проще давать key именно часть текста не зависимо от языка. Это лишь при обработке данных.
«svoistvo»
вообще промолчу… Чем «property» не угодило? Английского не знаете?
Javascript изучаете, изучите азы английского заодно. Не трудно и пригодится много где еще.
property — всем угодило. Но для быстроты и наглядности использовал именно русское слово на латинице.
Общее впечатление от Вашего подхода — нет структуры, каша какая-то.
p.p.p.s. Основная задача топика — написать быстро расширение без использования фреймворков. Быстро написать в течении дня, максимум двух. И быстро запустить.
Уж простите за такую критику.
Наоборот спасибо Вам за критику, если человек не критиковать он остановиться в развитии. Ведь сам себя любой может простить и понять. )))
Отдельного восхищения заслуживает «Редактирование названия»! С иллюстрацией!!!
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Пример создания одного chrome extension