-
Notifications
You must be signed in to change notification settings - Fork 36
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
Козлова Валерия #13
base: master
Are you sure you want to change the base?
Козлова Валерия #13
Conversation
🍅 Не пройден линтинг или базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройден линтинг и базовые тесты |
🍏 Пройден линтинг и базовые тесты |
<div class="content"> | ||
<div class="img"> | ||
<label for="card-2"> | ||
<img src="images/img2.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.
❗️ alt
не должен быть пустым. Изображение можно назвать понятней)
<input type="radio" name="gallery" id="card-6" class="card-6"> | ||
<input type="radio" name="gallery" id="card-7" class="card-7"> | ||
<br> | ||
|
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.
❗️ Хочется семантичных тегов
<div class="card card-6"> | ||
<input type="checkbox" id="show-recipe-6"> | ||
<div class="content"> | ||
<div class="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.
❗️ Можно придумать более конкретные названия для классов, чем content
и img
.
<h1>Название рецепта</h1> | ||
<div> | ||
<div class="ingredients"> | ||
Ингридиенты |
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.
Ингредиенты
</div> | ||
<div class="recipe-description"> | ||
<label class="close" for="show-recipe-5">X</label> | ||
<h1>Название рецепта</h1> |
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.
❗️ h1
может быть на странице только 1 и должен быть заголовком всей страницы
transition: all .4s ease-in-out; | ||
box-sizing: border-box; | ||
padding-top: 30%; | ||
z-index: -1; |
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.
А зачем z-index, если и так opacity: 0
?
z-index: 10; | ||
} | ||
|
||
body |
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: relative
здесь?
z-index: 11; | ||
} | ||
|
||
.recipe-description |
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.
❗️ Повторяющийся селектор
|
||
.recipe-description | ||
{ | ||
display: flex; |
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.
❗️ Хм, зачем ты используешь flex, если сам крестик направо перемещаешь с помощью text-align
?
.ingredients, | ||
.steps | ||
{ | ||
width: 50%; |
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 class="show-recipe" type="checkbox" id="show-recipe-5"> | ||
<input class="show-recipe" type="checkbox" id="show-recipe-6"> | ||
<input class="show-recipe" type="checkbox" id="show-recipe-7"> | ||
<div class="card card-1"> |
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.
❗️ Можно все же использовать семантичные теги main
, article
, section
<input type="checkbox" id="show-recipe-3"> | ||
<div class="content"> | ||
<div class="img"> | ||
<label for="card-3"> |
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.
И все-таки?
❗️ При анимации закрытия карточка мгновенно становится маленькой, потом переворачивается. Уменьшение тоже должно происходить плавно |
Карточка теперь выглядит очень классно) Оставил замечания |
🚀 |
Когда карточка закрывается, то происходят какие-то рывки и анимация совсем не похожа на анимацию открытия. |
🍅 |
Посмотреть решение