task-html-and-css-basics done#176
Conversation
devmentor-pl
left a comment
devmentor-pl
left a comment
There was a problem hiding this comment.
Neil,
Projekt prezentuje się bardzo dobrze! 👍
Miałem parę uwag dot. semantyki (odpowiednie znaczniki w danym miejscu) oraz do BEM - zostawiłem link do artykułu, jeśli on Ci nie rozjaśni tematu to umów się proszę na spotkanie to wszystko wyjaśnię.
| <div class="section__container"> | ||
| <nav class="section__navbar navbar"> | ||
| <div class="navbar__logo"> | ||
| <object data="./images/logo.svg"></object> |
There was a problem hiding this comment.
Jeśli używasz tylko svg jako obrazek (nie ma tam dodatkowej zawartości, tylko sam obraz) to raczej preferowałbym <img/>
| </div> | ||
| </nav> | ||
| <div class="section__landingpage landingpage"> | ||
| <div class="landingpage__container--left"> |
There was a problem hiding this comment.
Tutaj użyłbym article bo jego zawartość tj. nagłówek oraz treść (nawet niewielka) o tym świadczą.
| <div class="section__landingpage landingpage"> | ||
| <div class="landingpage__container--left"> | ||
| <h1 class="landingpage__title">Beautiful Free Nova template</h1> | ||
| <h4 class="landingpage__text"> |
There was a problem hiding this comment.
Raczej użyłbym p - nawet nazwa klasy o tym świadczy.
| </div> | ||
| </section> | ||
| <section class="section section__features features"> | ||
| <div class="section__container features__container--titel"> |
There was a problem hiding this comment.
Myślę, że ten div nie jest tu potrzebny.
| <div class="section__container features__container--cards"> | ||
| <div class="features__card"> |
There was a problem hiding this comment.
Można zrobić tutaj ul > li bo mamy listę elementów - nawet nazwy klas o tym świadczą tj. --cards > __card
| <div class="theme__container--imac"> | ||
| <img class="theme__imac" src="./images/imac.png" alt="" /> | ||
| </div> | ||
| <div class="theme__container--text"> |
| </div> | ||
| </section> | ||
| <section class="section section__pricing pricing"> | ||
| <div class="section__container pricing__container--title"> |
There was a problem hiding this comment.
Można użyć też header - ten znacznik może być nagłówkiem dla strony, ale również dla sekcji czy artykułu.
| </div> | ||
| </section> | ||
| <section class="section section__team team"> | ||
| <div class="team__container--title"> |
There was a problem hiding this comment.
Tutaj też raczej bym zrezygnował z div - nie wiele on daje, wyśrodkowanie w h2 wystarczy do osiągnięcia efektu.
| <div class="team__card--title">CEO</div> | ||
| <div class="team__card--name">Roll Over Beethoven</div> | ||
| <div class="team__card--role-description"> |
There was a problem hiding this comment.
Podobnie jak wcześniej brak Elementu z BEM tj. team__card team__card--title
- team__card - ma ogólne cechy dla wszystkich elementów
- team__card--title - ma szczególne cechy dla wszystkich elementów (np. zaznaczenie, że element jest aktywny).
Dlatego tutaj to rozwiązanie nie pasuje.
| .landingpage__container--right { | ||
| margin-right: 54px; | ||
| } | ||
| .landingpage__container--right > div { |
There was a problem hiding this comment.
w BEM stosujemy jedynie klasy. Dla div powinieneś przypisać klasę i po niej stylować.
2 participants
No description provided.