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

Fix: correct the width of main and footer #130

Merged
merged 1 commit into from Dec 30, 2019
Merged

Conversation

@auge8472
Copy link
Contributor

auge8472 commented Dec 29, 2019

The widths of the elements main and footer was broken (narrower than header and nav) because the width was only 776px instead the 800px of header and nav and in contrast to header and nav main and footer have a padding for their content which is part of the width calculation.

So the main and footer boxes had (as first issue) not the same width as header and nav and had (as second issue) a resulting width of 814pxwhen the rule for the box width said width: 800px; because the 7px for padding-left and padding-right got added to the 800pxof the width-rule.

The correction was done with changing the box model to border-box (which includes paddings and border-widths in the width calculation for the element). Additionally the width rule for the direct childs of body was moved to a descendent selector for all these elements.

Attention The descendent selector would select also every direct child of body that will possibly introduced in the future.

It's done with changing the box model to border-box and a descendent selector for all direct childs of body (header, nav, main and footer).
@TrueBrain
Copy link
Member

TrueBrain commented Dec 30, 2019

I absolutely love your explanation. Makes it perfectly clear to me why you are doing this; thank you for taking the time to write it out :)

@TrueBrain TrueBrain merged commit 95de15a into OpenTTD:master Dec 30, 2019
2 checks passed
2 checks passed
Docker build
Details
website Build #20191229.7 succeeded
Details
@LordAro
Copy link
Member

LordAro commented Dec 30, 2019

I'm not sure border-box is the correct solution - it feels like a bit of a "cheat", and seems to have other effects besides the intended - section headings move, as does the OTTD banner

EDIT: I have been reliably informed that I am wrong. Still, some extra padding in a few places looks to be required now...

@TrueBrain
Copy link
Member

TrueBrain commented Dec 30, 2019

Some minor issues, most likely because of this PR (which I was only going to spot after merging this, so that is fine by me :D).

image
Missing bottom padding now

image
text is closer together

image
Too few spacing between icon and text.

@auge8472
Copy link
Contributor Author

auge8472 commented Dec 30, 2019

I'll correct the unwanted changes.

@LordAro
Copy link
Member

LordAro commented Dec 30, 2019

You can leave the banner links - they're corrected in #132

@auge8472 auge8472 deleted the auge8472:box-model branch Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.