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

Бабич Людмила #28

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Ludmilishee
Copy link

@Ludmilishee Ludmilishee commented Nov 7, 2016

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

</head>
<body>
<input type="checkbox" class="output-switch">
<p>Включить отображение списком</p>
Copy link

Choose a reason for hiding this comment

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

❗️ Больше похоже на label, чем на p

</head>
<body>
<input type="checkbox" class="output-switch">
<p>Включить отображение списком</p>
<div class="cat">
Copy link

Choose a reason for hiding this comment

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

❗️ article

<img src="images/cat_01.jpg" alt="Мурзик">
</div>
<div class="cat-details">
<nav class="name-effect">
Copy link

Choose a reason for hiding this comment

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

❗️ Тег навигации обычно используется в навигации или меню)

</div>
<div class="cat-details">
<nav class="name-effect">
<p>
Copy link

Choose a reason for hiding this comment

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

❗️ Зачем ссылки оборачивать в p?

<input id="star1-0.5" type="radio" title="poor">
<label class="left-star" for="star1-0.5"></label>
</div>
<div class="price">
Copy link

Choose a reason for hiding this comment

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

❗️ Внутри каждого товара можно информацию ещё на секции побить)

{
padding-right: 20%;
display: inline-block;
*zoom: 1;
Copy link

Choose a reason for hiding this comment

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

Кажется, невалидное правило

-webkit-box-shadow: 4px 4px 4px rgba(0,0,0,.2);
-moz-box-shadow: 4px 4px 4px rgba(0,0,0,.2);
box-shadow: 4px 4px 4px rgba(0,0,0,.2);
-webkit-transition: all .5s ease-out;
Copy link

Choose a reason for hiding this comment

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

❗️ Почему на вебките другая функция перемещения?

box-shadow: 4px 4px 4px rgba(0,0,0,.2);
-webkit-transition: all .5s ease-out;
-moz-transition: all .5s ease;
-o-transition: all .5s ease;
Copy link

Choose a reason for hiding this comment

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

❗️ Обязательно указывать соответствующие правила без вендорных префиксов, даже если им никто не пользуется

text-align: left;
overflow: hidden;
text-overflow: ellipsis;
color: dimgrey;
Copy link

Choose a reason for hiding this comment

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

❗️ Лучше указывать цвета в hex

{
background-position: 0 0;
height: 37px;
width: 19px;
Copy link

Choose a reason for hiding this comment

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

👍 за спрайты

@Dodo888
Copy link

Dodo888 commented Nov 8, 2016

❗️ Более низкая оценка не ставится, если уже была поставлена более высокая

@Dodo888
Copy link

Dodo888 commented Nov 8, 2016

❗️ Разное поведение в разных браузерах
Chrome:
image
Edge и Firefox:
image

@Dodo888
Copy link

Dodo888 commented Nov 8, 2016

🚀

@honest-hrundel honest-hrundel assigned mokhov and unassigned Dodo888 Nov 8, 2016
@mokhov
Copy link
Contributor

mokhov commented Nov 10, 2016

Жду правок от Вовы

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