NHSLeadership / nightingale

NHS-Generic Frontend Framework.
Other
35 stars 8 forks source link

[refs #00224] Fix double heading border #225

Closed Android63 closed 6 years ago

Android63 commented 6 years ago

Following an earlier update, the faux bottom border was no longer being displayed. I investigated and it was being displayed as a table so I changed it to block.

cehwitham commented 6 years ago

Was there a reason for it being displayed as a table initially?

It used to work, so it would be good to understand what broke it. Anything in the header layout changes we made?

Android63 commented 6 years ago

I'll look into it @cehwitham

Android63 commented 6 years ago

@csswizardry could you take a look at this please? I believe the issue occurred after you updated tools/_tools.mixins.scss to use table rather than block when there's both a before and after pseudo element. See https://github.com/NHSLeadership/nightingale/commit/4d4f48acdc1375e4d3622e5c2b861249442325bc

Android63 commented 6 years ago

I've marked my fix and raised an issue to get Harry to review it. While I was here I fixed the issue that the faux double border was overwriting the ribbon or breadcrumbs directly below it.

This shows the page-specific ribbon being overwritten by the faux border:

screen shot 2018-05-11 at 12 05 04

This is the result of increasing the top margin on the ribbon to compensate:

screen shot 2018-05-11 at 12 05 44

This shows the same problem occurring with breadcrumbs:

screen shot 2018-05-11 at 12 06 12

This is the result of increasing the margin for breadcrumbs:

screen shot 2018-05-11 at 12 06 49

This shows that increasing the breadcrumb margin causes a gap to appear when it sits under a ribbon:

screen shot 2018-05-11 at 12 18 30

This is the result of using CSS sibling relationships to apply a larger negative top margin when a ribbon is present above the breadcrumbs:

screen shot 2018-05-11 at 12 07 13

Android63 commented 6 years ago

@cehwitham could you review this please?