.nhsuk-page-content first child margin-top:0 doesn't handle nested divs

[mikemonteith]

https://github.com/nhsuk/frontend/blob/ff1132862c73f7793a363d1298fd3923f4528fec/dist/packages/globals/settings/_typography.scss#L209

The > *:first-child rule here means that margin-top is not set to zero if we have a div inside .nhsuk-page-content.

Sometimes we use divs inside our main page content. e.g:

<div class="nhsuk-page-content">
  <div class="a-div-for-reasons">
    <p>This p tag has non-zero margin-top</p>
  </div>
  <div>
    <p>More content in here</p>
  </div>
</div>

Should this use case be supported?

[chrimesdev]

This is an issue we should fix. I’ve been thinking about it and I can’t think of a suitable dynamic way to do this without creating issues in other use cases.

I think the only way to do it would be to create some additional helper classes to remove the margin. We could extend this and possibly do something similar to GDS http://design-system.service.gov.uk/styles/spacing/ as @davidhunter08 suggested.

Any other ideas @davidhunter08 @mcheung-nhs @mikemonteith ?

[mikemonteith]

Looks like the gov design system uses margin-bottom on everything and never margin-top. What are the pros/cons of top vs bottom margins?

[mcheung-nhs]

I'm happy to use a simple helper/utility class as @AdamChrimes metioned, and extend if necessary. As for margin top/bottom... https://css-tricks.com/margin-bottom-margin-top/ https://matthewjamestaylor.com/blog/css-margin-top-or-bottom

[chrimesdev]

After looking it on Wagtail the helper classes may be difficult to implement as the sections are generated by Wagtail automatically and we would need some logic to figure out the first section and apply the class to it.

A proposed dynamic hot fix is:

.nhsuk-page-content {
  & > *:first-child {
    margin-top: 0;
    & > *:first-child {
      margin-top: 0;
    }
  }
}

We need to test it properly across different pages and see if theres any knock on effects. However this works for the Wagtail conditions pages from what I've seen from Selenium tests.

I'll implement it on Wagtail and we can have the testers test it on there and give it a trial run.

[chrimesdev]

Fix has been live on www.nhs.uk for a while with no further issues raised. Will close the issue and keep an eye on it.