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

Remove @media width query from print styles #328

Merged
merged 1 commit into from
Sep 22, 2016
Merged

Remove @media width query from print styles #328

merged 1 commit into from
Sep 22, 2016

Conversation

selfthinker
Copy link
Contributor

@selfthinker selfthinker commented Sep 19, 2016 •
edited
Loading

This fixes that print styles using any of the typography mixins would have font sizes in px < 640px width and in pt after and sometimes the wrong line-height.
Different browser print views also have different widths, making the print look less consistent between different browsers.

Please note, I did not test the changes as I don't know how test changes to the frontend toolkit locally.
This should also fix an outstanding issue on alphagov/govuk_template#237.

@selfthinker selfthinker mentioned this pull request Sep 19, 2016
Merged
NickColley
NickColley reviewed Sep 20, 2016
View reviewed changes
stylesheets/_typography.scss
@@ -40,13 +40,13 @@ $is-print: false !default;
font-weight: $font-weight;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we'd still inherit the line-height from here?

Only conditionally having the media query feels arbitrary, maybe it'd be clearer to override the line-height for print?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line-height isn't causing that much trouble. But you're right, it should be moved into the else case, together with the other font-size. And the normal font-size needs to be copied into the print style. I will adjust the commit accordingly.
I'm not sure I understand what you mean with "Only conditionally having the media query feels arbitrary". I would generally never have @media queries inside a print stylesheet. (Although I can think of legitimate reasons, I've never seen it used before.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just force-pushed the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Having less forks calms my simple mind 👍

Will let someone else have a peek at this before merging..

@selfthinker
Remove @media width query from print styles
f75020d 
This fixes that print styles using any of the typography mixins
would have font sizes in `px` < 640px width and in `pt` after
and sometimes the wrong line-height.
Different browser print views also have different widths,
making the print look less consistent between different browsers.
@robinwhittleton
Copy link
Contributor

Happy with this.

@robinwhittleton robinwhittleton merged commit 9db7fd8 into master Sep 22, 2016
@robinwhittleton robinwhittleton deleted the no-at-media-in-print branch September 22, 2016 14:02
selfthinker added a commit to alphagov/govuk_template that referenced this pull request Sep 29, 2016
@selfthinker
Remove obsolete CSS around logo
8bbaa06 
* Setting width and height via CSS is not necessary anymore
  since the dimensions were set in the HTML in
  c0e8bd3
* Removing the border on the logo img is not necessary anymore
  as it was removed on all images globally in
  db43128
* Setting `display` on the image is not necessary as it's also floating
  and everything that floats is always `display: block`
* Setting a `line-height` has no effect on replaced elements like images
* When the styling for the current print logo was changed
  in 0704a5e
  the old styling was not removed
* Using `core-48` together with redefining the font-size and font-weight
  is (nearly) the same as using `bold-80`, the original PR
  alphagov/static#352 mentions that this was not intentional
  This was only changed to `bold-48`, not `bold-80` because of a bug
  in govuk_frontend_toolkit, see alphagov/govuk_frontend_toolkit#328
