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

Гусева Зинаида #52

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

Conversation

zingus4
Copy link

@zingus4 zingus4 commented Oct 31, 2016

@honest-hrundel
Copy link

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

font-family: 'STIXGeneral';
}

table

Choose a reason for hiding this comment

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

Зачем переопределять тэги, если можно использовать классы)


.date
{
border: 3px;

Choose a reason for hiding this comment

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

Можно было в одну строку)


.head
{
border: 1px;

Choose a reason for hiding this comment

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

И тут

@savichev-igor
Copy link

В целом видно, что постаралась, но очень мало семантики в используемых тэгах, используй тэги header, section и т.д.

Также заметил, что газета болеет уже при ширине 900 :<

🍅

@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.

Немного замечаний, в остальном более-менее круто =)


p:first-letter
{
font-size: 24px;

Choose a reason for hiding this comment

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

В одну строку обычно)

</head>
<body>
<header>
<div class="head"><img src="love.png" alt="love" class="titleimg"></div>

Choose a reason for hiding this comment

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

Переноси их лучше, не пиши в одну строку

<div class="title"><h1>Сosmopolitan</h1></div>
<div class="head"><p><q>A woman who doesn't wear perfume has no future.</q> Coco Chanel</p>
</div>
<div class="date"><h2 class="when">ЕКАТЕРИНБУРГ, ПОНЕДЕЛЬНИК, 31 ОКТЯБРЯ, 1985 - 17

Choose a reason for hiding this comment

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

Что насчёт тэга <time> ?

Choose a reason for hiding this comment

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

Ну и не используй div'ы, если можно обойтись без них.

В 11-12 строчках, как мне кажется, они вообще ну нужны.

@savichev-igor
Copy link

🚀

@forshtreter
Copy link

В целом неплохо, хочется только вот с этим блоком что-нибудь сделать https://yadi.sk/i/UOrozscL32DSYN в вертикальном тексте пропали пробелы, а в тесте хочется вертикальных отступов между вопросами добавить. Ну и поправить оставшиеся замечания от Игоря :)

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