Skip to content

Правка отступов контента под некоторые экраны #723

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

Merged
merged 12 commits into from
Jun 24, 2023

Conversation

dimasites
Copy link
Contributor

Уменьшены отступы для нешироких desktop-ов, увеличен контейнер с контентом для широких экранов.

Описание улучшений

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

Я увеличил контейнер с контентом на больших экранах, и уменьшил отступы для экранов ноутбуков.

Проверял по этому URI: /components/pdotools/snippets/pdoresources — корректно без горизонтального скролла отображается теперь начиная с 1366px и шире. Мобильная версия не затронута изменениями.

Заодно переписал стили в LESS. Возможно вы захотите добавить LESS-компилятор в проект. Для сборки использую File Watcher PHPstorm, поэтому внешний сборщик мне не понадобился. Поправил ещё .editorconfig под для поддержки отступов в less-файлах в общем стиле.

Актуальные проблемы

Уменьшены отступы для нешироких desktop-ов,  увеличен контейнер с контентом для широких
Copy link
Member

@GulomovCreative GulomovCreative left a comment

Choose a reason for hiding this comment

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

В проекте используется сборщик и less ты не предлагаешь установить less как зависимость, но предлагаешь хранить в исходниках less файлы, это несерьезно

@dimasites
Copy link
Contributor Author

dimasites commented Jun 21, 2023

Возможно вы захотите добавить LESS-компилятор в проект.

Я же написал, что возможно вы захотите. Просто я не уверенно чувствую себя в настройке сборщиков т.к. лишь изредка ими пользуюсь, поэтому не добавил. Пожалуйста, помогите добавить сборщик, вот мои настройки PhpStorm File Watcher:

arguments (для lessc):
--source-map=$FileNameWithoutExtension$.css.map $FileName$ $FileNameWithoutExtension$.css

output path to refresh:
$FileNameWithoutExtension$.css

P.S. Если это проблема - заберите собранный .css - файл и не будем усложнять. Но на будущее компилятор подозреваю что все равно понадобится.

@GulomovCreative
Copy link
Member

P.S. Если это проблема - заберите собранный .css - файл и не будем усложнять. Но на будущее компилятор подозреваю что все равно понадобится.

  1. Хорошо, PR исправишь, чтобы я влил?
  2. Я в ревью пишу:

Даже если учесть добавление в стак разработки less, то что тут делают css переменные --vp-custom-breakpoint-*, судя по коду - ничего, они просто есть

т.е. в готовом css тоже проблемы, там 9 лишних css переменных, которые ничего не делают. Зачем они?

  1. Ты же компилятором форматирование всего файла изменил, с этим что мне сделать?

@GulomovCreative
Copy link
Member

Я могу твои идею взять и самому влить, но так уже было и ты сказал, что я "зажал" и "сделал всё чтобы не принять". Поэтому давай ты PR поправишь, я проверю и волью, не проблема

@GulomovCreative GulomovCreative self-requested a review June 21, 2023 16:25
Copy link
Member

@GulomovCreative GulomovCreative left a comment

Choose a reason for hiding this comment

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

Жду нескольких изменений:

  1. Файлов less не должно быть, ровно также как .map
  2. Неиспользуемых переменных не должно быть
  3. Форматирование нужно вернуть как было

@dimasites
Copy link
Contributor Author

dimasites commented Jun 21, 2023

Готово.