gemmaleigh added a commit to alphagov/govuk-prototype-kit that referenced this pull request Oct 3, 2016
@gemmaleigh
Update the govuk_frontend_toolkit to 4.18.2
287ffab 
- Remove unnecessary print font fallback that causes regression
downstream ([PR
#328](alphagov/govuk_frontend_toolkit#328))
@gemmaleigh gemmaleigh mentioned this pull request Oct 3, 2016
Merged
gemmaleigh added a commit to alphagov/govuk_elements that referenced this pull request Oct 3, 2016
@gemmaleigh
Update the govuk_frontend_toolkit to 4.18.2
5f7d46a 
- Remove unnecessary print font fallback that causes regression
downstream ([PR
#328](alphagov/govuk_frontend_toolkit#328))
@gemmaleigh gemmaleigh mentioned this pull request Oct 3, 2016
Merged
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 7, 2016
@gemmaleigh
Bump the govuk frontend toolkit to 4.18.3
df2f761 
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG
.md

# 4.18.3

- For smaller screens (<768px) ensure that the
GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was
previously "stuck" (by removing both the class which sets fixed
positioning and the shim). ([PR
#329](alphagov/govuk_frontend_toolkit#329))

# 4.18.2

- Remove unnecessary print font fallback that causes regression
downstream ([PR
#328](alphagov/govuk_frontend_toolkit#328))
- Update tooling to use npm script rather than grunt-shell ([PR
#327](alphagov/govuk_frontend_toolkit#327))

# 4.18.1

- Fix error in IE - remove trailing comma from shimLinksWithButtonRole
JavaScript ([PR
#323](alphagov/govuk_frontend_toolkit#323)).

# 4.18.0

- Add GOVUK.ShowHideContent JavaScript to support showing and hiding
content, toggled by radio buttons and checkboxes ([PR
#315](alphagov/govuk_frontend_toolkit#315)).

# 4.17.0

- SelectionButtons will add a class to the label with the type of the
child input ([PR
#317](alphagov/govuk_frontend_toolkit#317))

# 4.16.1

- Fix anchor-buttons.js to trigger a native click event, also rename to
shimLinksWithButtonRole.js to explain what it does
- Add tests for shimLinksWithButtonRole ([PR
#310](alphagov/govuk_frontend_toolkit#310))

# 4.16.0

- Add Department for International Trade organisation ([PR
#308](alphagov/govuk_frontend_toolkit#308))

# 4.15.0

- Add support for Google Analytics fieldsObject ([PR
#298](alphagov/govuk_frontend_toolkit#298))
- anchor-buttons.js: normalise keyboard behaviour between buttons and
links with a button role ([PR
#297](alphagov/govuk_frontend_toolkit#297))

# 4.14.1

- Fix tabular number sizing in Firefox ([PR
#301](alphagov/govuk_frontend_toolkit#301))

# 4.14.0

- Allow use of multiple GA customDimensionIndex. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti
cal-model) of the documentation for more information.
- Configurable duration (in days) for AB Test cookie. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#multivariate-test-framework) of the documentation
for more information.
- Allow base scripts to run within a module loader. See [this
PR](alphagov/govuk_frontend_toolkit#290) for
more information.

# 4.13.0

- Make headings block-level by default (PR #200). If you are styling
elements you want to be inline with heading includes, you’ll need to
explicitly make them inline in your CSS.

# 4.12.0

- Increase button padding to match padding from GOV.UK elements (PR
#275).
If you have UI which depends on the padding set by the button mixin in
the frontend toolkit and this is not overridden by button padding set
by GOV.UK elements, this change will affect it.

# 4.11.0

- Remove the GDS-Logo font-face definition (PR #272)
- Move the @Viewport statements to govuk_template (PR #272). If you
upgrade to this version of govuk_frontend_toolkit and you’re also using
govuk_template you’ll need to upgrade that to at least 0.17.2 to
maintain compatibility with desktop IE10 in snap mode.
@gemmaleigh gemmaleigh mentioned this pull request Oct 7, 2016
Merged
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 8, 2016
@gemmaleigh
Bump the govuk frontend toolkit to 4.18.3
09eccff 
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG
.md

4.18.3

- For smaller screens (<768px) ensure that the
GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was
previously "stuck" (by removing both the class which sets fixed
positioning and the shim). ([PR 329](alphagov/govuk_frontend_toolkit#329))

4.18.2

- Remove unnecessary print font fallback that causes regression
downstream ([PR 328](alphagov/govuk_frontend_toolkit#328))
- Update tooling to use npm script rather than grunt-shell ([PR 327](alphagov/govuk_frontend_toolkit#327))

4.18.1

- Fix error in IE - remove trailing comma from shimLinksWithButtonRole
JavaScript ([PR 323](alphagov/govuk_frontend_toolkit#323)).

4.18.0

- Add GOVUK.ShowHideContent JavaScript to support showing and hiding
content, toggled by radio buttons and checkboxes ([PR 315](alphagov/govuk_frontend_toolkit#315)).

4.17.0

- SelectionButtons will add a class to the label with the type of the
child input ([PR 317](alphagov/govuk_frontend_toolkit#317))

4.16.1

- Fix anchor-buttons.js to trigger a native click event, also rename to
shimLinksWithButtonRole.js to explain what it does
- Add tests for shimLinksWithButtonRole ([PR 310](alphagov/govuk_frontend_toolkit#310))

4.16.0

- Add Department for International Trade organisation ([PR 308](alphagov/govuk_frontend_toolkit#308))

4.15.0

- Add support for Google Analytics fieldsObject ([PR 298](alphagov/govuk_frontend_toolkit#298))
- anchor-buttons.js: normalise keyboard behaviour between buttons and
links with a button role ([PR 297](alphagov/govuk_frontend_toolkit#297))

4.14.1

- Fix tabular number sizing in Firefox ([PR 301](alphagov/govuk_frontend_toolkit#301))

4.14.0

- Allow use of multiple GA customDimensionIndex. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti
cal-model) of the documentation for more information.
- Configurable duration (in days) for AB Test cookie. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#multivariate-test-framework) of the documentation
for more information.
- Allow base scripts to run within a module loader. See [this
PR](alphagov/govuk_frontend_toolkit#290) for
more information.

4.13.0

- Make headings block-level by default (PR #200). If you are styling
elements you want to be inline with heading includes, you’ll need to
explicitly make them inline in your CSS.

4.12.0

- Increase button padding to match padding from GOV.UK elements (PR 275).
If you have UI which depends on the padding set by the button mixin in
the frontend toolkit and this is not overridden by button padding set
by GOV.UK elements, this change will affect it.

4.11.0

- Remove the GDS-Logo font-face definition (PR #272)
- Move the @Viewport statements to govuk_template (PR #272). If you
upgrade to this version of govuk_frontend_toolkit and you’re also using
govuk_template you’ll need to upgrade that to at least 0.17.2 to
maintain compatibility with desktop IE10 in snap mode.
gemmaleigh added a commit to alphagov/service-manual-frontend that referenced this pull request Oct 8, 2016
@gemmaleigh
Bump the govuk frontend toolkit to 4.18.3
a3e43dc 
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG
.md

4.18.3

- For smaller screens (<768px) ensure that the
GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was
previously "stuck" (by removing both the class which sets fixed
positioning and the shim). ([PR 329](alphagov/govuk_frontend_toolkit#329))

4.18.2

- Remove unnecessary print font fallback that causes regression
downstream ([PR 328](alphagov/govuk_frontend_toolkit#328))
- Update tooling to use npm script rather than grunt-shell ([PR 327](alphagov/govuk_frontend_toolkit#327))

4.18.1

- Fix error in IE - remove trailing comma from shimLinksWithButtonRole
JavaScript ([PR 323](alphagov/govuk_frontend_toolkit#323)).

4.18.0

- Add GOVUK.ShowHideContent JavaScript to support showing and hiding
content, toggled by radio buttons and checkboxes ([PR 315](alphagov/govuk_frontend_toolkit#315)).

4.17.0

- SelectionButtons will add a class to the label with the type of the
child input ([PR 317](alphagov/govuk_frontend_toolkit#317))

4.16.1

- Fix anchor-buttons.js to trigger a native click event, also rename to
shimLinksWithButtonRole.js to explain what it does
- Add tests for shimLinksWithButtonRole ([PR 310](alphagov/govuk_frontend_toolkit#310))

4.16.0

- Add Department for International Trade organisation ([PR 308](alphagov/govuk_frontend_toolkit#308))

4.15.0

- Add support for Google Analytics fieldsObject ([PR 298](alphagov/govuk_frontend_toolkit#298))
- anchor-buttons.js: normalise keyboard behaviour between buttons and
links with a button role ([PR 297](alphagov/govuk_frontend_toolkit#297))

4.14.1

- Fix tabular number sizing in Firefox ([PR 301](alphagov/govuk_frontend_toolkit#301))

4.14.0

- Allow use of multiple GA customDimensionIndex. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#using-google-custom-dimensions-with-your-own-statisti
cal-model) of the documentation for more information.
- Configurable duration (in days) for AB Test cookie. See [this
section](https://github.com/alphagov/govuk_frontend_toolkit/blob/master/
docs/javascript.md#multivariate-test-framework) of the documentation
for more information.
- Allow base scripts to run within a module loader. See [this
PR](alphagov/govuk_frontend_toolkit#290) for
more information.

4.13.0

- Make headings block-level by default (PR #200). If you are styling
elements you want to be inline with heading includes, you’ll need to
explicitly make them inline in your CSS.

4.12.0

- Increase button padding to match padding from GOV.UK elements (PR 275).
If you have UI which depends on the padding set by the button mixin in
the frontend toolkit and this is not overridden by button padding set
by GOV.UK elements, this change will affect it.

4.11.0

- Remove the GDS-Logo font-face definition (PR #272)
- Move the @Viewport statements to govuk_template (PR #272). If you
upgrade to this version of govuk_frontend_toolkit and you’re also using
govuk_template you’ll need to upgrade that to at least 0.17.2 to
maintain compatibility with desktop IE10 in snap mode.
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
@Guntrisoft
Upgrade frontend toolkit to version 5.0.1
c248440 
- Removed phase-banner scss file as no longer needed

Full list of changes:

SelectionButtons will add a class to the label with the type of the child input
alphagov/govuk_frontend_toolkit#317

Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes
alphagov/govuk_frontend_toolkit#315

Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript
alphagov/govuk_frontend_toolkit#323

Remove unnecessary print font fallback that causes regression downstream
alphagov/govuk_frontend_toolkit#328

Update tooling to use npm script rather than grunt-shell
alphagov/govuk_frontend_toolkit#327

For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
alphagov/govuk_frontend_toolkit#329

Lint codebase using standard
alphagov/govuk_frontend_toolkit#334

Add semicolons at the start of IIFE's
alphagov/govuk_frontend_toolkit#339

Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase
alphagov/govuk_frontend_toolkit#293

Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon
alphagov/govuk_frontend_toolkit#345

Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set
alphagov/govuk_frontend_toolkit#343

Allow custom options in GOVUK.analytics.trackPageview
alphagov/govuk_frontend_toolkit#332

Fix role="button" click shim
alphagov/govuk_frontend_toolkit#347

Make font variables lowercase
alphagov/govuk_frontend_toolkit#348

Update selection button documentation
alphagov/govuk_frontend_toolkit#350

Change colour used in phase tags to govuk-blue
alphagov/govuk_frontend_toolkit#353
@Guntrisoft Guntrisoft mentioned this pull request Dec 2, 2016
Merged
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
@Guntrisoft
Upgrade frontend toolkit to version 5.0.2
5ac53b8 
- Removed phase-banner scss file as no longer needed

Full list of changes:

SelectionButtons will add a class to the label with the type of the child input
alphagov/govuk_frontend_toolkit#317

Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes
alphagov/govuk_frontend_toolkit#315

Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript
alphagov/govuk_frontend_toolkit#323

Remove unnecessary print font fallback that causes regression downstream
alphagov/govuk_frontend_toolkit#328

Update tooling to use npm script rather than grunt-shell
alphagov/govuk_frontend_toolkit#327

For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
alphagov/govuk_frontend_toolkit#329

Lint codebase using standard
alphagov/govuk_frontend_toolkit#334

Add semicolons at the start of IIFE's
alphagov/govuk_frontend_toolkit#339

Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase
alphagov/govuk_frontend_toolkit#293

Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon
alphagov/govuk_frontend_toolkit#345

Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set
alphagov/govuk_frontend_toolkit#343

Allow custom options in GOVUK.analytics.trackPageview
alphagov/govuk_frontend_toolkit#332

Fix role="button" click shim
alphagov/govuk_frontend_toolkit#347

Make font variables lowercase
alphagov/govuk_frontend_toolkit#348

Update selection button documentation
alphagov/govuk_frontend_toolkit#350

Change colour used in phase tags to govuk-blue
alphagov/govuk_frontend_toolkit#353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NickColley NickColley NickColley left review comments

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
@selfthinker @robinwhittleton @NickColley