karinamaulitova / petcare

PetCare - тестовое задание для компании Picasel

В проекте использовались: flexbox и css grid для адаптивности, а также js для открытия попапа и выпадающего меню на мобильной версии. Для слайдера использован glider.js. Для сборки проекта использован webpack. Сайт адаптивный(мобильная и десктопная версии).

Стек: HTML, SCSS, JS, Webpack

Ссылка на сайт

petcare's Issues

Padding у input

Инпутам лучше добавлять паддинг, чтобы текст не прилипал к outline и border

Не прописан атрибут type

PetCare/src/index.html

Line 170 in 24d68e2

<button aria-label="Назад" class="glider__button glider__button_type_prev"><svg

Молодец, что сделала стрелки кнопками, но нужно было все-таки добавить атрибут type="button", так как по умолчанию submit

Для чего эта обертка?

PetCare/src/index.html

Line 16 in 24d68e2

<div class="page">

Все стили этого класса можно было повесить на body

Не важное для пользователя изображение

PetCare/src/index.html

Line 140 in 24d68e2

<img class="banner__image banner__image_type_cat"

Кот не является важным с точки зрения контента изображением для пользователя, его лучше было оформить как background-image. Со вторым слайдом то же самое.

Сложно читаемый класс

PetCare/src/index.html

Line 125 in 24d68e2

<h1 class="content__main-heading_type_hidden">PetCare</h1>

Здесь, в целом, достаточно было повесить visually-hidden.
Сам класс не очень понятен, если через нижнее подчеркивание модификаторы, тогда почему их два, то есть по такой логике у тебя где-то есть type_visible или что-то подобное. Плюс модификаторы иначе используются. Выдержка из документации:

С точки зрения БЭМ-методологии модификатор не может использоваться в отрыве от модифицируемого блока или элемента. Модификатор должен изменять вид, поведение или состояние сущности, а не заменять ее.

Нужно избегать длинных и сложных названий, там где они не нужны для лучшей читаемости кода.

Форма без единого атрибута

PetCare/src/index.html

Line 312 in 24d68e2

<form class="popup__form form">

Как минимум должны были быть name и action

Лучше в tel: писать через +7

PetCare/src/index.html

Line 272 in 24d68e2

<a class="footer__contact" href="tel:[phone]">8(800) 999-88-77</a>

Не критичная ошибка, но может обломать пользователей, набирающих номер из-за заграницы.
Лучше для перестраховки всегда писать +7

Ссылки без транзишенов

На все ссылки с hover нужно добавлять transition.

Например, если она меняет цвет, то так:

transition: color 0.3s ease

Можно также вешать transition: all 0.3s ease, тогда будет мягко меняться все ее свойства, но лучше все-таки прописывать именно те свойства, которые меняются при hover, чтобы избежать неожиданное поведение.

Как выглядит transition на деле можно посмотреть тут: https://www.royal-canin.ru/calculator/. Наведение на кнопки "Котенок" и "Щенок" работают с транзишнами, поэтому смена цветов происходит плавно, а у тебя в работе получилась резкая смена цветов.

У transition может быть разное время и анимации, подробнее можно прочитать тут: https://developer.mozilla.org/ru/docs/Web/CSS/transition

БЭМ: page__header

PetCare/src/index.html

Line 17 in 24d68e2

<header class="page__header header">

В рамках одной страницы header всегда будет являться самостоятельным блоком, и вряд ли будет элементом другого, только если это не header в section/article.

В комментариях указала также, что обертка page в данном случае не нужна.

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

Карточки являются самостоятельными блоками

PetCare/src/index.html

Line 190 in 24d68e2

<ul class="cards__list grid">

Карточки выглядят так, будто каждую из них можно вынести на любую другую страницу без потери контекста. Значит это самостоятельные блоки. Для них больше подходит не список, а выделить каждую в отдельный свой article. Причем именно article, а не section

Для лучшей семантики slider можно делать как ul

