Conversation
- add hover-slider library - update JSON data for showcase items - fix mlut utilities
| } | ||
|
|
||
| const script = document.createElement('script'); | ||
| script.src = 'https://cdn.jsdelivr.net/gh/web-projects-lab/[email protected]/hover-slider.min.js'; |
There was a problem hiding this comment.
Стороннюю библиотеку надо импортировать как es-модуль тут, а не как скрипт на страницу подключать
There was a problem hiding this comment.
Если я правильно понял и не ошибаюсь, ее нельзя импортировать через import ... from '...' (https://github.com/web-projects-lab/hover-slider/blob/master/README.md)
-add buttons -fix styles
There was a problem hiding this comment.
Зачем в названии каталога images, если по верхнему каталогу и так понятно, что тут изображения?
| alt="" | ||
| class="W100p H100p Ojf-cv Ojp-c Bdrd8 hover-switch-image" | ||
| /> | ||
| <button id="back" class="D-f P1u Bgc-$blueGray Bd-n Bdrd8 Ps-a T119 L16 md_D-n"> |
There was a problem hiding this comment.
При подключении компонента получится много элементов с одинаковым id на странице - такого быть не должно. Обычные классы для нужд js никто не запрещал использовать или data атрибуты
| %> | ||
|
|
||
| <<%= tag %>> | ||
| <a class="Txd-n C-ih W100p H100p D-f Fld-c Bdt1;s;$accent750 Bdb1;s;$accent750 Bdt1;s;$brand_h Bdb1;s;$brand_h Ts-bd,0.3s,e Cs" href="<%= item.link %>"> |
There was a problem hiding this comment.
Зачем 2 утилиты для разных сторон бордера, если есть одна: -Bdy?
| @@ -0,0 +1,79 @@ | |||
| const libraryJs = "https://cdn.jsdelivr.net/gh/web-projects-lab/[email protected]/hover-slider.min.js"; | |||
There was a problem hiding this comment.
Если эта библиотека не умеет работать с es-модулями, значит это плохая библиотека - надо другую. Ну и судя по репозиторию - это какой-то несерьезный велосипед
| <div class="D-n H8u W-$bleedWidth Ml-140 Bdt1;s;$accent750 Bdb1;s;$accent750 md_D-f"></div> | ||
| <ul class="Lss-n P0 M8u;5u D-g Gtc-t1 Gap8u sm_Gtc-t2 xl_Gtc-t3 xxl_Gtc-t4"> | ||
| <% for (let elem of it.cases) { %> | ||
| <%- include('_includes/components/show-card-slider.ejs', { optTag: 'li', item: elem }) %> |
There was a problem hiding this comment.
Если по дефолту тег li, то и параметр этот передавать нет смысла
| src="<%= defaultImage %>" | ||
| alt="<%= item.title %>" | ||
| data-hover-slides="<%= othersImages %>" | ||
| data-options='{"touch": "end", "infinite": false, "preventScroll": true}' |
There was a problem hiding this comment.
Если эти опции статичны, то почему они тут передаются, а не сразу в js указаны?
- add new library - fix mlut utilities
|
|
||
| async loadSwiperModule() { | ||
| try { | ||
| const { default: Swiper } = await import(swiperModule); |
There was a problem hiding this comment.
Почему динамический импорт, а не обычный?
| this.loadSwiperModule(); | ||
| } | ||
|
|
||
| render() { |
There was a problem hiding this comment.
Зачем тут рендеринг? Этот компонент легковесный должен быть, просто инкапсулировать логику. Предыдущий вариант в этом плане был лучше - не понятно, для чего сюда разметка переместилась
| } | ||
|
|
||
| ensureStyles() { | ||
| if (document.querySelector(`link[href*="swiper-bundle.min.css"]`)) return; |
There was a problem hiding this comment.
Если уж CSS подключать в этом модуле, то не в методах компонента, а просто в скрипте 1 раз
| --- | ||
|
|
||
| <% | ||
| const bdX = "-Bdx -Bdxc-$accent750"; |
There was a problem hiding this comment.
Зачем тут 2 утилиты если можно цвет сразу в -Bdx/-Bdy указать, как и просто в Bd
| </div> | ||
| <% }); %> | ||
| <% } else { %> | ||
| <div class="swiper-slide">Нет изображений</div> |
| </div> | ||
| <div class="swiper-pagination"></div> | ||
| <button class="prev D-f P1u Bg-n Bd-n Bdrd8 Ps-a T109 L0 Zi1 md_D-n"> | ||
| <svg class="Fi-$gray200 Fi-$brand_a Ts-shortTs Tf -Rt180d Cs" width="26px" height="26px" viewBox="0 0 512 512" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
Для svg иконок надо использовать svg спрайты. Примеры есть на сайте, можно даже существующий спрайт расширить
| </svg> | ||
| </button> | ||
| <button class="next D-f P1u Bg-n Bd-n Bdrd8 Ps-a T109 R0 Zi1 md_D-n"> | ||
| <svg class="Fi-$gray200 Fi-$brand_a Ts-$shortTs Cs" width="26px" height="26px" viewBox="0 0 512 512" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
Зачем тут $shortTs, используй везде дефолтный
| } | ||
|
|
||
| initSwiper() { | ||
| if (!this.isConnected) return; |
| } | ||
|
|
||
| connectedCallback() { | ||
| this.initSwiper(); |
There was a problem hiding this comment.
Отдельный метод уже не нужен
| <button class="prev D-f P1u Bg-n Bd-n Bdrd8 Ps-a T109 L0 Zi1 md_D-n"> | ||
| <svg class="Fi-$gray200 Fi-$brand_a Ts-shortTs Tf -Rt180d Cs" width="26px" height="26px" viewBox="0 0 512 512" xmlns="http://www.w3.org/2000/svg"> | ||
| <polygon points="150.46 478 129.86 456.5 339.11 256 129.86 55.49 150.46 34 382.14 256 150.46 478"/> | ||
| <svg class="W26 H26 Fi-$gray200 Fi-$brand_a Ts-fi,0.3s,e Tf -Rt180d Cs"> |
There was a problem hiding this comment.
Для transition по дефолту лучше вот эту утилиту использовать
There was a problem hiding this comment.
Размеры у svg лучше атрибутами задвать, как у обычного img
|
На десктопе навигация в фото-виджете должна появляться только при наведении |
No description provided.