Evgeniybalagurov/#10 #22
Conversation
Pavel-Rogozhkin
left a comment
Pavel-Rogozhkin
left a comment
There was a problem hiding this comment.
Огонь!! Огромная работа проделана! Молодец, Женя!)
После всех ревью надо будет зарезолвить все конфликты.
.vscode/settings.json
Outdated
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "liveServer.settings.port": 5501 | |||
There was a problem hiding this comment.
предлагаю пока этот файл и папку оставлять локально и добавить в .gitignore
There was a problem hiding this comment.
Добавил в .gitignore
| @@ -0,0 +1,57 @@ | |||
| .link { | |||
There was a problem hiding this comment.
этот файл у нас много где будет использоваться, правильно же понимаю?
если да, то надо будет после полной сборки протестировать работу всех подобных ссылок.
There was a problem hiding this comment.
Да, будет использоваться. Ок, протестим
| @@ -0,0 +1,4 @@ | |||
| .projects { | |||
| display: grid; | |||
| grid-template: 94px auto auto / repeat(2, 1fr); | |||
| .text-block__desc { | ||
| margin: 0; | ||
| max-width: 590px; | ||
| font-family: 'Neue Machina'; |
There was a problem hiding this comment.
мы кстати будем везде резервные шрифты добавлять? Ариал, саншериф там.
There was a problem hiding this comment.
Кстати да, нужно будет добавить. Я думаю будет достаточно только sans serif.
| line-height: 1; | ||
| letter-spacing: -0.01em; | ||
| color: var(--text-color); | ||
| flex-grow: 1; |
There was a problem hiding this comment.
ненужное свойство было до тебя добавлено?
There was a problem hiding this comment.
Да, оно не требуется
DmitriyMGN
left a comment
DmitriyMGN
left a comment
There was a problem hiding this comment.
Классно сделал, хороший четкий код. Есть несколько рекомендаций по стайлкодингу. Молодец!
| .lead__link { | ||
| margin-top: 16px; | ||
| padding: 16px; | ||
| width: 240px; |
There was a problem hiding this comment.
старайся писать в стилях сначала размеры, потом отступы, далее другие стили относящиеся к этому селектору.
В методолгии стайлкодинга, которую нам давали на первом спринте этот пунктик есть с ранжированием стилей в селекторе, в целом это не влияет никак на функционал, но структурированный код это всегда круто)
| align-items: center; | ||
| border-bottom: 1px solid var(--border-color); | ||
| border-right: 1px solid var(--border-color); | ||
| grid-column: 1 / 3; |
There was a problem hiding this comment.
стоит display: flex, а далее по коду grid-column: 1 / 3, не лишнее свойство?
| @@ -1,4 +1,5 @@ | |||
| .text-block_placed_slider-title { | |||
| max-width: 718px; | |||
| flex-direction: row; | |||
There was a problem hiding this comment.
Переопределяешь свойство? Если нет, то значение row по умолчанию и тут не требуется
Сверстал секцию projects под desktop.