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

[style] remove .md-content min-height override #136

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

the-overdriven
Copy link

Before (current):
image
What's the point of making short pages so long?

I believe it's an old override that doesn't take header and footer height into account anymore, so that the whole page min-height isn't 100vh (as expected), but rather becomes: 100vh + header height + footer height, which adds an unnecessary scroll.

After (this MR):
image

@kamilkrzyskow
Copy link
Collaborator

kamilkrzyskow commented Sep 29, 2024

iirc it was made to remove/avoid double scrollbars.
Please check on standard 1080p the Zengin section where the nav is a lot longer and see the website (body) having no scrollbar and yet the nav having one. UX is better with one body scrollbar imo, but I don't remember discussion about it on Discord, perhaps I just decided so on a whim 🤔

EDIT:
OK, like you said it's an older override from the time when the Zengin index would expand the whole nav tree and this would result in a very short body without a scrollbar and very narrow nav section with a long nav scrollbar.

Currently it doesn't auto-expand on the index, and the separate nav scrollbar is not so bad in a deeper page, since the user already (probably) navigated to the site they want, so no need to scroll over the whole nav to find something else.

@kamilkrzyskow
Copy link
Collaborator

Therefore after the edit above LGTM, but I'll ask for a review from Emu.

@kamilkrzyskow
Copy link
Collaborator

Auronen gave the green light on Discord, so I guess that's enough ✌️

@kamilkrzyskow kamilkrzyskow merged commit 94e7c82 into Gothic-Modding-Community:dev Sep 30, 2024
1 check passed
@kamilkrzyskow kamilkrzyskow removed the request for review from muczc1wek September 30, 2024 13:39
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