Skip to content

Comments

Козлов Александр#57

Open
KozlovAlexanderS wants to merge 6 commits intourfu-2016:masterfrom
KozlovAlexanderS:master
Open

Козлов Александр#57
KozlovAlexanderS wants to merge 6 commits intourfu-2016:masterfrom
KozlovAlexanderS:master

Conversation

@KozlovAlexanderS
Copy link

@KozlovAlexanderS KozlovAlexanderS commented Oct 31, 2016

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

index.css Outdated
@import url(https://fonts.googleapis.com/css?family=UnifrakturCook:700);
@import url(https://fonts.googleapis.com/css?family=Fira+Mono:400,700);
@import url(https://fonts.googleapis.com/css?family=IM+Fell+English:400,400italic);
@import url(https://fonts.googleapis.com/css?family=Bree+Serif);
Copy link

Choose a reason for hiding this comment

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

это хорошо, но давай попробуем подключать шрифты с помощью font-face. И не забудь про то, что они должны быть кроссбраузерными

h4,
h5,
h6
{
Copy link

Choose a reason for hiding this comment

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

Лучше класс, потому что, например ты напишешь такой код

{
width: 100px;
height: 100px;
margin-top: 10px;
}

И все картинки на твоей странице будут такого размера и с таким отступом сверху. А чаще всего, на странице картинки разных размеров, и удобно выделять их в классы и задавать уже каждому классу нужные параметры. Так ты во первых, сразу знаешь, где искать если что ошибку. Во-вторых, если ты будешь использовать и классы и теги, то как ты надеюсь помнишь, у тега меньше специфичность, и можно очень легко и много сделать ошибок если использовать и то, и другое. А чаще всего совсем без классов не обойтись

@Lakate
Copy link

Lakate commented Nov 1, 2016

как-то странно выглядит. Давай либо на обеих строках такой отступ, либо совсем без него
screenshot at 01 22-00-11

-moz-column-rule: 2px solid #000;
}

.first-img
Copy link

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

тут не исправлено

@Lakate
Copy link

Lakate commented Nov 1, 2016

кажется это такой шрифт, но тоже странненько смотрится
screenshot at 01 22-01-34

@Lakate
Copy link

Lakate commented Nov 1, 2016

смотрю в firefox:
screenshot at 01 22-01-47
screenshot at 01 22-02-06

@Lakate
Copy link

Lakate commented Nov 1, 2016

привет!
посмотрела, как исправишь замечания - пиши в slack @Lakate

@Lakate
Copy link

Lakate commented Nov 1, 2016

🍅

@honest-hrundel
Copy link

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

1 similar comment
@honest-hrundel
Copy link

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

}

header
{
Copy link

Choose a reason for hiding this comment

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

не пишем стили к тегам, создай для этого класс

со­стя­за­ни­ем Фе­де­ра­ции в 2016 го­ду».
</p>
</div>
<h3 class="head3">Федерация компьютерного спорта — это еще что?</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, h4, h5, h6) правильно использовать так: h2 ты правильно использовал для названия статьи. Остальные нельзя тут использовать, потому что ты с их помощью просто выделяеь визуально текст. Либо разбей это на несколько маленьких статей и каждая с таким заголовком, либо создавай классы и с их помощью выделяй какие-то фразы

-moz-column-rule: 2px solid #000;
}

.first-img
Copy link

Choose a reason for hiding this comment

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

тут не исправлено

font-family: 'NewRocker', cursive;
}

article
Copy link

Choose a reason for hiding this comment

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

заведи класс

align-content: center;
}

.first
Copy link

Choose a reason for hiding this comment

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

имена классов должны быть говорящими. Что за first? первый сталбик? первый абзац? первый заголовок?

text-indent: 20px;
}

img
Copy link

Choose a reason for hiding this comment

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

  1. используй класс
  2. про размеры картинок не забывай

border-top: 1px solid #000;
border-bottom: 1px solid #000;
font-family: 'IM FELL English PRO Roman', serif;
font-size: 32px;
Copy link

Choose a reason for hiding this comment

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

а почему везде только точные значения шрифта? попробуй еще использовать em

font-family: 1.1em 'Bree Serif', serif;
line-height: 20px;
padding-top: 10px;
float: right;
Copy link

Choose a reason for hiding this comment

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

это задание нужно выполнять без плавающих элементов


.more
{
font: 1.1em 'IM FELL English PRO Roman', cursive;
Copy link

Choose a reason for hiding this comment

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

Олег рассказывал вам, что нужно стили, в которых указывается шрифт писать как можно раньше. Потому что браузер не начнет загружать шрифт до тех пор, пока не встретить с ними стили. Поправь везде пожалуйста

@Lakate
Copy link

Lakate commented Nov 13, 2016

🍅

@Lakate
Copy link

Lakate commented Nov 13, 2016

пиши пожалуйста в slack. А то видишь как долго получается

@Lakate
Copy link

Lakate commented Nov 13, 2016

и вот еще много пустого места
screenshot at 13 12-25-27
screenshot at 13 12-24-19

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