Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Личный проект: завершаем разметку #5

Merged
merged 39 commits into from Feb 7, 2020

Conversation

Vogrim21 added 3 commits Feb 3, 2020
Добавил три слайда
Добавление 14 картинок
Добавляю разметку всей страницы
@keksobot keksobot changed the title Бонусные задания Личный проект: дополняем разметку Feb 4, 2020
keksobot added a commit that referenced this pull request Feb 4, 2020
@keksobot

This comment has been minimized.

Copy link
Contributor

keksobot commented Feb 4, 2020

♻️ Я собрал ваш пулреквест. Посмотреть можно здесь.

@keksobot keksobot changed the title Личный проект: дополняем разметку Личный проект: продолжаем подготовку графики Feb 4, 2020
@keksobot

This comment has been minimized.

Copy link
Contributor

keksobot commented Feb 4, 2020

♻️ Я собрал ваш пулреквест. Посмотреть можно здесь.

keksobot added a commit that referenced this pull request Feb 4, 2020
@keksobot keksobot changed the title Личный проект: продолжаем подготовку графики Личный проект: завершаем разметку Feb 4, 2020
@keksobot

This comment has been minimized.

Copy link
Contributor

keksobot commented Feb 4, 2020

♻️ Я собрал ваш пулреквест. Посмотреть можно здесь.

keksobot added a commit that referenced this pull request Feb 4, 2020
Copy link
Collaborator

efiand left a comment

К перечисленным замечаниям добавлю насчет графики:
слайды сейчас не годятся, они на белом фоне. Либо танцы с бубном по экспорту на сером, либо тут вполне допустим png, к тому же количество цветов и деталей не так велико.

Карта: тут рекомендательно - jpg, так как не так уж и мало деталей и к тому же в большинстве случаев будет заменена на интерактивную, смысла грузить много байт нет.

Иконки: надо экспортировать как svg, раз такая возможность есть у нас (инфа 100% про макет Нердса). png в 2020 простительно только для проектов старых, когда дизайнера уж и не найти. В новых, если дизайнер продолжает генерить иконки в ФШ растром, не работать больше с таким чудаком. Если есть проблемы экспорта в svg, дай знать.

catalog.html Outdated Show resolved Hide resolved
catalog.html Outdated Show resolved Hide resolved
catalog.html Outdated Show resolved Hide resolved
catalog.html Outdated Show resolved Hide resolved
<ul class="sorting">
<li class="sorting-item">По цене</li>
<li class="sorting-item">По типу</li>
<li class="sorting-item">По названию</li>

This comment has been minimized.

Copy link
@efiand

efiand Feb 4, 2020

Collaborator

По типу можно сортировать как по возрастанию, так и по убыванию, и по функционалу, и по цене тоже. То есть у нас тут 2 списка (второй с визуально скрытыми текстами, на макете - просто треугольнички, очень легко делаемые на css)

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Vogrim21 added 16 commits Feb 5, 2020
delete
delete
delete
delete
delete
delete
delete
delete
delete
deletev
delete
delete
delete
delete
delete
delete
delete
delete
delete
delete
delete
delete
Иконки
Логотипы
Vogrim21 added 10 commits Feb 5, 2020
карта
логотипы
Устранено
delete
delete
delete
delete
Иконки
@efiand

This comment has been minimized.

Copy link
Collaborator

efiand commented Feb 6, 2020

Новые замечания:

  • <h1> не надо класть в <nav>, извини, пропустил этот момент ранее.
  • Есть два пути: 1) понял и сделал, 2) не понял и спросил, остальное фу. А ты заигнорил замечание о втором списке сортировки (
  • То же самое о слайдах: пересохрани под другими именами в png-8, а то их нет сейчас
  • Проигнорено замечание о критерии Д 29. Как это у тебя получается, я удивляюсь, учитывая то, что читаем мы последовательно справа налево и сверху вниз.
  • Неединообразное наименование картинок: не только дефис, но и нижнее подчеркивание.

Новые рекомендации:

  • Торопливость приводит не к закрытию дедлайна, а к его провалу:
    изображение
  • Надо исправлять баги в пределах всего проекта.
    изображение
    изображение
    (-subtitle же)
  • убрать _1 из имени иконки, нафиг оно ей вообще?
  • 2.jpg - лучше map, так понятнее.
  • в идеале у картинок тоже есть неймспейсы: лого начинается с logo-, товары с напр. offer-, иконки с icon- и т.п. Они и понятны сразу, и лежать будут в папке рядом.
  • минифицировать svgo.
  • положить секцию контактов в футер, она же на всех страницах, а форма обратной связи идеологически относится к этой секции тоже.
Vogrim21 added 6 commits Feb 7, 2020
Минифицировал изображение и svg прогнал через svgo
переименовал
Готово
Исправление расстановки классов
@efiand

This comment has been minimized.

Copy link
Collaborator

efiand commented Feb 7, 2020

Старые критические замечания:

  • изображение
    По типу можно сортировать как по возрастанию, так и по убыванию, и по функционалу, и по цене тоже. То есть у нас тут 2 списка (второй с визуально скрытыми текстами, на макете - просто треугольнички, очень легко делаемые на css)
  • изображение
    изображение
    Там все очевидно написано и даже пример показан, что не ссылка на форму, а телефон делается ссылкой с протоколом tel.

Новые критические замечания:

  • Почему заголовок перед <nav>, он же по макету после?
Vogrim21 added 3 commits Feb 7, 2020
Готово
готово
исправление
@efiand

This comment has been minimized.

Copy link
Collaborator

efiand commented Feb 7, 2020

Старое замечание

  • Заголовок после навигации

Новое замечание

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

Рекомендация

  • Как я и писал в диалоге, логично дать классы не <li>, а ссылкам. По крайней мере странно, что у тебя в одной сортировке и пагинации не так, а в другой сортировке так.
готово
@@ -23,6 +22,7 @@ <h1 class="promo">Магазин готовых шаблонов</h1>
</nav>
</header>
<main>
<h1 class="promo">Магазин готовых шаблонов</h1>

This comment has been minimized.

Copy link
@efiand

efiand Feb 7, 2020

Collaborator

Можно и в main, тут нет единственно верного решения, но к чему крайности? Речь шла всего лишь о том, что после навигации, а не до нее.

@efiand efiand merged commit a3aa502 into htmlacademy-htmlcss:master Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.