PetCare/src/index.html

Line 127 in 24d68e2

<div class="glider__slides-wrapper">

Где ul - slider__list, a li – slider__item
И код будет лучше читаться

Второй слайд должен быть без скругленного бэкграунда

То есть clip-path надо было повесить только на первый слайд.

БЭМ: микс

PetCare/src/index.html

Line 46 in 24d68e2

<p class="header__logo-text logo__text">PetCare</p>

Та же самая проблема, что с иконкой. Текст является частью блока logo, значит оставляем только класс logo__text

То же самое перетащилось в футер

Ссылкам, которые уводят с сайта, лучше ставить атрибут target="_blank"

PetCare/src/index.html

Line 76 in 24d68e2

<a class="social__link" aria-label="Инстаграмм" href="#"><svg width="40" height="40"

Слишком общее название для класса

PetCare/src/index.html

Line 124 in 24d68e2

<main class="page__main content">

Название content ничего плохого в себе не имеет, но все же практически не несет никакой смысловой нагрузки. На main можно было вообще не вешать никаких классов. А на секциях оставить только названия блоков, например, slider и cards. Тогда их не нужно было бы выделять в элементы страницы. Проще говоря, лишнее усложнение. Что опять же снижает читаемость кода

Повторение классов

PetCare/src/index.html

Line 130 in 24d68e2

<div class="banner__container desktop-grid">

grid и desktop-grid. Не нашла между ними никакой разницы, нужно оставить какой-то один

Структура заголовков

Тут, в целом, все хорошо, только h1 стоило бы дать какое-то более осмысленное название типа "Ветеринарная клиника PetCare".
Также
H2 Онлайн-консультация с терапевтом
H2 Сколько стоит онлайн-консультация с терапевтом? – это все части одного блока.

Тут скорее так:
H2 Сколько стоит онлайн-консультация с терапевтом?
H3 Онлайн-консультация с терапевтом

Блок в отрыве от контекста

PetCare/src/index.html

Line 199 in 24d68e2

<ul class="text-card__list benefits">

Мы создаем блоки, когда понимаем, что можем использовать их в другом месте в отрыве от контекста. Например, логотип действительно блок, потому что он может появляться на сайте где угодно и быть использован множество раз.

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

Структура бы выглядела так:

ul.text-card__list
  li.text-card__list-item
    svg.text-card__list-icon
    p.text-card__list-text

БЭМ: микс

PetCare/src/index.html

Line 21 in 24d68e2

<svg class="header__logo logo__icon" width="64" height="64" viewBox="0 0 64 64" fill="none"

Если элемент выделяется в отдельный блок, то его элементы становятся только элементами этого блока. То есть в данном случае должен был остаться только logo__icon

Та же проблема перетащилась в футер

Адаптив

1. Очень маленькие отступы боковые для десктопного разрешения

2. Вот такого не должно быть ни на каких промежуточных разрешениях. Всегда рассчитываем контейнер так, чтобы он был меньше ширины экрана пользователя. В данном случае это 1024px – это планшет.

3. Это 768px, также планшеты, причем по планшетам это разрешение самое популярное – более 52%. Для них вряд ли подойдет верстка под мобильные. В данном случае пользователь скорее всего покинет сайт, и будет прав. Так как все слишком мелко и неудобно.

4. Также на мобильных надо делать резиновую верстку всегда. Дизайнеры всегда будут давать какое-то одно мобильное разрешение, верстальщику же надо продумать, как будет выглядеть верстка и на 320px и на 420px. Оставлять контейнер 280px для больших мобильных экранов – bad practice.

5. Это просто случайно мною найденный забавный баг, что-то тут с контекстом наложения случилось

6. Расстояние от первого слайда до кнопок и от второго до кнопок разное, смотрится не очень. Это можно было избежать с помощью min-height

7. Лого какое-то маленькое совсем получилось. На макете пикселей на 30 больше

8. Между телефоном и эмэйлом потерялся отступ 8px