Skip to content
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

Антонов Алексей #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlekseyAnt
Copy link

@AlekseyAnt AlekseyAnt commented Nov 6, 2016

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

1 similar comment
@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

index.css Outdated
.h-view:checked ~ .main .cat
{
display: block;
width: 1000px;

Choose a reason for hiding this comment

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

Когда показывается списком, то если разрешение экрана у пользователя меньше 1000px, он будет расстроен

index.css Outdated
margin: 15px 0;
}

.h-view:checked ~ .main .img-content

Choose a reason for hiding this comment

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

Давай переименуем h-view и v-view на более понятные названия

index.css Outdated

.h-view:checked ~ .main .description
{
width: 400px;

Choose a reason for hiding this comment

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

Давай сделаем верстку поадаптивнее, например, можно посмотреть в сторону min-width или max-width

<h1 class="logo">Мяндекс.<span>Муррркет</span></h1>
<p>
<label for="vertical">
<img src="images/v-view.jpg" alt="Горизонтальные плитки" title="Горизонтальные плитки">

Choose a reason for hiding this comment

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

Молодец, что сделал не обычным инпутом 👍

@bazhenova
Copy link

Привет. Мне очень понравилась твоя работа) Есть пара некритичных замечаний, но я надеюсь, что ты их поправишь 😉 Еще есть последний вопрос: будешь ли делать звездочки?

@bazhenova
Copy link

🚀

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

index.html Outdated
<main class="main">
<div class="cat male">
<div class="img-content">
<img src="images/01.jpg" alt="" title="Генрих 4">

Choose a reason for hiding this comment

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

Пустой alt


.price:after
{
content: '₽';

Choose a reason for hiding this comment

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

В случае с ценой так делать, пожалуй, не стоит, потому что текст в псевдоэлементе нельзя выделить и скопировать. Но в целом пододное внимание к мелочам – это очень хорошо 👍 Так что можно не исправлять :)

index.html Outdated
</p>
</header>
<main class="main">
<div class="cat male">

Choose a reason for hiding this comment

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

Алексей, многовато div'ов. Здесь, например, подойдёт article, для обёртки картинки figure, для имени кота – заголовок.

@forshtreter
Copy link

🍅

@forshtreter
Copy link

Пока помидор, но в целом очень даже неплохо, по звёздочке вобще никаких вопросов, только мелочёвку подправить.

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

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.

4 participants