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.

Sign up for GitHub

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

Jump to bottom

core-layout printing #638

Merged
merged 3 commits into from
Aug 24, 2015
Merged

core-layout printing #638

merged 3 commits into from
Aug 24, 2015

Conversation

robinwhittleton
Copy link
Contributor

This PR sets up a new print stylesheet specific to core-layout (and header-footer-layout) that only contains the necessary styles.

Remove logo print styles
b7a604d 
They now live in govuk_template print stylesheets
@robinwhittleton
Copy link
Contributor Author

@dsingleton opinion on this?

Add a core-layout-specific print stylesheet
bbbf7d8 
First, this makes print stylesheets layout specific. Then, for core-layout (and header-footer-only), we add a new print-base stylesheet that only contains the styling needed. This also adds print-specific stylesheets for components that need print styles.
@dsingleton
Copy link
Contributor

This LGTM.

Some of the (older) naming makes this a little harder to follow than it should be (static-print pulling in core-print is a little confusing. Not caused by this PR, but until we can get rid of the legacy(ish) static layout it's going to be confusing :/

@dsingleton
Copy link
Contributor

One quick question, static.css pulls in header-footer-only, but the print equivalent doesn't, i'm not sure if this will causes problems with real pages, but it is unintuitive.

@robinwhittleton
Copy link
Contributor Author

Hmm, good point. I’m not entirely sure what the relationship here is: was under the impression that header-footer-only was something completely new. @edds ?

@edds
Copy link
Contributor

edds commented Aug 24, 2015

Good point @dsingleton.

header-footer-only.css is supposed to be a subset of static.css. It would probably make sense to remove anything from _core-print.scss that also exists in _print-base.css. Then change static-print.scss to first include header-footer-only-print.scss then also include what is left in _core-print.scss. If that makes sense @robinwhittleton?

Static printing should not duplicate print styles
8cf9f92 
Currently static’s print stylesheet duplicates a lot of the styles in the new header-footer-only print stylesheet. This imports header-footer-only-print into static-print and removes the duplicates in core-print.
@robinwhittleton
Copy link
Contributor Author

@edds: Pushed a commit that sorts that out. Look OK?

edds added a commit that referenced this pull request Aug 24, 2015
@edds
Merge pull request #638 from alphagov/core_layout_printing
881a181 
core-layout printing
@edds edds merged commit 881a181 into master Aug 24, 2015
@edds edds deleted the core_layout_printing branch August 24, 2015 10:02
@edds edds mentioned this pull request Aug 25, 2015
Merged
@dsingleton dsingleton mentioned this pull request Jan 12, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@robinwhittleton @dsingleton @edds