Глянь финальный diff: из форматирования остались только начальные нули от моего линтера (не было единой стилистики, либо везде их писать, либо везде нет (я оставил сокращенный синтаксис, как преобладающий), см. вот этот коммит и исправление опечаток (спасибо less-у)

Исходники и упоминания less пока выпилил.

@GulomovCreative GulomovCreative self-requested a review June 21, 2023 23:28
@GulomovCreative
Copy link
Member

@dimasites Когда что-то переделываешь и нужен новый ревью, то ты можешь запросить через функционал github

Copy link
Member

@GulomovCreative GulomovCreative left a comment

Choose a reason for hiding this comment

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

Ну, есть ряд проблем, глянь пожалуйста.

изображение

Мы с !important только усложним всё. Предлагаю переопределить компонент VPDoc.vue и уже там у тебя развяжутся руки

@dimasites
Copy link
Contributor Author

dimasites commented Jun 22, 2023

Ну, есть ряд проблем, глянь пожалуйста.
(скриншот)

Проверял и до этого, и сейчас после синхронизации форка (там 2 коммита всего, без конфликтов) ещё раз проверил, записал видео с полным тестом, от запуска master-ветки и переключение на мою https://yadi.sk/d/jC1igotRCOYuPg - эффекта как у тебя на скрине - повторить не получилось

Мы с !important только усложним всё. Предлагаю переопределить компонент VPDoc.vue и уже там у тебя развяжутся руки

В theme такого компонента нет, как его на основе реализованных готовых создать и какой написать базовый код обертки, мне не очевидно, потому что (https://yadi.sk/i/h2k8d4iH4AhN1w), так что руки и правда «завязаны».

Если настаиваешь на расширении, я не против, но нужна помощь в подготовке базовой обертки, в которой понятно будет, где именно добавить/исправить CSS-код.

Как альтернативу предлагаю оставить текущую реализацию с триклятым !important, которая работает вроде бы корректно.

@GulomovCreative
Copy link
Member

Проверял и до этого, и сейчас после синхронизации форка (там 2 коммита всего, без конфликтов) ещё раз проверил, записал видео с полным тестом, от запуска master-ветки и переключение на мою https://yadi.sk/d/jC1igotRCOYuPg - эффекта как у тебя на скрине - повторить не получилось

@dimasites Ну как это не получилось, если прямо на твоём же видео видно

изображение

Если настаиваешь на расширении, я не против, но нужна помощь в подготовке базовой обертки, в которой понятно будет, где именно добавить/исправить CSS-код.

Как альтернативу предлагаю оставить текущую реализацию с триклятым !important, которая работает вроде бы корректно.

Хорошо, давай с css разберешься, а там посмотрим

@dimasites
Copy link
Contributor Author

Ну как это не получилось, если прямо на твоём же видео видно

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

Copy link
Member

@GulomovCreative GulomovCreative left a comment

Choose a reason for hiding this comment

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

изображение

@dimasites Справа на скрине видишь косяк? Пожалуйста, тестируй основательно прежде чем запрашивать request, спасибо!

@dimasites
Copy link
Contributor Author

Справа на скрине видишь косяк? Пожалуйста, тестируй основательно прежде чем запрашивать request

Сорри, мой косяк, даже не первом скриншоте ты обвёл, а я не заметил(
Исправлено.

Copy link
Member

@GulomovCreative GulomovCreative left a comment

Choose a reason for hiding this comment

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

изображение

изображение

Еще раз прощу, протестировать сначала основательно, а потом запрашивать request. Спасибо!

@dimasites
Copy link
Contributor Author

Еще раз прощу, протестировать сначала основательно, а потом запрашивать request. Спасибо!

Для понимания, я проверяю: 375, 1024, 1280, 1366, 1440, 1600, 1920, 2560 ширину, а ради поддержания «идеальной резиновой верстки» нагромождать и делать неподдерживаемым (а без препроцессора это примерно так) код я в моём pull request не планировал. Вроде бы и не очень код разросся, ну и ок.

Раз без этого никак, то внёс правки. Конечно, стало ещё лучше. Спасибо за рекомендацию!

@GulomovCreative
Copy link
Member

Еще раз прощу, протестировать сначала основательно, а потом запрашивать request. Спасибо!

Для понимания, я проверяю: 375, 1024, 1280, 1366, 1440, 1600, 1920, 2560 ширину, а ради поддержания «идеальной резиновой верстки» нагромождать и делать неподдерживаемым (а без препроцессора это примерно так) код я в моём pull request не планировал. Вроде бы и не очень код разросся, ну и ок.

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

@GulomovCreative
Copy link
Member

@dimasites PR можно протестировать?

@dimasites
Copy link
Contributor Author

dimasites commented Jun 23, 2023

@dimasites PR можно протестировать?

Да. Сорри, забываю нажимать кнопочку re-request... Тестировать можно!

@GulomovCreative
Copy link
Member

@dimasites Протестировал. Идея хорошая, но реализация к сожалению не очень. Т.к. я не понял как при расширении контентной части ты аж задел блок оглавлений.

Всё вернул и в одну строчку расширил контентную часть страницы

@dimasites
Copy link
Contributor Author

dimasites commented Jun 24, 2023

Т.к. я не понял как при расширении контентной части ты аж задел блок оглавлений.

@GulomovCreative я делал не только это, как и писал в заголовке PR. В тексте сделал анонс только расширения контентной области, как основной правки среди других. Возможно стоило описать все, но я рассчитывал на то, что код не большой и понять что он делает можно прямо из diff-а. Сделал в общем я новый PR, в котором предлагаю совмещенное решение

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.

2 participants