nhsuk-frontend icon indicating copy to clipboard operation
nhsuk-frontend copied to clipboard

Adjust spacing for back link

Open frankieroberto opened this issue 1 year ago • 9 comments

The current default spacing for the back link component means that, when used in the correct position within the page template it appears too close to the header.

This adds some spacing before the back link, using the spacing given in the transaction page example in page templates, which currently manually adjusts the spacing by adding this class to the back link: nhsuk-u-margin-top-4.

Setting the default spacing before the link would mean that the override class doesn’t have to be added to every back link on every page. However the spacing could still be overridden if needed.

Screenshots

Before After
Screenshot 2024-06-19 at 12 00 05 Screenshot 2024-06-19 at 12 00 13

Checklist

frankieroberto avatar Jun 10 '24 20:06 frankieroberto

Does this make sense? I was confused why, when following the guidance in the page template to use the outerContent block for the Back link (which puts in outside <main> element but inside the <div class="nhsuk-width-container">), the default spacing was off.

@paulrobertlloyd pointed out that the transaction page example uses override classes to fix the spacing - but perhaps this should just be the default?

The downside is that this might break usages of the Back link component which don’t follow the guidance to include it within outerContent and rely on the current spacing defaults - those usages would have to either use override classes or move the back link to the suggested HTML location.

The current Back link guidance isn't explicit about precisely where in the HTML the component should go, even suggesting that a couple of services have "found that it works better at the bottom of the page below the primary action". However when I checked both examples (111 online and 'Find out why your NHS data matters'), they either have the back link at the top or not at all, so maybe that is out of date?

The reason given for having the back link at the bottom ("we don't want to suggest to people who use a screen reader that they leave the page prematurely") is curious. One of the reasons for having the back link outside the <main> element is so that it gets skipped when using the "skip to main content link".

If we agree the adjusting the default spacing makes sense, then we could also update the guidance on the Back link page to more explicitly say where it should appear in the HTML if appearing at the top of the page (ie before the <main>), and perhaps either drop mention of it appearing below the primary action, or suggest that if you place it elsewhere you may need to adjust the spacing?

frankieroberto avatar Jun 10 '24 21:06 frankieroberto

Digging a bit further into the 111 online service, it seems that some later pages (once you’ve selected a topic) do have links to go back to the previous question beneath the main Continue button - but these are regular links labelled as 'Change my previous answer' without the back arrow... (technically they're buttons styled as links)

Screenshot 2024-06-11 at 08 59 47

frankieroberto avatar Jun 11 '24 09:06 frankieroberto

Just to note, this would be a breaking change for anyone currently using the back link

mikemonteith avatar Jun 19 '24 11:06 mikemonteith

@mikemonteith:

Just to note, this would be a breaking change for anyone currently using the back link

I've never quite sure what counts as a breaking change (different projects have different definitions!) but I agree that if services have added extra spacing in some other way then they might need to make some HTML changes.

If they’re currently using a modifier class on the back link like nhsuk-u-margin-top-4 (see example then it should be unaffected.

We released a minor change to nested list spacing in v8.2.0 but I suspect that’s a lot less likely to affect any live services as nested lists are so rare.

This could wait for a version 9 release if we want to be cautious though? Thoughts @anandamaryon1?

frankieroberto avatar Jun 19 '24 14:06 frankieroberto

This might want to be thought of in conjunction with breadcrumbs. We'd want the same spacing for both, but I'm not sure the current spacing is good - the h1 ends up very close to the back link and breadcrumbs.

Aside: the breadcrumbs sass removes the top padding of main, which I'm sure about.

edwardhorsford avatar Jun 19 '24 14:06 edwardhorsford

I had a quick check of the Find your NHS number service and they seem to have solved this issue by adding padding to the top instead in some custom css:

.nhsuk-back-link {
  padding-top: 16px;
  margin-bottom: 0; 
}
  
@media (min-width: 40.0625em) {
  .nhsuk-back-link {
    padding-top: 24px; 
  } 
}

...which probably suggests that this should be a breaking change, as it'd cause that service to have too much spacing (existing padding + new margin) if they didn't remove their CSS when upgrading nhsuk-frontend?

It does add some good evidence of some default spacing above back links being needed though!

frankieroberto avatar Jun 19 '24 14:06 frankieroberto

Register with a GP surgery has the back link within the <main> element instead of outside it as recommended, so the spacing is different again.

Screenshot 2024-06-19 at 15 52 19

I'm leaning towards releasing this as a breaking change, along with some improved guidance on where to put the back link (and breadcrumbs) in the HTML?

frankieroberto avatar Jun 19 '24 14:06 frankieroberto

Agree that the spacing should be included in the component, rather than relying on adding utility classes, for consistency and ease of use.

But how this works relies on where the back link is placed in the HTML, so I think we need to update the guidance and also the page layout guidance to work alongside this and specify where it should be placed. And making sure that this isn't within the main. And as Ed mentioned, it should be placed and spaced the same as the Breadcrumbs.

anandamaryon1 avatar Jun 19 '24 16:06 anandamaryon1

@anandamaryon1 agree with you on the Breadcrumbs component, but unlike the Back link, that one currently includes its own <div class="nhsuk-width-container"> element, so needs to go in the beforeContent block, whereas the Back link needs to go in outerContent.

This is different from the GOVUK design system, where both components go in beforeContent and there is no outerContent.

Compare NHS page template options to GOVUK page template options.

Not sure how the difference came about or if there’s a good reason for it?

frankieroberto avatar Jun 19 '24 16:06 frankieroberto

@roshaanbajwa would you mind running the backstop tests on this and updating the PR with the updated images? The only changes should just be some minor changes to the Back link vertical spacing.

frankieroberto avatar Jul 09 '24 21:07 frankieroberto

Closing in favour of #1002

frankieroberto avatar Aug 28 '24 14:08 frankieroberto