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

Wrap sass-mq as govuk-media-query #763

Merged
merged 3 commits into from
Jun 7, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ Note: We're not following semantic versioning yet, we are going to talk about th
- Remove `pageStart` block from template, as could result in rendering issues in older IE.
([PR #765](https://github.com/alphagov/govuk-frontend/pull/765))

- You should no longer call the `mq` mixin directly - you should replace any
calls to it from your own code with `govuk-media-query` which accepts the same
arguments. All mixins and settings that start with `mq-` should be considered
private – they could be removed in the future without notice.
([PR #763](https://github.com/alphagov/govuk-frontend/pull/763))

🔧 Fixes:

- Fix govuk-equilateral-height function usage in shape-arrow helper
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
border-bottom: $button-shadow-size solid $govuk-button-shadow-colour;
}

@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
width: auto;
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/error-summary/_error-summary.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

border: $govuk-border-width-mobile solid $govuk-error-colour;

@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
border: $govuk-border-width solid $govuk-error-colour;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/header/_header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
margin-right: 0;
}

@include mq($media-type: print) {
@include govuk-media-query($media-type: print) {
.govuk-header {
border-bottom-width: 0;
color: $govuk-black;
Expand Down
2 changes: 1 addition & 1 deletion src/components/panel/_panel.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

text-align: center;

@include mq($until: tablet) {
@include govuk-media-query($until: tablet) {
padding: $govuk-spacing-scale-6 - $govuk-border-width;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/_lists.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
}

%govuk-list > li {
@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
margin-bottom: $govuk-spacing-scale-1;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/core/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
margin-bottom: $govuk-spacing-scale-1;
color: $govuk-secondary-text-colour;

@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
margin-bottom: 0;
}
}
Expand Down Expand Up @@ -171,7 +171,7 @@
%govuk-body-l + %govuk-heading-l {
padding-top: $govuk-spacing-scale-1;

@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
padding-top: $govuk-spacing-scale-2;
}
}
Expand All @@ -190,7 +190,7 @@
%govuk-list + %govuk-heading-s {
padding-top: $govuk-spacing-scale-1;

@include mq($from: tablet) {
@include govuk-media-query($from: tablet) {
padding-top: $govuk-spacing-scale-2;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/_grid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ $_govuk-grid-widths: (
width: 100%;
}
padding: 0 $govuk-gutter-half;
@include mq($from: $at) {
@include govuk-media-query($from: $at) {
width: grid-width($width);
float: $float;
}
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/_links.scss
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
/// @access public

@mixin govuk-link-print-friendly {
@include mq($media-type: print) {
@include govuk-media-query($media-type: print) {
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for not using single letter variables for this new mixin


&[href^="/"],
&[href^="http://"],
Expand Down
72 changes: 69 additions & 3 deletions src/helpers/_media-queries.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// Import sass-mq as a helper for media queries
////
/// @group helpers
////



// =========================================================
// Wrangle sass-mq config...
// =========================================================

// Pass our breakpoints and static breakpoint definitions through to sass-mq.
$mq-breakpoints: if(variable-exists(govuk-breakpoints), $govuk-breakpoints, ());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we do this configuration 'at import time', any changes to $govuk-is-ie8, $govuk-breakpoints, $govuk-ie8-breakpoint made after the import has been done will not have any effect on govuk-media-query. We could move this wrangling into the govuk-media-query mixin, but it complicates the way we pass arguments somewhat, and relies on us being able to pass that config to mq which is an undocumented feature, and I can't think of any valid use cases where you'd want to change that config mid-way through your codebase.

Thoughts?

Expand All @@ -13,10 +21,9 @@ $mq-show-breakpoints: ();
// When building a stylesheet for IE8, set $mq-responsive to false in order to
// 'rasterize' any media queries.

$mq-responsive: true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving this because the way Sass variable scoping works means that unless $mq-responsive is defined before this point, the $mq-responsive declaration only works within the scope of the @if – which caused a lot of headaches in the tests.

@if (variable-exists(govuk-is-ie8) and $govuk-is-ie8) {
$mq-responsive: false;
} @else {
$mq-responsive: true;
}

// This is a horrible, horrible hack to prevent the 'dev mode' CSS to display
Expand All @@ -33,3 +40,62 @@ $sass-mq-already-included: false !default;
@import "../vendor/sass-mq";

$sass-mq-already-included: true;



// =========================================================
// Helpers
// =========================================================

/// Media Query
///
/// This is a currently a wrapper for sass-mq - abstracted so that we can
/// replace it in the future if we so choose.
///
/// @param {String | Boolean} $from [false] - One of $govuk-breakpoints
/// @param {String | Boolean} $until [false] - One of $govuk-breakpoints
/// @param {String | Boolean} $and [false] - Additional media query parameters
/// @param {String} $media-type [all] - Media type: screen, print…
///
/// @ignore Undocumented API, for advanced use only:
/// @ignore @param {Map} $breakpoints [$govuk-breakpoints]
/// @ignore @param {String} $static-breakpoint [$govuk-ie8-breakpoint]
/// @ignore @param {Boolean} $responsive [$govuk-is-ie8]
///
/// @content styling rules, wrapped into a @media query when $responsive is true
///
/// @example scss
/// .element {
/// @include govuk-media-query($from: mobile) {
/// color: red;
/// }
/// @include govuk-media-query($until: tablet) {
/// color: blue;
/// }
/// @include govuk-media-query(mobile, tablet) {
/// color: green;
/// }
/// @include govuk-media-query($from: tablet, $and: '(orientation: landscape)') {
/// color: teal;
/// }
/// @include govuk-media-query(950px) {
/// color: hotpink;
/// }
/// @include govuk-media-query(tablet, $media-type: screen) {
/// color: hotpink;
/// }
///
/// @example scss Advanced use:
/// $my-breakpoints: (L: 900px, XL: 1200px);
/// @include govuk-media-query(L, $breakpoints: $my-breakpoints, $static-breakpoint: L) {
/// color: hotpink;
/// }
/// }
///
/// @access public

@mixin govuk-media-query($args...) {
@include mq($args...) {
@content;
};
}
2 changes: 1 addition & 1 deletion src/helpers/_spacing.scss
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
#{$property}-#{$direction}: $breakpoint-value iff($important, !important);
}
} @else {
@include mq($from: $breakpoint) {
@include govuk-media-query($from: $breakpoint) {
@if $direction == all {
#{$property}: $breakpoint-value iff($important, !important);
} @else {
Expand Down
8 changes: 4 additions & 4 deletions src/helpers/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
@include _govuk-font-face-nta;
}

@include mq($media-type: print) {
@include govuk-media-query($media-type: print) {
font-family: $govuk-font-family-print;
}
}
Expand All @@ -34,7 +34,7 @@
@mixin govuk-text-colour {
color: $govuk-text-colour;

@include mq($media-type: print) {
@include govuk-media-query($media-type: print) {
color: $govuk-print-text-colour;
}
}
Expand Down Expand Up @@ -112,12 +112,12 @@
font-size: $font-size;
line-height: $line-height;
} @elseif $breakpoint == "print" {
@include mq($media-type: print) {
@include govuk-media-query($media-type: print) {
font-size: $font-size;
line-height: $line-height;
}
} @else {
@include mq($from: $breakpoint) {
@include govuk-media-query($from: $breakpoint) {
font-size: $font-size;
line-height: $line-height;
}
Expand Down
Loading