-
-
Notifications
You must be signed in to change notification settings - Fork 129
Правка отступов контента под некоторые экраны #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
Правка отступов контента под некоторые экраны #723
Conversation
Уменьшены отступы для нешироких desktop-ов, увеличен контейнер с контентом для широких
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В проекте используется сборщик и less ты не предлагаешь установить less как зависимость, но предлагаешь хранить в исходниках less файлы, это несерьезно
Я же написал, что возможно вы захотите. Просто я не уверенно чувствую себя в настройке сборщиков т.к. лишь изредка ими пользуюсь, поэтому не добавил. Пожалуйста, помогите добавить сборщик, вот мои настройки PhpStorm File Watcher: arguments (для lessc): output path to refresh: P.S. Если это проблема - заберите собранный .css - файл и не будем усложнять. Но на будущее компилятор подозреваю что все равно понадобится. |
т.е. в готовом css тоже проблемы, там 9 лишних css переменных, которые ничего не делают. Зачем они?
|
Я могу твои идею взять и самому влить, но так уже было и ты сказал, что я "зажал" и "сделал всё чтобы не принять". Поэтому давай ты PR поправишь, я проверю и волью, не проблема |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Жду нескольких изменений:
- Файлов less не должно быть, ровно также как .map
- Неиспользуемых переменных не должно быть
- Форматирование нужно вернуть как было
Готово. Глянь финальный diff: из форматирования остались только начальные нули от моего линтера (не было единой стилистики, либо везде их писать, либо везде нет (я оставил сокращенный синтаксис, как преобладающий), см. вот этот коммит и исправление опечаток (спасибо less-у) Исходники и упоминания less пока выпилил. |
@dimasites Когда что-то переделываешь и нужен новый ревью, то ты можешь запросить через функционал github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Проверял и до этого, и сейчас после синхронизации форка (там 2 коммита всего, без конфликтов) ещё раз проверил, записал видео с полным тестом, от запуска master-ветки и переключение на мою https://yadi.sk/d/jC1igotRCOYuPg - эффекта как у тебя на скрине - повторить не получилось
В theme такого компонента нет, как его на основе реализованных готовых создать и какой написать базовый код обертки, мне не очевидно, потому что (https://yadi.sk/i/h2k8d4iH4AhN1w), так что руки и правда «завязаны». Если настаиваешь на расширении, я не против, но нужна помощь в подготовке базовой обертки, в которой понятно будет, где именно добавить/исправить CSS-код. Как альтернативу предлагаю оставить текущую реализацию с триклятым !important, которая работает вроде бы корректно. |
@dimasites Ну как это не получилось, если прямо на твоём же видео видно
Хорошо, давай с css разберешься, а там посмотрим |
Действительно) Я не любитель усложнять стили для отлова промежуточных разрешений, но согласен что правильнее, когда такой эффект вообще не повторить. Добавил правило для фикса. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dimasites Справа на скрине видишь косяк? Пожалуйста, тестируй основательно прежде чем запрашивать request, спасибо!
Сорри, мой косяк, даже не первом скриншоте ты обвёл, а я не заметил( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…/Docs into feature/adaptive-styles
Для понимания, я проверяю: 375, 1024, 1280, 1366, 1440, 1600, 1920, 2560 ширину, а ради поддержания «идеальной резиновой верстки» нагромождать и делать неподдерживаемым (а без препроцессора это примерно так) код я в моём pull request не планировал. Вроде бы и не очень код разросся, ну и ок. Раз без этого никак, то внёс правки. Конечно, стало ещё лучше. Спасибо за рекомендацию! |
На всякий случай напомню, есть пользователи, которые могут просматривать сайт открыв браузер не на весь экран |
@dimasites PR можно протестировать? |
Да. Сорри, забываю нажимать кнопочку re-request... Тестировать можно! |
@dimasites Протестировал. Идея хорошая, но реализация к сожалению не очень. Т.к. я не понял как при расширении контентной части ты аж задел блок оглавлений. Всё вернул и в одну строчку расширил контентную часть страницы |
@GulomovCreative я делал не только это, как и писал в заголовке PR. В тексте сделал анонс только расширения контентной области, как основной правки среди других. Возможно стоило описать все, но я рассчитывал на то, что код не большой и понять что он делает можно прямо из diff-а. Сделал в общем я новый PR, в котором предлагаю совмещенное решение |
Уменьшены отступы для нешироких desktop-ов, увеличен контейнер с контентом для широких экранов.
Описание улучшений
В последнее время люди жаловались что не влезают таблицы документации в экран, появляется горизонтальный скролл и не удобно читать.
Я увеличил контейнер с контентом на больших экранах, и уменьшил отступы для экранов ноутбуков.
Проверял по этому URI:
/components/pdotools/snippets/pdoresources
— корректно без горизонтального скролла отображается теперь начиная с 1366px и шире. Мобильная версия не затронута изменениями.Заодно переписал стили в LESS. Возможно вы захотите добавить LESS-компилятор в проект. Для сборки использую File Watcher PHPstorm, поэтому внешний сборщик мне не понадобился. Поправил ещё .editorconfig под для поддержки отступов в less-файлах в общем стиле.
Актуальные проблемы