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

Add some whitespace to the CSS #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joeytwiddle
Copy link

Greetings. We felt the lines on the homepage were a little close together, so we expanded the line-height.

After that we got excited and added some other margins between elements. We hope you like them!

We only had time to look at the front page today, but hopefully that these changes will also work well on the other pages.

Greetings! We felt the lines on the homepage were a little close together, so we expanded the line-height.

After that we got excited and added some other margins between elements. We hope you like them!

We only had time to look at the front page today, but hopefully that these changes will also work well on the other pages.
Copy link

@JoshuaKimsey JoshuaKimsey left a comment

Choose a reason for hiding this comment

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

This does look better overall in my opinion. However, I do have a few suggestions. Firstly, maybe just shrink the line spacing back just a bit... Somewhere between the original amount and what you did here. I say this because on my 27" iMac, the spacing is rather excessive for the screen size. Also, on the section describing what the 8 Values are, maybe shrink that line spacing down just a bit more than the rest of the page to offset it and make it standout as its own section. Maybe you could move that sections side margins in just a bit more to make it stand out more with the wider line spacing?

Great work though! Definitely like what I'm seeing developed here.

@Genora51 Genora51 added the tweak label Oct 1, 2019
@joeytwiddle
Copy link
Author

I have adjusted the line-heights as suggested, making these tweaks a lot milder.

I have also indented the list of values an extra 12pt on each side.

(Whilst doing that I found a very weird bug with font-sizes in Chrome mobile, caused by the big blue button, but I will leave that for another time.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants