Skip to content

Add a margin at the bottom of every page #99

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 2 commits into from
May 6, 2018
Merged

Add a margin at the bottom of every page #99

merged 2 commits into from
May 6, 2018

Conversation

Butt4cak3
Copy link
Contributor

@Butt4cak3 Butt4cak3 commented May 1, 2018

When I brought up the AAA a few days ago, a friend of mine brought up that he doesn't like how the chapter text ends at the very bottom of the page. The Introduction chapter is a good example - the last line of text is at the very bottom of the monitor when viewing the page in fullscreen. I see where he's coming from, so I tried adding a margin at the bottom and I think it's nicer this way. What do you all think?

@Butt4cak3 Butt4cak3 added the Discussion This is open for a discussion. label May 1, 2018
@leios
Copy link
Member

leios commented May 2, 2018

I agree, I don't like how the chapters end so abruptly. I think this is a good change, but I'll leave it up for a bit in case there is discussion.

Copy link
Member

@june128 june128 left a comment

Choose a reason for hiding this comment

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

The changes produce the targeted result for the pages I tested.

@Butt4cak3
Copy link
Contributor Author

It's also about the amount of margin. Should I make it wider or narrower or is it just right?

@june128
Copy link
Member

june128 commented May 3, 2018

I think the margin should be a bit more narrow. Maybe 10em instead of 15em?

@Butt4cak3
Copy link
Contributor Author

I don't know. I made it 10em first but I thought it wasn't enough. Let's hear some more opinions.

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented May 4, 2018

Alright. Everyone seems to be either approving or indifferent. This is also not so important that everyone absolutely has to have a say in it. I say we define some kind of deadline, leave the thread open until then and then merge it, or else it will end up as one of these topics that never come to an end.

I opened this on Monday. Let's make it a week and merge it next Monday unless someone can provide a good reason not to.

@june128
Copy link
Member

june128 commented May 5, 2018

Maybe 12 or 13 em? I find 15 still a bit too much.

@june128
Copy link
Member

june128 commented May 5, 2018

Here's a direct comparison.

12em
screen shot 2018-05-05 at 21 03 01_page_margin_12em

15em
screen shot 2018-05-05 at 21 03 53_page_margin_15em

In the end it doesn't really matter, but 12em looks a bit better imo.

@Butt4cak3
Copy link
Contributor Author

@julianschacherpp I guess 12em is alright, too. I just thought maybe someone else would comment on it, but nobody seems to care :x

@june128
Copy link
Member

june128 commented May 5, 2018

Yeah, disappointing. :/

@leios
Copy link
Member

leios commented May 5, 2018

I care and agree on 12em

@leios
Copy link
Member

leios commented May 5, 2018

I think this is a case where everyone agrees we should have some space, but no one really knows how much space.

I changed the margin from 15em to 12em due to popular demand.
Copy link
Member

@june128 june128 left a comment

Choose a reason for hiding this comment

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

The stuff seems to work. Since no one - with significantly more CSS knowledge than @Butt4cak3 - cares, I approve this one.

@leios
Copy link
Member

leios commented May 5, 2018

Perfect! Merge time?

Copy link
Member

@june128 june128 left a comment

Choose a reason for hiding this comment

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

Selecting the right thing.

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented May 6, 2018

My suggestion was to wait until one week has passed to allow more opinions, but at this point I'm pretty sure it's not important enough. Go ahead and merge if you want.

Edit: Nevermind. I did it myself.

@june128
Copy link
Member

june128 commented May 6, 2018

I guess so. We could wait to Monday, but I don't expect significant input till then.

@Butt4cak3 Butt4cak3 merged commit 974a005 into master May 6, 2018
@Butt4cak3 Butt4cak3 deleted the page-margin branch May 6, 2018 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion This is open for a discussion. General
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants