-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Трушков Валерий #50
base: master
Are you sure you want to change the base?
Трушков Валерий #50
Conversation
🍏 Пройден линтинг и базовые тесты |
🍏 Пройден линтинг и базовые тесты |
<section class="products"> | ||
<div class="products-item"> | ||
<div class="products-item-image"> | ||
<img src="./images/cat1.jpg" alt="котик"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не хватает title, height и width для картинок
</a> | ||
</div> | ||
<ul class="categories"> | ||
<li><a href="#">cat</a></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем ссылки?
dicta sequi accusantium! | ||
</div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишняя пустая строка
<input type="checkbox" id="style-switcher" name="style-switcher" value="list" title="list"> | ||
<label for="style-switcher">Выводить списком?</label> | ||
|
||
<section class="products"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему section, а не main?
input[name='style-switcher']:checked ~ .products .products-item-description | ||
{ | ||
display: inline-block; | ||
vertical-align: top; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Часто повторяются значения
display: inline-block;
vertical-align: top;
Лучше объединить их в одном блоке, записав селекторы через запятую. А потом отдельно дописывать различающиеся значения
.products-item | ||
{ | ||
display: inline-block; | ||
font-size: 1rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему именно rem?
width: 31%; | ||
} | ||
|
||
.products-item .products-item-image img |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Может лучше поставить картинкам класс, и использовать селектор по одному этому классу?
|
||
.products-item-info .price | ||
{ | ||
margin: .31rem 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Часто повторяется это значение, надо убрать в один блок
position: absolute; | ||
right: 0; | ||
text-indent: 0; | ||
top: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше top, right, bottom и left записать подряд, а то они чередуются с остальными свойствами
🍅 |
🍏 Пройден линтинг и базовые тесты |
Маловато эффектов при наведении, но в остальном с виду ок |
🚀 |
– Картинка и ссылка в заголовке должны быть единой ссылкой |
🍏 Пройден линтинг и базовые тесты |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍅
color: #ffdcbd; | ||
} | ||
|
||
.rank input:checked + label:hover, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Откуда взял это решение?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
css-tricks
direction: rtl; | ||
} | ||
|
||
.rank legend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем нужен этот стиль? Нужно ответить, а не убрать эту строчку
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не нужен он тут вовсе. легенда была, но ее не стало, а стили остались
|
||
.rank input | ||
{ | ||
position: absolute; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем нужен position: absolute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишний
🍏 Пройден линтинг и базовые тесты |
🍏 Пройден линтинг и базовые тесты |
🍅 |
Посмотреть решение