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

Removing font style adjustments #1441

Merged
merged 6 commits into from
Jun 11, 2019
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
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,21 @@

🔧 Fixes:

- Removed adjustments that were needed for v1 Transport

If v2 font is being used then there is no need for the adjustments that are currently used to compensate for the baseline issues of v1 Transport

The following components were updated:
- Back-link
- Breadcrumbs
- Button (mostly affected start button)
- Header
- Tags

If v1 font is being used then the adjustments will still automatically be included in the compiled css.

([PR #1441](https://github.com/alphagov/govuk-frontend/pull/1441))

- Stop appending hash when error summary link clicked

This prevents incorrectly focusing the form element with the hash id, instead of the error summary, when form is re-submitted with the hash in the URL and there are further errors.
Expand Down
20 changes: 11 additions & 9 deletions src/components/back-link/_back-link.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,23 @@
// Vertically align with the parent element
position: absolute;

top: -1px;
bottom: 1px;
top: 0;
bottom: 0;
left: 0;

margin: auto;
}
}

// Begin adjustments for font baseline offset
// These should be removed when the font is updated with the correct baseline
@if $govuk-use-legacy-font {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like using this instead of compatibility mode, feels more explicit 👍

// Begin adjustments for font baseline offset
// These should be removed when legacy font support is dropped
.govuk-back-link:before {
$offset: 1px;

.govuk-back-link:before {
$offset: 1px;

top: $offset * -1;
bottom: $offset;
top: $offset * -1;
bottom: $offset;
}
}

}
21 changes: 10 additions & 11 deletions src/components/breadcrumbs/_breadcrumbs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,16 @@

position: absolute;

// Begin adjustments for font baseline offset
// These should be reverted when the font is updated with the correct
// baseline

// top: 0;
// bottom: 0;

top: -1px;
bottom: 1px;

// End adjustments for font baseline offset
@if $govuk-use-legacy-font {
// Begin adjustments for font baseline offset
// These should be removed when legacy font support is dropped
top: -1px;
bottom: 1px;

} @else {
top: 0;
bottom: 0;
}

// Offset by the difference between the width of the non-rotated square
// and its width when rotated
Expand Down
30 changes: 11 additions & 19 deletions src/components/button/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
width: 100%;
margin-top: 0;
@include govuk-responsive-margin(6, "bottom", $adjustment: $button-shadow-size); // s2
padding: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2)) govuk-spacing(2); // s1
padding: (govuk-spacing(2) - $govuk-border-width-form-element) govuk-spacing(2) (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2)); // s1
border: $govuk-border-width-form-element solid transparent;
border-radius: 0;
color: $govuk-button-text-colour;
Expand Down Expand Up @@ -226,16 +226,10 @@
display: inline-flex;
min-height: auto;

padding-top: govuk-spacing(2) - $govuk-border-width-form-element;
padding-right: govuk-spacing(3);
padding-bottom: govuk-spacing(2) - $govuk-border-width-form-element;
padding-left: govuk-spacing(3);

justify-content: center;
}

.govuk-button__start-icon {
margin-top: -3px;
margin-left: govuk-spacing(1);

@include govuk-media-query($from: desktop) {
Expand All @@ -246,19 +240,17 @@
align-self: center;
}

// Begin adjustments for font baseline offset
// These should be removed when the font is updated with the correct baseline
// For the 1px addition please see https://github.com/alphagov/govuk-frontend/pull/365#discussion_r154349428

$offset: 2;
@if $govuk-use-legacy-font {
// Begin adjustments for font baseline offset when using v1 of nta
$offset: 2;

.govuk-button {
padding-top: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) + $offset); // s1
padding-bottom: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) - $offset + 1); // s1
}
.govuk-button {
padding-top: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) + $offset); // s1
padding-bottom: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) - $offset + 1); // s1
}

.govuk-button--start {
padding-top: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) + $offset); // s1
padding-bottom: (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2) - $offset + 1); // s1
.govuk-button__start-icon {
margin-top: -3px;
}
}
}
35 changes: 22 additions & 13 deletions src/components/header/_header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@
}

.govuk-header__logotype-crown {
position: relative;
top: -1px;
margin-right: 1px;
fill: currentColor;
vertical-align: middle;
vertical-align: top;
}

.govuk-header__logotype-crown-fallback-image {
Expand Down Expand Up @@ -97,7 +99,7 @@

display: inline-block;
font-size: 30px; // We don't have a mixin that produces 30px font size
line-height: 30px;
line-height: 1;

&:link,
&:visited {
Expand Down Expand Up @@ -290,18 +292,25 @@
}
}

// Begin adjustments for font baseline offset
// These should be removed when the font is updated with the correct baseline
.govuk-header__logotype-crown,
.govuk-header__logotype-crown-fallback-image {
position: relative;
top: -4px;
}
@if $govuk-use-legacy-font {
// Begin adjustments for font baseline offset
// These should be removed when the font is updated with the correct baseline
.govuk-header__logotype-crown,
.govuk-header__logotype-crown-fallback-image {
position: relative;
top: -4px;
vertical-align: middle;
}

.govuk-header {
$offset: 3px;
padding-top: $offset;
.govuk-header {
$offset: 3px;
padding-top: $offset;
}

.govuk-header__link--homepage {
line-height: 30px;
}
// End adjustments
}
// End adjustments

}
2 changes: 1 addition & 1 deletion src/components/header/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
class="govuk-header__logotype-crown"
xmlns="http://www.w3.org/2000/svg"
viewbox="0 0 132 97"
height="32"
height="30"
width="36"
>
<path
Expand Down
23 changes: 17 additions & 6 deletions src/components/tag/_tag.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,8 @@

@include govuk-exports("govuk/component/tag") {
.govuk-tag {
@include govuk-font($size: 16, $weight: bold, $line-height: 1.25);

display: inline-block;
padding: 4px 8px;
// Since New Transport sits slightly higher than other common fonts.
// We use intentionally uneven padding to make it balanced, this can be
// removed using the version of the font that has a more common vertical spacing.
padding-bottom: 1px;

// When a user customises their colours often the background is removed,
// by adding a outline we ensure that the tag component still keeps it's meaning.
Expand All @@ -25,6 +19,23 @@

text-decoration: none;
text-transform: uppercase;

@if $govuk-use-legacy-font {
// Since New Transport sits slightly higher than other common fonts.
// We use intentionally uneven padding to make it balanced, this can be
// removed using the version of the font that has a more common vertical spacing.
@include govuk-font($size: 16, $weight: bold, $line-height: 1.25);
padding-top: 4px;
padding-right: 8px;
padding-bottom: 1px;
padding-left: 8px;
} @else {
@include govuk-font($size: 16, $weight: bold, $line-height: 1);
padding-top: 5px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking this could be short hand as a one liner.

padding-right: 8px;
padding-bottom: 4px;
padding-left: 8px;
}
}

.govuk-tag--inactive {
Expand Down