Skip to content

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

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

Бабич Людмила#76
Ludmilishee wants to merge 4 commits into
urfu-2016:masterfrom
Ludmilishee:master

Conversation

@Ludmilishee
Copy link
Copy Markdown

@Ludmilishee Ludmilishee commented Nov 4, 2016

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

Comment thread index.html
<link href="https://fonts.googleapis.com/css?family=Roboto+Slab" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Oranienbaum" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Ultra" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Playfair+Display+SC" rel="stylesheet">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Здорово! Но попробуй ещё какой-нибудь шрифт подключить через font-face

Comment thread index.html
</head>
<body>
<img src="images/logo.png" height="120" class="fig" alt="Logo">
<div class="block-2">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Для отдельных больших цитат обычно используют тег <blockquote>

Comment thread index.html
коллег по цеху.</q><br><br>
Артемий Лебедев
</div>
<header class="news-header-small">Новости дизайна</header>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Скорее это все один header, но в нем 2 заголовка)

Comment thread index.html
<hr class="main">
<main>
<section>
<header class="article-header">
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
</div>

</article>
<img src="images/gray.png" width="410" class="fig" alt="gray">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Здесь img лучше добавить в figure и при желании добавить figcaption. Также у изображений необходимо ещё указывать title

Comment thread index.css
{
font-family: 'Playfair Display SC', serif;
font-size: 20px;
text-align: center;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Это правило не работает для второго заголовка
image

Comment thread index.css

article:first-letter
{
font-family: 'Playfair Display SC', serif;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Все 3 правила объединяются в правило font

Comment thread index.css
background: #696969;
color: white;
display: inline-block;
float: right;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Зачем совмещать здесь разные способы построения колонок? Давай обойдемся здесь без float

Comment thread index.css
font-family: 'Oranienbaum', serif;
}

.block-1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ По названию непонятно, чем хорош этот блок-1

Comment thread index.css
font-family: 'Oranienbaum', serif;
}

aside
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ В html у тебя же нет aside?

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 2016

❗️ В Firefox столбцы немножко сдвинулись
image

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 2016

❗️ При разрешении 1280*960 столбцы тоже немного наезжают друг на друга в Chrome
image

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 2016

❗️ Edge думает, что здесь должна быть ещё одна колонка
image

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 2016

❗️ В Edge одна колонка накрывает другую
image

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 2016

Привет! В целом, хорошее решение, но есть некоторые замечания касательно семантики тегов и отображения. Молодец, что добавила мягкие переносы строк и использовала большое количество шрифтов. Обязательные для исправления замечания пометил "❗️". По всем вопросам можешь писать здесь или в слаке

@Dodo888
Copy link
Copy Markdown

Dodo888 commented Nov 5, 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.

3 participants