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

Increase margins between text in govspeak component #733

Merged
merged 4 commits into from Feb 16, 2016

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Feb 15, 2016

  • Use 20px margins (15px at thinner viewports) to match GOV.UK elements. This improves the legibility of content that contains many medium to long paragraphs, it comes at the cost of legibility for content with groups of one line paragraphs, mainly specialist content.
  • Add spacing between list items, 5px – matching GOV.UK elements
  • Use bold mixins
  • Clean up call to action styles

cc @dsingleton @gemmaleigh @alextea

Before and after examples

(The more spaced out examples in the GIFs are the new margins)

Travel advice

  • Better legibility on content with long paragraphs

spacing-travel-advice 1

Mobile

  • Clearer spacing at thinner viewports

spacing-mobile

Specialist content

  • Better positioning of call to action when it's the first piece of content
  • Better padding on call to action

spacing-specialist-content

fofr added 4 commits Feb 9, 2016
* Put line breaks between selectors
* Put each comma separated selector on a new line
Use 20px margins (15px at thinner viewports) to match GOV.UK elements.
This improves the legibility of content that contains many medium to
long paragraphs, it comes at the cost of legibility for content with
a group of one line paragraphs, mainly specialist content.

Add spacing between list items, as per older front-end typography.
* Simplify background rule, there’s no background image for either call
to action or information block (information block uses a pseudo element)
* Move the right padding needed for information-block into its own
declaration
* Increase spacing around the block to better match text spacing
* Remove margins from last child only, and apply to lists too - a list
in a call to action doesn’t sit correctly in the box
@fofr fofr changed the title [DO NOT MERGE] Increase margins betwen text in govspeak component [DO NOT MERGE] Increase margins between text in govspeak component Feb 15, 2016
@fofr fofr changed the title [DO NOT MERGE] Increase margins between text in govspeak component Increase margins between text in govspeak component Feb 16, 2016
@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Feb 16, 2016

Huge improvement 👍

It makes body copy much more readable, and moves us closer to consistency between Whitehall/Mainstream styling.

@alextea
Copy link
Contributor

@alextea alextea commented Feb 16, 2016

This looks great. Especially the list items.

@fofr
Copy link
Contributor Author

@fofr fofr commented Feb 16, 2016

Before

screen shot 2016-02-16 at 11 54 35

After

screen shot 2016-02-16 at 11 53 40

@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Feb 16, 2016

I'd be happy with this getting merged, but should we leave this open a bit longer to get more reckons?

@gemmaleigh
Copy link
Contributor

@gemmaleigh gemmaleigh commented Feb 16, 2016

👍 Nice work @fofr!

@fofr
Copy link
Contributor Author

@fofr fofr commented Feb 16, 2016

@markhurrell pointed out that Whitehall doesn't add margins to list items. As frontend and elements both do this we're keeping the 5px margin for now – mostly to match elements. When things are consistent we can re-asses and address all at once.

dsingleton added a commit that referenced this pull request Feb 16, 2016
Increase margins between text in govspeak component
@dsingleton dsingleton merged commit 293593d into master Feb 16, 2016
1 check passed
1 check passed
default "Build #837 succeeded on Jenkins"
Details
@dsingleton dsingleton deleted the spaced-out-govspeak branch Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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