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

[#153] add footer (internationalized) #161

Draft
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

RedGradient
Copy link

@RedGradient RedGradient marked this pull request as ready for review July 1, 2023 15:02
@ola-9
Copy link
Contributor

ola-9 commented Jul 3, 2023

@Malcom1986 посмотри, пжлста

@Malcom1986
Copy link
Collaborator

@RedGradient Привет! Здорово, такой подход будет работать, но можно пойти еще дальше. Давай попробуем сделать наследование шаблонов. То есть у нас будет общий костяк страницы - layout, который будет содержать общую для всех страниц разметку и такие фрагменты типа плейсхолдера. А шаблоны конкретных страниц будут дополнять layout (как бы наследоваться от него) и будут содержать только уникальную для них разметку. Когда мы рендерим конкретную страницу, будет браться контент конкретной страницы и вставляться на место плейсхолдера в layout. И получится такая комбинированная разметка. В чем плюс. Если нам нужно будет добавить еще како-то общий элемент - хедер, панель навигации и так далее, нам понадобится изменить только layout. Все остальные страницы останутся неизменными. При нынешнем подходе придется опять вставлять их при помощи th:replace на каждую страницу

@Malcom1986
Copy link
Collaborator

@fey
Copy link
Collaborator

fey commented Jul 4, 2023

Добрый день.
А что за пустое пространство между хедером и верхним краем страницы?
image

@@ -13,6 +13,18 @@
<script th:src="@{/webjars/bootstrap/js/bootstrap.bundle.min.js}" type="text/javascript"></script>
<!-- Language switcher-->
<script th:src="@{/fragments/lang-switcher.js}" type="text/javascript"></script>
<style>
body {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для чего используется этот стиль?
Мы не используем кастомные стили - обходимся тем, что дает нам Bootstrap
Плюс в таком виде добавляется пустое пространство над хедером.

Copy link
Collaborator

@fey fey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кастомизацию main, body нужно удалить. Не использовать секцию style внутри страницы-шаблона (мы стили пишем в отдельном файле css и загружаем его в шаблон), но здесь кастомные стили в принципе не нужны.
Используйте классы, которые предоставляют Bootstrap. Там кстати есть готовые примеры с хедерами и футерами.

@fey
Copy link
Collaborator

fey commented Jul 4, 2023

Еще обратите внимание, что работа ведется также в #155
Предлагаю вам скооперироваться, чтобы не делать одно и то же).

Salivya and others added 4 commits July 6, 2023 10:38
Добавлены требования к сервису hexlet correction
перефразированы требования, относящиеся к личному пространству пользователя. перефразированы требования из раздела implicit
Требования к проекту
@Malcom1986
Copy link
Collaborator

@RedGradient Привет! Есть успехи по этому ПРу?

@RedGradient
Copy link
Author

@Malcom1986 Привет 👋 Я подумал, что лучше оставлю эту задачу amirhraj-у, поскольку после некоторой паузы он снова подключился к работе.

@RedGradient
Copy link
Author

Возвращаюсь к работе над ПР. Последний отправленный коммит - промежуточный результат.

@RedGradient
Copy link
Author

@fey, добрый день! Уточните, пожалуйста, какое поведение футера предпочтительнее:

  1. По-умолчанию футер находится внизу экрана. Если контента много, футер опускается ниже за пределы экрана, следуя за контентом.
  2. Футер прикреплен к низу экрана и виден всегда. В случае, если контента много, он его будет перекрывать.

…/Issue179

Issue179 - Удаление пользователя из воркспейса
@fey
Copy link
Collaborator

fey commented Jul 20, 2023

Скорее первый вариант, чем второй, футер не должен перекрывать контент. Если контента мало, то на странице должен быть стиль что-то типа min height, когда футер все равно будет внизу.

Можете посмотреть, как ведет себя футер на хекслете,хекслет-цв, код бейзикс и так далее.

@fey
Copy link
Collaborator

fey commented Jul 20, 2023

т.е. футер должен быть частью страницу, а не оставаться в поле видимости при скроллинге

@RedGradient RedGradient requested a review from fey July 20, 2023 19:38
@fey
Copy link
Collaborator

fey commented Jul 21, 2023

прежде чем приступить к ревью, я предлагаю пересобрать ветку, чтобы в истории не было лишнших коммитов. Сейчас там коммиты идут из основной ветки.
Можете создать новую ветку от текущей и в ней потренироваться.

Тут можно почитать, как это можно сделать - https://ru.hexlet.io/qna/git/questions/kak-aktualizirovat-vetku-v-pull-request

.main-content {
flex: 1;
padding-top: 2rem;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

посмотрите стили, которые используютсяв самом бутстрапе + можно поспрашивать в сообществе Хекслета, как можно эту задачу решить.

Мы стараемся не использовать кастомные стили. Если они все таки нужны, то используем префикс x-. Но все таки лучше не использовать кастом.
Плюс

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Подскажите, пожалуйста, куда конкретно можно обратиться по этому вопросу?

@fey
Copy link
Collaborator

fey commented Jul 21, 2023

волонтеры или фронтенд https://t.me/hexletcommunity/12
https://t.me/hexletcommunity/19

@RedGradient
Copy link
Author

RedGradient commented Jul 25, 2023

Взял паузу в работе.

@fey fey marked this pull request as draft July 27, 2023 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants