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

Improve print styles #882

Merged
merged 6 commits into from Jan 24, 2017
Merged

Improve print styles #882

merged 6 commits into from Jan 24, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jan 20, 2017

  • Indicate end of content and beginning of footnotes
  • Ensure headings are bold when printing
  • Hide sticky contents link when printing
  • Use decimal list styles for ordered lists - fixes https://govuk.zendesk.com/agent/tickets/1593796
  • Update mainstream guide print file to use modern typography mixins
    • Delete old and un-used typography mixins
Feature Old New
Mainstream guide mainstream_guide_print_old mainstream_guide_print_new
Mainstream screen shot 2017-01-20 at 13 02 44 screen shot 2017-01-20 at 13 02 24
HTML publication screen shot 2017-01-20 at 13 16 53 screen shot 2017-01-20 at 13 17 31
fofr added 6 commits Jan 20, 2017
* Matches non-print styles but uses text colour
At the top of the print styles we set the font weight for all headings
to be bold. But we were using the `core` mixin which resets those
styles to be font-weight: 400.

Use `bold-xx` instead of `core-xx` to maintain the bold styles.
Al ink back to the top of the page is not useful in print.
When printing content, ordered lists were rendered without the correct
numbers.

This will also render numbers against ordered lists used elsewhere that
don’t have a specific reset applied.
Mainstream guides have their own print view which had outdated
typography mixins imported from styleguide/typography.

These didn’t include `bold-xx` mixins and so compilation of Sass would
fail when running tests.

* Switch to the latest mixins
* Delete the old mixins, now un-used
`print-base` sets heading styles for h2, h3 and h4 as bold.

For guides, h1s can appear within the page. When they are not also
bold, the structure of the content isn’t as clear.

* Make h1s bold for older layouts when printing
Copy link
Contributor

@gpeng gpeng left a comment

This looks good to me but I'd appreciate a second more knowledgable opinion. cc/ @nickcolley

@@ -1,218 +0,0 @@
@import "_print.scss";

This comment has been minimized.

@nickcolley

nickcolley Jan 20, 2017
Contributor

Are we sure these mixins were not being used? Just thinking these could be out of date and be used, so us removing them could be a regression.

This comment has been minimized.

@fofr

fofr Jan 20, 2017
Author Contributor

_print sets:

@mixin print {
  @if $is-print {
    @content;
  }
}

The only use of @include print was in styleguide/_typography. If something had been using it I would have expected to see a compile error from sass.

This comment has been minimized.

@nickcolley

nickcolley Jan 20, 2017
Contributor

I was referring to the type mixins, is that the same story?

This comment has been minimized.

@fofr

fofr Jan 20, 2017
Author Contributor

I've removed the only reference to styleguide/_typography.scss. There are no Sass compile errors.

Copy link
Contributor

@nickcolley nickcolley left a comment

This looks really great, I just have a comment about the removal of mixins. 👍

@gpeng
gpeng approved these changes Jan 23, 2017
@fofr fofr merged commit 2391f1d into master Jan 24, 2017
1 check passed
1 check passed
@govuk-ci
default "Build #1438 succeeded on Jenkins"
Details
@fofr fofr deleted the fix-ordered-list-print-styles branch Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants