-
Notifications
You must be signed in to change notification settings - Fork 86
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
Рахимянов Эмиль #11
base: master
Are you sure you want to change the base?
Рахимянов Эмиль #11
Conversation
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройден линтинг и базовые тесты |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index.html
Outdated
<body> | ||
<main> | ||
<h1>Lorem Ipsum</h1> | ||
<div class="firstLine"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Может быть можно использовать что-то более семантичное?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И здесь можно использовать более семантичный элемент
index.html
Outdated
dolorem eum fugiat quo voluptas nulla pariatur? | ||
</p> | ||
<figure> | ||
<img src="lenna.jpg" alt="lenna.jpg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай title укажем для картинки
index.css
Outdated
word-break: break-all; | ||
border-radius: 5px; | ||
border: 1px solid black; | ||
height: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется это лишнее
index.css
Outdated
width: 1em; | ||
word-break: break-all; | ||
border-radius: 5px; | ||
border: 1px solid black; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай использовать hex-код цветов
index.css
Outdated
letter-spacing: .2em; | ||
margin: 10px; | ||
padding: 3px; | ||
font-family: 'Abril Fatface'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ты используешь шрифты, которые не подключил. У меня, например, они не отображаются)
index.css
Outdated
font-family: 'HFF Clip Hanger'; | ||
} | ||
|
||
article:nth-child(1) > h3 |
There was a problem hiding this comment.
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 переписать таким образом
🍏 Пройден линтинг и базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройден линтинг и базовые тесты |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index.css
Outdated
border-top: 1px #000 solid; | ||
} | ||
|
||
section article:nth-child(2n+1) blockquote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь nth-child оставил. Использование класса значительно упростит селектор и повторное использование таких границ. Например, понадобится использовать такой стиль в другом порядке и тогда надо будет лишь проставить классы у нужных элементов, а не менять селектор. Ведь изменить класс проще?
index.css
Outdated
word-break: break-all; | ||
border-radius: 5px; | ||
border: 1px solid #000; | ||
height: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Объясни, пожалуйста, зачем ты здесь используешь height: auto?
index.css
Outdated
|
||
.vertical | ||
{ | ||
display: inline-block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем этот элемент делать инлайновым?
index.html
Outdated
<head> | ||
<meta charset="utf-8"> | ||
<title>Задача «DeLorean DMC-12»</title> | ||
<link href="http://tinyurl.com/ycoym3ls" rel="stylesheet"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А давай не будем пользоваться чем-то уже готовым, а сами подключим нужные тебе шрифты. Пока что это курсы и здесь надо научиться понимать как все работает
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Там используются как шрифты из репозитория так и с гугла
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не вижу в этом проблемы. Почему бы просто не подключить те же шрифты?
🍅 Не пройден линтинг или базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройден линтинг и базовые тесты |
Помоги пожалуйста с заголовком в конце колонки. Я пробовал break-after, но он в firefox не работает(( Пробовал article {display:inline-block}, но тогда внутри нет переносов( |
🍅 Не пройден линтинг или базовые тесты |
Посмотреть решение