alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.18k stars 325 forks source link

govuk-width-container mixin changes how header looks #1658

Closed jonheslop closed 4 years ago

jonheslop commented 4 years ago

I set

.pay-width-container--wide {
  @include govuk-width-container(1200px);
}

and our page header looked like this Screen Shot 2019-11-20 at 12 18 18

Note the blue border isnt hanging below the black but within it.

Behaviour confirmed with @hannalaakso on the govuk-frontend review app looking at the template-custom example.

edwardhorsford commented 4 years ago

I think this is the same issue I've come across in our service - where an excess margin has appeared (but on chrome only).

On Safari our bottom margin on the UL sits outside the container: Screenshot 2019-11-26 at 16 13 09

On Chrome our bottom margin on the UL sits within the container: Screenshot 2019-11-26 at 16 12 50

We don't have the blue border on our service - so I think we're using the margin to keep content in the same apparent space.

36degrees commented 4 years ago

The header is inside a container with the following classes:

<div class="govuk-header__container govuk-width-container">

.govuk-header__container has a negative margin-bottom: -10px which is what pulls the black bar up behind the blue bottom border.

.govuk-width-container has margin: 0 auto; to center the container.

Both classes have the same specificity. But because .govuk-header__container is defined after .govuk-width-container in the source order, the margin-bottom: -10px wins.

However, when the user generates their own modifier class it (probably) ends up after the .govuk-header__container class, which then means the margin: 0 auto; then wins, removing the negative bottom margin.

I'm not sure the width container should have opinions about vertical margins – should we only set horizontal margins in the .govuk-width-container class?

36degrees commented 4 years ago

Until we fix this in Frontend, I think adding the following to your application's SCSS file should fix it, as it's more specific:

.govuk-header .govuk-header__container {
  margin-bottom: -10px;
}
NickColley commented 4 years ago

I'm not sure the width container should have opinions about vertical margins – should we only set horizontal margins in the .govuk-width-container class?

Sounds good to me, more specific to what we're actually trying to achieve.