Aleksey Varov #1
Conversation
css/blocks/form/form.less
Outdated
| box-sizing: border-box; | ||
| width: 100%; | ||
|
|
||
| &__block { |
There was a problem hiding this comment.
Элемент блока form с названием block обычно наводить на мысль, что где-то с композицией элементов была ошибка
css/blocks/form/form.less
Outdated
| display: flex; | ||
| align-items: center; | ||
| } | ||
| &-error { |
There was a problem hiding this comment.
почему этот в отличие от остальных — является отдельным блоком?
There was a problem hiding this comment.
кажется, что должен быть модификатором
| @@ -0,0 +1,10 @@ | |||
| { | |||
There was a problem hiding this comment.
Сюда буду скидывать недочеты в интерфейсе \ логике. Провалидацию помню, поэтому не пишу.
-
самовывоз, а адрес все равно хотят знать
-
Клик вне попапа не закрывает его. Не очень удобно
-
На макете селект выглядит по-другому
-
Мобильный вид "едет":
css/blocks/navbar/navbar.less
Outdated
| } | ||
|
|
||
| &__nav { | ||
| &-list { |
There was a problem hiding this comment.
по бэм не рекомендуется делать так — это усложняет восприятие и ты описываешь list внутри nav — это получается элемент, образованный от элемента
js/components/common/asyncLoader.js
Outdated
| let citiesResponse = await response.json(); | ||
| return citiesResponse; | ||
| } catch (e) { | ||
| alert("Ошибка в получении данных от сервера"); |
There was a problem hiding this comment.
А что пользователь должен сделать с такой ошибкой?)
There was a problem hiding this comment.
я к тому, что паттерн хорошего поведения — проверить есть ли коннект к сети, если нет, написать. Если это ошибка сервера — предложить или сделать самим запрос чуть позже
| //Навешиваем обработчики на все карточки | ||
| //При клике на подложку (body) скрываем и удаляем элементы | ||
| const cards = document.querySelectorAll(".product-card"); | ||
| const body = document.querySelector("#body"); |
There was a problem hiding this comment.
лучше выбрать контейнер ближайший. иначе ты пытаешься трекать все
js/components/popup-card/popup.js
Outdated
| const body = document.querySelector("#body"); | ||
| const handlerActive = event => { | ||
| cards.forEach(card => { | ||
| if (event.target.closest(".product-card") === card) { |
There was a problem hiding this comment.
я говорил на практикуме — не очень хорошо завязываться на стилевые классы, для логики лучше завести специальные классы для js
| } | ||
| sizeLabel.addEventListener("click", () => { | ||
| orderButton.classList.remove("button_disabled"); | ||
| orderButton.disabled = false; |
There was a problem hiding this comment.
достаточно просто disabled свойство и матчится на :disabled псевдокласс
| const sendObj = {}; | ||
| allElements.forEach(element => { | ||
| if(element.name === "user-name") { | ||
| sendObj["Имя пользователя"] = element.value; |
There was a problem hiding this comment.
не нужно объект для отправки формировать кирилицей.
Лучше держать поля унифицированно на английском и более того, соответствие element.name предпочтительнее. Что вижу, то и получаю
| } else if(element.name === "size" && element.checked){ | ||
| sendObj["Выбранный размер"] = element.value; | ||
| } else if (productName) { | ||
| sendObj["Выбранный продукт"] = productName; |
There was a problem hiding this comment.
у форм есть готовые возможности сериализации
| if (element.name === "user-name") { | ||
| //Имя пользователя | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "user-email") { | ||
| //Электронный адрес пользователя | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "user-name") { | ||
| //Имя пользователя | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "code-country") { | ||
| //Код страны | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "mobile-number") { | ||
| //Номер мобильного телефона | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "delivery" && element.checked) { | ||
| //Выбранный способ получения | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "city") { | ||
| //Выбранный город | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.value && element.name === "address") { | ||
| //Указанный адрес | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "pay" && element.checked) { | ||
| //Выбранный способ оплаты | ||
| sendObj[element.name] = element.value; | ||
| } else if (element.name === "sms") { | ||
| //Оповещение по sms | ||
| sendObj[element.name] = element.checked; | ||
| } else if (element.name === "size" && element.checked) { | ||
| //Выбранный размер | ||
| sendObj[element.name] = element.value; |
There was a problem hiding this comment.
Такой код сложно читается, он быстро растет и по большей части, это все выносится в разного рода конфиги
| //Находим в карточке первый дочерний элемент | ||
| //если он является крестиком закрытия, то удаляем его | ||
| const orderInfo = card.firstElementChild; | ||
| if (orderInfo.classList.contains("product-card__button-close")) { |
There was a problem hiding this comment.
в некоторых местах остались стилевые классы, выполняющие бизнес-логику
| <input type="radio" name="size" class="radio js-radio" value="${size}" disabled /> | ||
| `; | ||
| } | ||
| templateSizes += `<span class="radio-button product-card__sizing-button">${size}</span> |
There was a problem hiding this comment.
Логика выбора размера очень сложная с кучей вложенных условий
No description provided.