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

Ensure govuk-font-size() handles string keys #4713

Merged
merged 4 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ We have deprecated point 14 (14px large screens, 12px small screens) on the GOV.

We will be removing these classes and point 14 on the type scale in GOV.UK Frontend release v6.0.0. From the next major release, you will no longer be able to call the Sass mixins `govuk-font` or `govuk-font-size` with `$size` set to '14'.

This change was introduced in [#4649: Deprecate 14 on the type scale](https://github.com/alphagov/govuk-frontend/pull/4649)
This change was introduced in [#4649: Deprecate 14 on the type scale](https://github.com/alphagov/govuk-frontend/pull/4649) and [#4713: Ensure `govuk-font-size()` handles string keys](https://github.com/alphagov/govuk-frontend/pull/4713)

### Fixes

Expand Down
30 changes: 27 additions & 3 deletions packages/govuk-frontend/src/govuk/helpers/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,41 @@
/// @access public

@mixin govuk-font-size($size, $line-height: false, $important: false) {
@if not map-has-key($govuk-typography-scale, $size) {
@error "Unknown font size `#{$size}` - expected a point from the type scale.";
// Flag font sizes that start with underscores so we can suppress warnings on
// deprecated sizes used internally, for example `govuk-font($size: "_14")`
$size-internal-use-only: str-slice(#{$size}, 1, 1) == "_";

// Remove underscore from font sizes flagged for internal use
@if $size-internal-use-only {
$size: str-slice(#{$size}, 2);
}

// Check for a font map exactly matching the given size
$font-map: map-get($govuk-typography-scale, $size);

// No match? Try with string type (e.g. $size: "16" not 16)
@if not $font-map {
@each $font-size in map-keys($govuk-typography-scale) {
@if not $font-map and #{$font-size} == #{$size} {
$font-map: map-get($govuk-typography-scale, $font-size);
}
}
}

// Still no match? Throw error
@if not $font-map {
@error "Unknown font size `#{$size}` - expected a point from the type scale.";
}

// Check for a deprecation within the type scale
$deprecation: map-get($font-map, "deprecation");

@if $deprecation {
@include _warning(map-get($deprecation, "key"), map-get($deprecation, "message"));
// Warn on deprecated font sizes unless flagged for internal use
@if not $size-internal-use-only {
@include _warning(map-get($deprecation, "key"), map-get($deprecation, "message"));
}

// remove the deprecation map keys so they do not break the breakpoint loop
$font-map: map-remove($font-map, "deprecation");
}
Expand Down
67 changes: 67 additions & 0 deletions packages/govuk-frontend/src/govuk/helpers/typography.unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,73 @@ describe('@mixin govuk-font-size', () => {
})
})

it('outputs CSS when passing size as a string', async () => {
const sass = `
${sassBootstrap}

.foo {
@include govuk-font-size($size: "14")
}
`

await expect(compileSassString(sass)).resolves.toMatchObject({
css: outdent`
.foo {
font-size: 0.75rem;
line-height: 1.25;
}
@media (min-width: 30em) {
.foo {
font-size: 0.875rem;
line-height: 1.4285714286;
}
}
`
})
})

it('outputs CSS using points as strings', async () => {
const sass = `
$govuk-breakpoints: (
desktop: 30em
);

$govuk-typography-scale: (
"small": (
null: (
font-size: 12px,
line-height: 15px
),
print: (
font-size: 14pt,
line-height: 1.5
)
)
);

@import "base";

.foo {
@include govuk-font-size($size: "small")
}
`

await expect(compileSassString(sass)).resolves.toMatchObject({
css: outdent`
.foo {
font-size: 0.75rem;
line-height: 1.25;
}
@media print {
.foo {
font-size: 14pt;
line-height: 1.5;
}
}
`
})
})

it('throws an exception when passed a size that is not in the scale', async () => {
const sass = `
${sassBootstrap}
Expand Down
18 changes: 7 additions & 11 deletions packages/govuk-frontend/src/govuk/overrides/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@
//
// govuk-!-font-size-14 is deprecated
@each $size, $font-map in $govuk-typography-scale {
// Suppress class if the map key is a string with an underscore eg: _14 to
// avoid classes like govuk-!-font-size-_14 getting generated
@if type-of($size) == "number" or str-slice(#{$size}, 1, 1) != "_" {
.govuk-\!-font-size-#{$size} {
$font-map: map-get($govuk-typography-scale, $size);
.govuk-\!-font-size-#{$size} {
$font-map: map-get($govuk-typography-scale, $size);

// Add underscore to deprecated typography scale keys
@if map-has-key($font-map, "deprecation") {
$size: _#{$size};
}

@include govuk-font-size($size, $important: true);
// Add underscore to deprecated typography scale keys
@if map-has-key($font-map, "deprecation") {
$size: _#{$size};
}

@include govuk-font-size($size, $important: true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,19 +157,5 @@ $govuk-typography-scale: (
message: "14 on the type scale is deprecated and will be removed as " +
"a possible option in the next major version."
)
),
_14: (
null: (
font-size: 12px,
line-height: 15px
),
tablet: (
font-size: 14px,
line-height: 20px
),
print: (
font-size: 12pt,
line-height: 1.2
)
)
) !default;