Skip to content

Рахимянов Эмиль#11

Open
trszdev wants to merge 11 commits into
urfu-2017:masterfrom
trszdev:master
Open

Рахимянов Эмиль#11
trszdev wants to merge 11 commits into
urfu-2017:masterfrom
trszdev:master

Conversation

@trszdev
Copy link
Copy Markdown

@trszdev trszdev commented Oct 21, 2017

@honest-hrundel honest-hrundel changed the title xxxx Рахимянов Эмиль Oct 21, 2017
@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@Mokoshka Mokoshka left a comment

Choose a reason for hiding this comment

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

В Firefox немного не заработало
delorean dmc-12 2017-10-24 15-56-55

Comment thread index.html Outdated
<body>
<main>
<h1>Lorem Ipsum</h1>
<div class="firstLine">
Copy link
Copy Markdown

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.html Outdated
</article>
<article>
<h3>Section 1.10.32 of "de Finibus Bonorum et Malorum", written by Cicero in 45 BC</h3>
<div class="vertical">
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
dolorem eum fugiat quo voluptas nulla pariatur?
</p>
<figure>
<img src="lenna.jpg" alt="lenna.jpg">
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 укажем для картинки

Comment thread index.css Outdated
word-break: break-all;
border-radius: 5px;
border: 1px solid black;
height: auto;
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
width: 1em;
word-break: break-all;
border-radius: 5px;
border: 1px solid black;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Давай использовать hex-код цветов

Comment thread index.css Outdated
letter-spacing: .2em;
margin: 10px;
padding: 3px;
font-family: 'Abril Fatface';
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
font-family: 'HFF Clip Hanger';
}

article:nth-child(1) > h3
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-bobo-dylan. Тогда, если ты захочешь еще в каком то элементе использовать шрифт, тебе надо будет всего лишь указать класс нужный.
Предлагаю все селекторы с nth-child переписать таким образом

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@Mokoshka Mokoshka left a comment

Choose a reason for hiding this comment

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

Заголовок в конце колонки выглядит не очень. Давай сделаем так, чтобы заголовок всегда был в одной колонке со своей статьей
delorean dmc-12 2017-11-03 16-26-49

Comment thread index.css Outdated
border-top: 1px #000 solid;
}

section article:nth-child(2n+1) blockquote
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь nth-child оставил. Использование класса значительно упростит селектор и повторное использование таких границ. Например, понадобится использовать такой стиль в другом порядке и тогда надо будет лишь проставить классы у нужных элементов, а не менять селектор. Ведь изменить класс проще?

Comment thread index.css Outdated
word-break: break-all;
border-radius: 5px;
border: 1px solid #000;
height: auto;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Объясни, пожалуйста, зачем ты здесь используешь height: auto?

Comment thread index.css Outdated

.vertical
{
display: inline-block;
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
<head>
<meta charset="utf-8">
<title>Задача «DeLorean DMC-12»</title>
<link href="http://tinyurl.com/ycoym3ls" 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.

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

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.

Там используются как шрифты из репозитория так и с гугла

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не вижу в этом проблемы. Почему бы просто не подключить те же шрифты?

eyebrow elite added 2 commits November 6, 2017 16:46
@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

@honest-hrundel
Copy link
Copy Markdown

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

@trszdev
Copy link
Copy Markdown
Author

trszdev commented Nov 6, 2017

Помоги пожалуйста с заголовком в конце колонки. Я пробовал break-after, но он в firefox не работает(( Пробовал article {display:inline-block}, но тогда внутри нет переносов(

@Mokoshka
Copy link
Copy Markdown

Mokoshka commented Nov 6, 2017

  • Сделай break-after. Это будет верным решением
  • Еще сделай так, чтобы длинный заголовок не переносило
    image
  • Переносов не хватает в тексте, а очень хочется

@honest-hrundel
Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants