Fomenko/#3 #21
Fomenko/#3 #21
Conversation
evgenybalagurov
left a comment
evgenybalagurov
left a comment
There was a problem hiding this comment.
Привет, Николай. Проделана огромная работа. Понравилось построение сетки grid через auto-fit. Почти не нужно будет делать адаптив. Все, что необходимо поправить в комментариях.
| <section class="photos"> | ||
|
|
||
| <section class="gallery"> | ||
| <div class="gallery__block"> |
There was a problem hiding this comment.
Предлагаю вынести текст в отдельный блок - text-block и создавать для него элементы и модификаторы в зависимости от ситуации. В своей секции feedback, я использовал элемент text-block__title для заголовка. Вместо элемента gallery__block тоже можно использовать text-block__title. И создать модификатор, например .text-block_placed_title, для задания ширины и правого padding.
| <div class="gallery__block"> | ||
| <h2 class="gallery__block_title">Заголовок блока с фотографиями</h2> | ||
| </div> | ||
| <div class="gallery__list"> |
There was a problem hiding this comment.
gallery__list необходимо сделать списком ul, так будет семантичнее)
| </div> | ||
| <div class="gallery__list"> | ||
| <img src="./images/img-gallery-1.jpg" alt="Подготовка к пьесе #4" class="gallery__list_element"> | ||
| <img src="./images/img-gallery-2.jpg" alt="Подготовка к пьесе #1" class="gallery__list_element"> |
There was a problem hiding this comment.
По БЭМ модификатор нельзя использовать отдельно от элемента на одном узле. Возможно имелся ввиду элемент gallery__list-element. Т.к. это элемент списка его лучше назвать gallery__item
There was a problem hiding this comment.
Не до конца понимаю, что ты имеешь ввиду, если честно. Блок gallery с модификатором list я не могу использовать?
There was a problem hiding this comment.
__list - это элемент у тебя,
_element -модификатор,
но по БЭМ одиночные модификаторы можно использовать только если они принимают бинарные значения (true, false), например _active или _visible.
Как написал Евгений тебе либо надо исправить на gallery__list-element, либо лучше на gallery__item.
There was a problem hiding this comment.
gallery__item тут будет самым оптимальным решением
| margin: 0 auto; | ||
| max-width: 100%; | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fit, minmax(206px, 359px)); |
There was a problem hiding this comment.
Здорово, что использовал auto-fit, адаптив почти не нужно будет делать). Можно немного его изменить, тогда на мой взгляд будет эффектнее выглядеть. Вот так: grid-template-columns: repeat(auto-fit, minmax(290px, 1fr)) и для элемента с классом .gallery__list_element width: 100%. И margin: 0 auto; max-width: 100%; будут не нужны. Можешь оставить, как сделал, так тоже классно.
There was a problem hiding this comment.
grid-template-columns: repeat(auto-fit, minmax(290px, 1fr)) - поддерживаю так будет круто)
классом .gallery__list_element - изменится скорее всего, смотри комменты выше.
| @@ -0,0 +1,3 @@ | |||
| .gallery__list_element { | |||
|
|
|||
There was a problem hiding this comment.
Файл для селекторов, которые не содержат стилей создавать не нужно.
Для изображения лучше задать display: block, что-бы сделать его блочным
DmitriyMGN
left a comment
DmitriyMGN
left a comment
There was a problem hiding this comment.
Молодец! Практически нечего было писать в замечания, есть мелкие рекомендации по альтам. Так держать!
| </div> | ||
| <div class="gallery__list"> | ||
| <img src="./images/img-gallery-1.jpg" alt="Подготовка к пьесе #4" class="gallery__list_element"> | ||
| <img src="./images/img-gallery-2.jpg" alt="Подготовка к пьесе #1" class="gallery__list_element"> |
There was a problem hiding this comment.
gallery__item тут будет самым оптимальным решением
| <img src="./images/img-gallery-5.jpg" alt="Подготовка к пьесе #5" class="gallery__list_element"> | ||
| <img src="./images/img-gallery-6.jpg" alt="Подготовка к пьесе #6" class="gallery__list_element"> | ||
| <img src="./images/img-gallery-7.jpg" alt="Подготовка к пьесе #7" class="gallery__list_element"> | ||
| <img src="./images/img-gallery-8.jpg" alt="Подготовка к пьесе #8" class="gallery__list_element"> |
There was a problem hiding this comment.
Посмотри пожалуйста альты к изображением, я думаю они не отражают того что происходит на картинке. По методологии альт должен отражать текстовое описание картинки в случае если она не загрузится, а так не очень понятно что все таки на ней.
| </p> | ||
| </div> | ||
| <div class="footer__design"> | ||
| <img src="./images/shishki.svg" alt="" class="footer__copyrights-logo"> |
Выполнена верстка секции "gallery"(переименовал её из "photos")