Ответы пользователя по тегу Code review
  • Дайте оценку верстке?

    @Flying
    Визуально выглядит вполне пристойно и очевидных косяков почти нет, но если копнуть глубже - возникают вопросы.

    Есть целый ряд претензий по использованию графики. Часть их них, безусловно относится к косякам дизайнера, но и вы отработали не самым лучшим образом.

    Из наиболее заметного - заглавная картинка с автомобилем в PNG которая занимает почти 600кб и из-за этого грузится весьма и весьма неспешно (и заметно для пользователя). В целом это по большей части косяк дизайнера, не приложившего усилий к тому чтобы выбрать правильную графику (автомобиль снят явно на улице и отражения в стёклах дают существенный вклад в визуальный шум и, как следствие, в размер картинки, нужно было выбирать фотографию сделанную в специальном помещении). Кроме того дизайнер, очевидно, не слышал про требования к такси в Нью-Йорке и рисовал как взбредёт в голову, но оставим это на его совести. Сочетание фоновой картинки, на которой весь траффик едет в обратном направлении и делает автомобиль такси нарушающим правила дорожного движения - тоже на совести дизайнера.

    Однако и в этом случае и, тем более, в случае фоновых изображений ниже по странице вы допускаете ошибки с выбором форматов файлов, способами их вставки в страницу и оптимизацией. К примеру из картинки с автомобилем можно выжать почти 100кб просто за счёт использования оптимизаторов. Гораздо грустнее ситуация с фоновыми картинками ниже по тексту. Во-первых вы сохраняете фотографии в PNG, получая на выходе файлы по мегабайту, хотя они же в JPEG занимали бы в 5-10 раз быстрее. Во-вторых вы, скорее всего, сохранили фоновые картинки уже обработанными (затемнёнными). Я не видел макета, но предположу что там эти картинки стоят в их оригинальном виде и на них наложены какие-нибудь фильтры. На первый взгляд кажется что проблемы нет, но на практике (в случае вёрстки для реального сайта) вы вынуждаете человека который будет поддерживать сайт либо готовить картинки с такой же пост-обработкой либо мириться с тем что стиль сайта меняется. Правильное решение здесь - грузить картинки как они есть и реализовывать фильтры на CSS, тем более что здесь это делается элементарно через multi background или псевдо-класс с полупрозрачным фоном. Очевидно также что для таких тёмных картинок вполне можно использовать JPEG с меньшим качеством и тем самым существенно сэкономить пользователям трафик.

    Ещё одна проблема связанная с фоновыми картинками - вы не подкладываете под них близкий по цвету solid color. Попробуйте включить в dev.tools "Network throttling", отключить кэш и перегрузить свою страницу - думаю вы поймёте что я имею в виду - белые блоки с белым текстом стоят довольно продолжительное время, постепенно заполняясь довольно тёмными картинками. Если бы background-color под ними был бы чёрным - проблемы бы не было.

    Далее - логотип. Обычно логотипы разрабатываются отдельно и даже если он выглядит просто набранным шрифтом - это вовсе не значит что это не так. Логотип Google, Microsoft или Яндекс - тоже просто название, но, надеюсь вы не сверстаете их, написав надпись текстом? В общем логотип = картинка, лучше в векторе. Сейчас даже одно съезжание слогана на пиксель влево относительно названия уже рушит всю конструкцию логотипа.

    Обратите внимание на то как вы работаете с формами. Все поля в форме являются <input type="text">, хотя часть названий явно намекает на date / time селекторы, а "Choose Vehicle" - на список выбора.

    Хотелось бы отметить работу с иконками - их всё-таки лучше хранить в SVG и либо требовать с дизайнера либо подбирать на том же Icon Finder. При этом оформление (те пресловутые жёлтые кружки) лучше делать через CSS т.к. это позволяет вам существенно гибче работать с размерами элементов.

    Есть всякие недочёты касающиеся responsive, к примеру, внимание как отображается блок "Our Tariffs" в размере чуть более 600px, в частности название тарифа и описание.

    Пожалуйста обратите внимание на то что вы используете два разных меню для desktop и mobile представления. В целом в вашем случае меню довольно простое и можно было бы обойтись одним. Конечно две копии используют часто, но у этого решения есть свои недостатки (в частности отсутствие синхронизации состояния), так что вы должны осознанно принимать решение по таким вопросам. Кроме того inline обработчики onclick там явно могут быть заменены на элементарный
    document.querySelectorAll('.menu a, .menu-hover a').addEventListener()
    что явно сделает код более простым и поддерживаемым.

    Ещё один важный момент который зачастую опускают при вёрстке - поведение макета с реальными данными. То что дизайнер в макете понапихал везде lorem ipsum и тексты примерно одинакового размера - отнюдь не означает что на реальном сайте эти условия будут соблюдаться. Отсутствие навыка проверять поведение макета в изменяющихся условиях ведёт к множеству ошибок которые не видны в условиях синтетических данных. К примеру попробуйте в блоке "We Do Best Than You Wish" (претензии по поводу английского языка оставим в стороне) в любом из элементов банально увеличить количество текста в 2-3 раза. В Chrome это приводит только к излому сетки, в Firefox - ещё и к изменению размера иконки. При этом я предполагаю что Firefox ведёт себя правильно т.к. пропорции элементов изменились, а ограничения размеров на картинки у вас не заданы.

    В целом похоже что макет верстался и проверялся только в Chrome. К примеру посмотрите как ведёт себя картинка с рукой и телефоном в Firefox при изменении размеров. Опять же Firefox вполне корректен т.к. вы не обрезали картинку корректно, предпочитая выгрузить "как есть" и подгонять положение в CSS, но забыв при этом про overflow: hidden для контейнера.

    Теперь перейдём к CSS:

    Обратите внимание на то как вы подключаете внешний шрифт:
    family=Lato:400,700,700i,900,900i&amp;subset=latin-ext
    . Возникают два вопроса:
    1. Зачем вам subset=latin-ext на сайте где есть только базовая латиница?
    2. Как вы выбирали начертания? У вас подключаются 5 начертаний (400, 700, 900 + два italic'а), при этом grep по CSS даёт значения font-weight 200, 300, 400, 500, 600, 800 и ни одного italic. Вам не кажется что эти множества почти не пересекаются?


    Кроме того вы постоянно забываете про fallback шрифты что на медленном интернете и при отсутствии инструкций для font loading приводит к невидимому контенту страницы на период загрузки.

    Отсутсвие ограничения по ширине для .wrapper приведёт к недопустимо широкому сайту на больших мониторах с высоким разрешением. Можете уменьшить масштаб страницы до 50% и полюбоваться результатом.

    В стилях повсеместно используются достаточно общие названия классов в global namespace. К примеру кто бы мог подумать что стилизует селектор .text? Вы уверены что нигде больше на сайте подобный селектор не встретится? Даже при дальнейшем развитии сайта? Другими словами именование селекторов - важная часть работы, вы можете использовать любую методологию (тот же БЭМ или что-то ещё) или разработать свою, но ваш код не должен ломаться при добавлении ещё пары блоков, особенно если это будет делать другой человек.

    Списки элементов, к примеру тот же .product-cont лучше делать именно списками, а не распихивать элементы по столбцам вручную, благо flexbox и column layout здесь всё прекрасно сделают за вас, зато имея одноранговый список вы обеспечите себе куда большую свободу действий.

    Использование id в качестве CSS селектора - плохая практика, но у вас таких мест немало, 11 штук.

    Уверен что мог бы найти ещё что-то, но надеюсь для затравки хватит, и так много получилось... :)
    Ответ написан
    4 комментария
  • Хорошая ли верстка моего сайта?

    @Flying
    Сразу отмечу что смотрел только главную страницу.

    HTML код в целом выглядит вполне чисто, PHPStorm ругается только на ul внутри menu что действительно не разрешено.

    Несколько смущает принудительная анимация на все элементы. Не думаю что браузеры, особенно мобильные, будут рады таким стилям. Кроме того она приводит как минимум к весьма странным прыжкам меню при переходе через 1200px.

    Меню для десктопа не выравнивается по вертикали т.к. для блоков меню и телефонов в заголовке используется "выравнивание" через margin-top, а не нормальное выравнивание через flexbox с align-items: center.

    Для блоков в разделе "Наши преимущества" не хватает как минимум горизонтальных padding'ов, из-за этого текст в блоках прилипает к границам контейнеров. В этом же блоке у основного заголовка и заголовков блоков потерян letter-spacing (не мерял, на глаз видно). При ширине меньше 980px блоки выезжают за экран, видимо не доделано.

    Блок "Переезд в Болгарию" полностью сломан на ширине 980..1200px, а ещё уже - часть элементов теряет центровку. У блока .move__how та же проблема с горизонтальным отступом что и в блоке выше.

    Блок про недвижимость - та же проблема с центровкой (кнопка), letter-spacing и горизонтальным padding'ом.

    Кстати, посмотрел подробнее - многие проблемы возникают из-за потерянных закрывающихся скобок для media запросов здесь и здесь.

    Блок отзывов: иконки навигации влево / вправо не соответствуют макету, очевидно взяты из font-awesome, хотя в макете они другие. То же, кстати, касается и иконки "fa-angle-down", она в макете тоже не из font awesome. Вертикальное положение иконок навигации не соответствует макету (не должен учитываться блок пагинатора). Отступы внутри блока и letter-spacing - похоже общая больная тема. Из-за того что 3 элемента заголовка отзыва не объединены в общий контейнер - есть проблема с выравниванием и без контейнера она лечится не совсем тривиально. Стрелка в "Москва -> София" не соответствует макету.

    Блок формы: состояние :focus не реализовано (в макете это E-mail), в размерах ниже 768px элементы формы прилипают к краям экрана без какого-либо отступа. Нижний затемняющий градиент блока утерян. Иконка со стрелкой вниз не соответствует макету (здесь и в других местах). Стиль кнопки отличается от макета (бордюр и стиль шрифта)

    Отдельное большое фи - графика и общее отношение к размерам ресурсов:
    • 4.5 мегабайта картинок в PNG, вы это серьёзно? Банальная оптимизация через optipng / pngout даёт почти 800кб. Две фоновые картинки занимают суммарно 3.5 мегабайта, хотя они же в JPEG при качестве в 85% и оптимизацией через guetzly дают 225кб, а можно ужать ещё больше. city.png - почти полностью прозрачная картинка, используемая исключительно на белом фоне - зачем она в PNG с альфа-каналом? JPEG + guetzly даёт 43кб против 376кб.
    • 4 шрифта, при этом размерами 100 и 900, хотя на странице есть ещё как минимум 400, по 700 глифов! Зачем они там если у вас на сайте русский и латиница? Subsetting в Font Squirrel уже отменили? При этом два шрифта italic хотя реально italic используется на данный момент только в тексте комментария (толщиной 100), зачем?
    • Во всех макетах используется суммарно 4 разных шрифтовых иконки (3 стрелки и "спасательный круг"), при этом стрелки, как написано выше, не соответствуют макету. И ради этого грузятся все 1145 (!) иконок font awesome суммарным объёмом (в лучшем случае) почти 100кб. Конечно интернет сегодня быстрый, но зачем вот так сразу-то?
    • На все страницы грузится весь Bootstrap 3.3.7 целиком (почти 120кб CSS) ради Bootstrap grid? Она там в лучшем случае 3-5кб занимает, а остальное не используется? При этом загрузка CSS - блокирующая операция т.е. рендер стоит пока грузится CSS.
    Ответ написан
    3 комментария