Skip to content

Conversation

@slavavikharev
Copy link

@slavavikharev slavavikharev commented Nov 8, 2016

@honest-hrundel honest-hrundel changed the title Вихарев Вячесалав Вихарев Вячеслав Nov 8, 2016
@honest-hrundel
Copy link

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

Copy link

@savichev-igor savichev-igor left a comment

Choose a reason for hiding this comment

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

Переключалка неинтересная
image

А тут картинка вообще болеет
image

Выглядит очень раздражающе, когда одинаковый контент прыгает
image

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="13" height="13" viewBox="0 0 13 13"><path fill="#FAB600" d="M6.491 0l1.509 5h5l-4 3 2 5-4.5-3-4.5 3 2-5-4-3h5z"/></svg> No newline at end of file

Choose a reason for hiding this comment

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

SVG, найс

Copy link

Choose a reason for hiding this comment

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

Этот svg нигде не используется. Неиспользуемых файлов быть не должно!

.item-category
{
display: inline-block;
color: #666;

Choose a reason for hiding this comment

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

😈


.star-label
{
float: right;

Choose a reason for hiding this comment

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

float нельзя использовать

<div class="card-top">
<img class="item-pic" src="img/1.jpg" alt="1">
</div>
<div class="card-middle">

Choose a reason for hiding this comment

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

Слишком много div'ов, избавляйся от них

@savichev-igor
Copy link

🚀

Copy link

@sladex sladex left a comment

Choose a reason for hiding this comment

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

4, 8 и 9 картинки не отображаются скрин

Странице не хватает главного заголовка (на какую страницу мы попали?).

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="13" height="13" viewBox="0 0 13 13"><path fill="#FAB600" d="M6.491 0l1.509 5h5l-4 3 2 5-4.5-3-4.5 3 2-5-4-3h5z"/></svg> No newline at end of file
Copy link

Choose a reason for hiding this comment

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

Этот svg нигде не используется. Неиспользуемых файлов быть не должно!


<section class="item-card">
<div class="card-top">
<img class="item-pic" src="img/1.jpg" alt="1">
Copy link

Choose a reason for hiding this comment

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

"1" - плохой альтернативный текст для картинки

</fieldset>
<div class="item-price">
<h2 class="current-price">£750</h2>
<h3 class="old-price">£800</h3>
Copy link

Choose a reason for hiding this comment

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

Какие же это заголовки?
Тут ты используешь h2/h3 для получения крупного текста. Так делать не нужно - заголовки только для заголовков!

<li class="item-category"><a href="#">British Shorthair</a></li>
<li class="item-category"><a href="#">Chelmsford</a></li>
</ul>
<fieldset class="item-stars">
Copy link

Choose a reason for hiding this comment

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

Кажется ты спутал figure с fieldset :)
http://htmlbook.ru/html/fieldset
http://htmlbook.ru/html/figure

fieldset имеет смысл только внутри форм.

</div>
</div>
<div class="card-bottom">
<p class="item-description">
Copy link

Choose a reason for hiding this comment

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

item-description нет в css. Неиспользуемых классов быть не должно, как и лишних оберток.

2 элемента:
<div class="card-bottom"><p class="item-description">...
тоже самое одним:
<p class="card-bottom">...

<img class="item-pic" src="img/1.jpg" alt="1">
</div>
<div class="card-middle">
<div class="item-info">
Copy link

Choose a reason for hiding this comment

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

item-info нет в css. Неиспользуемых классов быть не должно.

{
display: inline-block;
vertical-align: top;
height: 100%;
Copy link

Choose a reason for hiding this comment

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

У родителя (.item-card) не задана высота, соответственно height: 100% здесь никаким образом на верстку не повлияет.

@sladex
Copy link

sladex commented Nov 14, 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