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

Шмонина Ирина #39

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

Conversation

IrinaShmonina
Copy link

@IrinaShmonina IrinaShmonina commented Nov 7, 2016

@honest-hrundel honest-hrundel changed the title IrinaShmonina Шмонина Ирина Nov 7, 2016
@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@IrinaShmonina
Copy link
Author

🍏

@dotokoto
Copy link

dotokoto commented Nov 9, 2016

Мне не очень нравится эффект при наведении на фото. Начинает скакать текст и плитки, которые находятся ниже. Лучше не увеличивать размер шрифта и сделать так, чтобы плитки не перескакивали. Можно поискать более интересные эффекты для фоток, но это по желанию :)

@dotokoto
Copy link

dotokoto commented Nov 9, 2016

  1. Скидка выделена красным, а "бесплатно" - нет. Но бесплатно - это же еще лучше, чем скидка, надо сделать заметнее )
  2. Не вижу длинных имен (по заданию длинные имена должны обрезаться)
  3. В виде "список" разъезжается цена. Да и в обычном виде тоже, надпись с одной стороны, а цифра совсем в другом месте
    2016-11-09 14-34-16

<div class="cat">
<h3>Даша</h3>
<img src="http://obretudom.ru/wp-content/files_mf/1478517418noidIMG_9882.JPG"
width="350" height="250" alt="cat1">
Copy link

Choose a reason for hiding this comment

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

alt не говорит ничего о картинке. Лучше написать "кошка Даша" или что-то в таком духе

<h3>Даша</h3>
<img src="http://obretudom.ru/wp-content/files_mf/1478517418noidIMG_9882.JPG"
width="350" height="250" alt="cat1">
<p><strong>Категория:</strong> котенок</p>
Copy link

Choose a reason for hiding this comment

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

Зачем strong? Он предназначен для выделения важного текста, на котором нужно акцентировать внимание

<span class="starEmpty">&#9734;</span>
</p>
<p><strong>Цена:</strong></p>
<p class="сost">Бесплатно.</p>
Copy link

Choose a reason for hiding this comment

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

Без точки выглядит лучше

text-align: center;
margin: 0;
padding: 0;

Copy link

Choose a reason for hiding this comment

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

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

overflow: hidden;
text-align: center;
margin: 0;
padding: 0;
Copy link

Choose a reason for hiding this comment

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

margin: 0; padding: 0; Встречается много раз. Можно объединить селекторы через запятую и один раз написать margin и padding, а не в каждом писать

{
margin: 4px;
width: 20%;
float: left;
Copy link

Choose a reason for hiding this comment

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

Эту задачу нужно решить без использования float. Можно сделать то же самое с помощью inline-block, если всё описание завернуть в еще один блок

@dotokoto
Copy link

dotokoto commented Nov 9, 2016

🍅

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@dotokoto
Copy link

🚀

@honest-hrundel honest-hrundel assigned meded90 and unassigned dotokoto Nov 13, 2016
@meded90
Copy link

meded90 commented Nov 14, 2016

2016-11-14 12-56-23

@meded90
Copy link

meded90 commented Nov 14, 2016

2016-11-14 12-57-14

@meded90
Copy link

meded90 commented Nov 14, 2016

у всех фотак нарущена пропорция

@meded90
Copy link

meded90 commented Nov 14, 2016

🍅

@honest-hrundel
Copy link

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

@meded90
Copy link

meded90 commented Nov 14, 2016

2016-11-14 18-55-24

белое поле не куда не делось

@meded90
Copy link

meded90 commented Nov 14, 2016

2016-11-14 18-56-32
так тоже не должнобыть

@meded90
Copy link

meded90 commented Nov 14, 2016

🍅

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@voychitskaya voychitskaya assigned trixartem and unassigned meded90 Nov 14, 2016
@IrinaShmonina
Copy link
Author

🍏

@trixartem
Copy link

Проблемы верстки:

  • При сжатии экрана карточки сжимаются, а не переносятся. Когда сжимаются не очень красиво выглядит.
  • Если нажимать на текст "Список"/"Обычный вид" то радиокнопки не переключаются
  • Звездочки не работают(

Copy link

@trixartem trixartem left a comment

Choose a reason for hiding this comment

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

🍅

body
{
min-width: 550px;
margin: 2%;

Choose a reason for hiding this comment

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

Для чего задавать вертикальные марджины в процентах?

.cat
{
width: 22.45%;
padding: 1%;

Choose a reason for hiding this comment

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

Зачем задавать вертикальные паддинги в процентах

<header>
<h1>Мяндекс.Муррркет</h1>
</header>
<div class="cat">

Choose a reason for hiding this comment

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

Не семантичная верстка

<br>
<h4>Цена:</h4>
<p class="newCost">Бесплатно</p>
<br>

Choose a reason for hiding this comment

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

Зачем?

<span class="starEmpty">&#9734;</span>
<span class="starEmpty">&#9734;</span>
</p>
<br>

Choose a reason for hiding this comment

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

для чего здесь br?

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.

5 participants