Skip to content

Трушков Валерий - #50

Open
flafla wants to merge 4 commits into
urfu-2016:masterfrom
flafla:master
Open

Трушков Валерий#50
flafla wants to merge 4 commits into
urfu-2016:masterfrom
flafla:master

Conversation

@flafla

@flafla flafla commented Nov 7, 2016

Copy link
Copy Markdown

@honest-hrundel

Copy link
Copy Markdown

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

@honest-hrundel

Copy link
Copy Markdown

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

@dotokoto

dotokoto commented Nov 9, 2016

Copy link
Copy Markdown
  1. В firefox обрезаются верхушки звездочек
    2016-11-09 15-52-46
  2. Нет эффектов при наведении
  3. Нет возможности посмотреть полноразмерную фотографию (этого не требуется по заданию. Но было бы удобно, если бы она была. Можешь добавить, но это не обязательно.)
  4. Если навести на меньший рейтинг, чем уже есть у кота, то звездочки не становятся прозрачными (хотя поставить меньший рейтинг можно, и тогда звездочки станут прозрачными).

Comment thread index.html Outdated
<section class="products">
<div class="products-item">
<div class="products-item-image">
<img src="./images/cat1.jpg" alt="котик">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не хватает title, height и width для картинок

Comment thread index.html
</a>
</div>
<ul class="categories">
<li><a href="#">cat</a></li>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем ссылки?

Comment thread index.html Outdated
dicta sequi accusantium!
</div>
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

лишняя пустая строка

Comment thread index.html Outdated
<input type="checkbox" id="style-switcher" name="style-switcher" value="list" title="list">
<label for="style-switcher">Выводить списком?</label>

<section class="products">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему section, а не main?

@dotokoto

dotokoto commented Nov 9, 2016

Copy link
Copy Markdown

Картинки котиков очень сильно сжимаются при уменьшении ширины экрана
2016-11-09 16-05-08

Comment thread index.css Outdated
input[name='style-switcher']:checked ~ .products .products-item-description
{
display: inline-block;
vertical-align: top;

Copy link
Copy Markdown

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;

Лучше объединить их в одном блоке, записав селекторы через запятую. А потом отдельно дописывать различающиеся значения

Comment thread index.css Outdated
.products-item
{
display: inline-block;
font-size: 1rem;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему именно rem?

Comment thread index.css Outdated
width: 31%;
}

.products-item .products-item-image img

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Может лучше поставить картинкам класс, и использовать селектор по одному этому классу?

Comment thread index.css Outdated

.products-item-info .price
{
margin: .31rem 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Часто повторяется это значение, надо убрать в один блок

Comment thread index.css Outdated
position: absolute;
right: 0;
text-indent: 0;
top: 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучше top, right, bottom и left записать подряд, а то они чередуются с остальными свойствами

@dotokoto

dotokoto commented Nov 9, 2016

Copy link
Copy Markdown

🍅

@honest-hrundel

Copy link
Copy Markdown

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

@dotokoto

dotokoto commented Dec 6, 2016

Copy link
Copy Markdown

Маловато эффектов при наведении, но в остальном с виду ок

@dotokoto

dotokoto commented Dec 6, 2016

Copy link
Copy Markdown

🚀

@honest-hrundel honest-hrundel assigned mokhov and unassigned dotokoto Dec 6, 2016
@mokhov

mokhov commented Dec 10, 2016

Copy link
Copy Markdown
Contributor

– Картинка и ссылка в заголовке должны быть единой ссылкой
– Ссылки должны реагировать на ховер изменением цвета

@mokhov

mokhov commented Dec 10, 2016

Copy link
Copy Markdown
Contributor

2016-12-10 17-08-55
Очень не нравятся большие дырки, которые появляются на больших экранах

@honest-hrundel

Copy link
Copy Markdown

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

@mokhov mokhov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🍅

Comment thread index.css
color: #ffdcbd;
}

.rank input:checked + label:hover,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Откуда взял это решение?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

css-tricks

Comment thread index.css Outdated
direction: rtl;
}

.rank legend

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Зачем нужен этот стиль? Нужно ответить, а не убрать эту строчку

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

не нужен он тут вовсе. легенда была, но ее не стало, а стили остались

Comment thread index.css Outdated

.rank input
{
position: absolute;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Зачем нужен position: absolute?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

лишний

@honest-hrundel

Copy link
Copy Markdown

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

@honest-hrundel

Copy link
Copy Markdown

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

@mokhov

mokhov commented Dec 13, 2016

Copy link
Copy Markdown
Contributor

2016-12-13 14-19-08
Пока картинки не загружены всё разъезжается

@mokhov

mokhov commented Dec 13, 2016

Copy link
Copy Markdown
Contributor

🍅

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