Skip to content

завершение сайта#6

Open
dotmirzaev wants to merge 1 commit intomasterfrom
task-5.1
Open

завершение сайта#6
dotmirzaev wants to merge 1 commit intomasterfrom
task-5.1

Conversation

@dotmirzaev
Copy link
Owner

No description provided.

Copy link

@captain-zsa captain-zsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не надо вырезать идентичные картинки.
source/img/banner_arrow-left.svg
и
source/img/banner_arrow-right.svg
ничем не отличаются. Вырежите одну и просто поверните на 180 градусов с помощью transform: rotate(180deg). Это гораздо эффективнее, чем вырезать несколько картинок. Ровно также и с другими картинками.

Также нет никакого смысла вырезать Slick несколько раз. У вас идентичная картинка, меняются только размеры. Это svg-картинка (векторная). Вектор без проблем может увеличиваться/уменьшаться как угодно. Просто установить нужный размер ему и всё (либо через стили либо через img, зависит от того, как вы его вставляете, через фон или через img-тег)

background-size: 58%;
padding-bottom: 100px;

&::before {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем вы снова пишете всё тоже самое? У вас стили от планшетного разрешения применились и на десктопное. Если что-то на десктопе надо заменить – то прям это и напишите, чтоб заменить. Но никаких position: absolute; и прочего не нужно совершенно. Они и так уже есть на десктопе, перекочевали с планшета. Не надо так.

}
}

&__slider-box-arrow-icon-left {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вам не кажется, что название класса катастрофически огромно? Более того у вас обе стрелки на 100% одинаковы. Всё что надо было сделать – назвать их banners__arrow и применить общие стили, а потом в модификаторах banners__arrow--left и banners__arrow--right просто добавить нужные отличительные стили. И названия классов будут оооочень сильно короче и понятнее, и кода будет в разы меньше.

}
}

&__list-anim {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Опять пишете один в один такие же стили, что и для селектора выше. Зачем это дублировать то? Почему нельзя применять один класс и отличие писать просто в модификаторе? Вы ведь за счет этого раздуваете код до огромных размеров.

&__slider-box-arrow-icon-left {

@media (min-width: 768px) {
position: absolute;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Проблемы с табуляцией и опять же это невменяемо-огромные названия классов. Сокращать как в баннерах всё надо и делать модификаторы, а не отдельные элементы. Не раздувайте код на ровном месте. Думайте, где можно его сокращать, а не просто писать по 100 раз одно и то же. Вам не влом это делать? Сидеть тож самое писать?

span {
@media (max-width: 767px) {
display: none;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Табуляция

font-size: 28px;
font-weight: 600;
line-height: 43px;
background-color: black;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

НИКОГДА не испоьзуем цвет как название. Только hex или rgba форматы.

&__button {
@include clear-button;

background-color: black;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Аналогично. И в остальных местах тоже. Почему так – потому что вы так запутаетесь в момент. Завтра надо будет поменять везде черный на серый – вы будете искать #000 по проекту (или другой разработчик), часть найдет, а часть останется, потому что оно как black записано. Палитра цветов таких крайне скудная. Их катастрофически не хватает и поэтому мы всегда используем ТОЛЬКО hex или rgba форматы.

@import 'root.scss';
@import 'blocks/container.scss';
@import 'blocks/header.scss';
@import 'blocks/show-all.scss';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вы сейчас просто подряд тыкаете сюда файлы как попало. Так нельзя делать. Почему – потому что допустим social появится в header, что вы будете делать, чтобы переопределить часть его стилей там? Ничего не сделаете, кроме как придется повышать специфичность.

То есть будет

.header__social {
   margin: 10px;
}

и

.social {
   margin: 0;
}

вы ожидаете что в header сработает именно margin: 10px; но это не так. Он НЕ СРАБОТАЕТ потому что у вас social подключен ПОЗЖЕ чем header. Внутрь файла css итогового, который собирается с помощью gulp из ваших файлов SCSS, попадут стили эти последовательно так, как вы их подключали и header будет выше, чем social. А чем ниже код – тем он приоритетнее.

Поэтому правило элементарнейшее – чем меньше внутри блока других блоков (или их вообще не предвидится) тем выше должен быть файл подключен. Надеюсь, суть понятна.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants