Landing_page #2049
Landing_page #2049
Conversation
There was a problem hiding this comment.
Good and well-structured project, but some minor improvements are needed
Please add the deploy link to your PR
Please make sure to follow BEM rule: one block = one file
Please remove external styling from BEM blocks (width, height, margins). Some exceptions are ok, but you have too many of these.
If you need a margin on your block, please add an extra element to the parent block and move this styling there.
The intent of blocks is reusability, and it is killed with such styling.
src/styles/blocks/buttons.scss
Outdated
| text-align: center; | ||
| border-width: 0; | ||
|
|
||
| width: 280px; |
There was a problem hiding this comment.
Avoid explicit width on BEM blocks
| h1, | ||
| h2, | ||
| h3, | ||
| h4, | ||
| h5, | ||
| h6, | ||
| p, | ||
| ul, | ||
| li { | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
This should go to a separate file - it is not related to page block
Alternatively - break this file by removing all blocks to their own files, and keeping this styling here
There was a problem hiding this comment.
якщо ми користуємось контейнером, стандартними відступами, заголовками і тд, що є спільним на усіх сторінках сайту, то для класів, що їх визначають, потрібно окремі файли? не дуже розумію як їх організцивати у проекті
src/styles/blocks/common.scss
Outdated
| } | ||
| } | ||
|
|
||
| .container { |
There was a problem hiding this comment.
separate block - separate file
src/styles/blocks/common.scss
Outdated
|
|
||
| .container { | ||
| padding-inline: 20px; | ||
| width: 280px; |
There was a problem hiding this comment.
again - no explicit width on BEM blocks
src/styles/blocks/common.scss
Outdated
| } | ||
| } | ||
|
|
||
| .main { |
There was a problem hiding this comment.
this is a separate block as well
BEM is one block - one file
src/styles/blocks/header.scss
Outdated
| flex-direction: column; | ||
| justify-content: flex-end; | ||
|
|
||
| margin-top: -72px; |
There was a problem hiding this comment.
don't set explicit margins on blocks
There was a problem hiding this comment.
потрібно створити обгортку, додатковий блок, вірно ж? а де потрібно надавати для них властивості і значенння цим властивостям, наприклад, у файлі common?
There was a problem hiding this comment.
Можна додати ці відступи у батьківский блок, або створити елемент у батьківському блоці
Типу такого
.parent {
&--margins {
margin-top: 10px;
}
}
. child {
...
}
usage:
<div class="parent">
<div class="child parent--margins">
content
</div>
</div>
| } | ||
| } | ||
|
|
||
| .menu { |
There was a problem hiding this comment.
Please move this block to a separate file
src/styles/blocks/menu.scss
Outdated
| box-sizing: border-box; | ||
|
|
||
| height: 100vh; | ||
| background-color: #f9f9f9; |
src/styles/blocks/recommended.scss
Outdated
| @@ -0,0 +1,65 @@ | |||
| .recommended { | |||
| margin-bottom: 96px; | |||
| &--limited { | ||
| width: 280px; |
DEMO LINK
B&O