Conversation
devmentor-pl
left a comment
devmentor-pl
left a comment
There was a problem hiding this comment.
Marcelu,
Projekt prezentuje się super! 👍
Zostawiłem parę drobnych uwag, ale to raczej kosmetyka ;)
assets/style.css
Outdated
| width: 60%; | ||
| } | ||
|
|
||
| .hero__column--wider > img { |
There was a problem hiding this comment.
Lepiej nadać obrazkowi klasę i po niej stylować - w BEM jest takie zalecenie
assets/style.css
Outdated
| padding: 40px 0 80px 0; | ||
| } | ||
|
|
||
| .features__title, .pricing__title, .team__title { |
There was a problem hiding this comment.
Zamiast grupować można też stworzyć nowy Block lub Element i do niego przypisać wspólne cech np. .header lub title
index.html
Outdated
| </nav> | ||
| </header> | ||
|
|
||
| <div class="hero"> |
There was a problem hiding this comment.
Można by było się zastanowić nad znacznikiem article, bo mamy jedną spójną treść - niewielką, ale jest :P
index.html
Outdated
| </div> | ||
|
|
||
| <div class="hero__column--wider"> | ||
| <img src="/images/screen.png" alt="screen"> |
There was a problem hiding this comment.
Zwróć uwagę, że między obramowaniem, a obrazkiem jest prześwit - warto go usunąć - wystarczy nadać białe tło :)
index.html
Outdated
| <div class="features__container"> | ||
|
|
||
| <div class="features__element element"> |
There was a problem hiding this comment.
Zastanawim się czy bym tutaj nie zrobił ul > li, bo mamy "listę" cech czyli "...container" to ul, a "element" to li
index.html
Outdated
| <p class="plan__description">Up to 7 Projects</p> | ||
| <p class="plan__description">2 Additional Developers</p> |
There was a problem hiding this comment.
Tutaj zdecydowanie ul > li jako lista elementów dot. danego pakietu cenowego
index.html
Outdated
| <br> | ||
| <br> |
There was a problem hiding this comment.
Odstępów nie robimy przy pomocy br tylko margin/padding lub inny sposób. Ten znacznik jest używany tylko do łamania tekstu.
index.html
Outdated
| </div> | ||
|
|
||
| <div class="pricing__container"> | ||
| <!-- PONIZSZE ELEMENTY ALTERNATYWNIE DO ZROBIENIA ZA POMOCĄ UL I LI --> |
There was a problem hiding this comment.
Potwierdzam, można też dla plan zrobić article - jako jedna spójna treść, bo przecież jeden pakiet taką treści jest czyli mielibyśmy x3 article
index.html
Outdated
| <div class="team__container"> | ||
|
|
||
| <div class="team__member member"> |
There was a problem hiding this comment.
TUtaj też może być lista członków zespołu więc ul > li
index.html
Outdated
| </div> | ||
| </section> | ||
|
|
||
| <section class="footer"> |
devmentor-pl
left a comment
devmentor-pl
left a comment
There was a problem hiding this comment.
Marcelu,
Zostały jeszcze 2 uwagi :)
| } | ||
|
|
||
| .hero__column--wider > img { | ||
| .hero_image { |
| } | ||
|
|
||
| .features__title, .pricing__title, .team__title { | ||
| .title { |
| </header> | ||
|
|
||
| <div class="hero"> | ||
| <article class="hero"> |
| <section class="features" id="features"> | ||
| <h2 class="features__title">FEATURES</h2> | ||
| <div class="features__container"> | ||
| <h2 class="features__title title">FEATURES</h2> |
|
|
||
| <div class="pricing__title--container"> | ||
| <h2 class="pricing__title">CHOOSE YOUR PRICING PLAN</h2> | ||
| <header class="pricing__title--container"> |
index.html
Outdated
| <br> | ||
| <br> |
There was a problem hiding this comment.
Tutaj br powinno być usunięte - używamy tylko do rozdzielania tekstu, a nie do robienia odstępu.
| <li class="plan__description">Up to 7 Projects</li> | ||
| <li class="plan__description">2 Additional Developers</li> |
There was a problem hiding this comment.
Tylko te 2 elementy powinny być listą - reszta tj. nazwa pakietu czy cenna itp. już nie. To nie jest cześć listy.
2 participants
No description provided.