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

grid fallback over flexbox #8

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

grid fallback over flexbox #8

wants to merge 4 commits into from

Conversation

otus-learning
Copy link
Owner

No description provided.

Copy link

@Ambiens Ambiens left a comment

Choose a reason for hiding this comment

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

Сам fallback работает. Но с точки зрения отладки было бы лучше не разносить это по всему файлу, а убрать под один @support. Иначе отладка и проверка, становится затруднительной. Надо бегать автозаменой и искать, что отключить, что не отключить. Это, кстати, касается и конструкции @media.
Хотя тут может быть вопрос, какую парадигму выбирать. Но конкретно в данном случая, я не вижу смысла эти конструкции размазывать по разным местам кода. А если ещё и несколько файлов будет, то вообще беда.

css/style.css Outdated
{
.all-links
{
left: 2rem;
Copy link

Choose a reason for hiding this comment

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

Не понятно зачем тут конструкция not? Можно же без неё. У нас fallback, это как бы то, что работает по умолчанию. А вдруг у нас сама конструкция @support не поддерживается?

Copy link

@Ambiens Ambiens left a comment

Choose a reason for hiding this comment

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

Пример что я имел ввиду под позиционированием контента section на примере Designers.
Также позиционирование заголовка. Его поворот и fallback.

@@ -244,18 +220,6 @@ h2.designers
transform-origin: bottom;
}
Copy link

Choose a reason for hiding this comment

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

div.designers { position: absolute; top: 46%; right: 0; left: 0; transform: translateY(-50%); } h2.designers { position: absolute; left: 70%; top: 46%; margin-top: -300px; transform: rotate(-0.25turn) translateX(-100%); transform-origin: top left; }

Если уж центрировать контент section, то так. И поворот заголовка и позиционирование его таким образом. И falback ниже.

top: 12%;
transform: rotate(0);
writing-mode: vertical-rl;
}
Copy link

Choose a reason for hiding this comment

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

h2.designers { transform: rotate(-180deg); transform-origin: center; writing-mode: vertical-rl; }

css/style.css Outdated

h2.designers
{
top: 10%;
Copy link

Choose a reason for hiding this comment

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

Не нужно

css/style.css Outdated
{
h2.designers
{
top: 5%;
Copy link

Choose a reason for hiding this comment

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

Не нужно

css/style.css Outdated

h2.designers
{
top: 10%;
Copy link

Choose a reason for hiding this comment

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

Не нужно

css/style.css Outdated
{
h2.designers
{
top: 5%;
Copy link

Choose a reason for hiding this comment

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

Не нужно

Copy link

@Ambiens Ambiens left a comment

Choose a reason for hiding this comment

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

Пару фиксов

css/style.css Outdated
@@ -55,6 +55,7 @@ p.menu
section
{
position: relative;
width: 100vw;
Copy link

Choose a reason for hiding this comment

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

Это зачем вдруг взялось? Убираем, не нужно.

css/style.css Outdated
left: 2%;
bottom: -65%;
}
}

.stgh-h3
@media(min-width: 1450px)
Copy link

Choose a reason for hiding this comment

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

Убираем с содержимым

Copy link

@Ambiens Ambiens left a comment

Choose a reason for hiding this comment

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

Вижу, что поправили.

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