Update usage of the title component #3918
Conversation
- moving towards a world where the title component has no margin_top option: alphagov/govuk_publishing_components#4508 - this removes instances of the use of the margin_top option for the title component - in some cases it was possible to replace the title component with the heading component for no visual change (where the margin_top: 0 option was applied) - in other cases there is a visual change, but this will be minimal
andysellick
changed the title
There was a problem hiding this comment.
The changes look good to me overall.
After looking further at the changes to the gem and other apps mentioned in the PR description, it appears that it was possible to swap most of the title components out with a heading component and the context option is only heavily used in collections.
I think this makes it a bit easier avoid adding the padding-top CSS to the title component for now as a workaround, as there are only a few apps left that may need to use the govuk-maim-wrapper class approach.
I've created a PR to do this in collections which also fixes several layout issues - #3944
If this could go live first, then it would mean that there are no visual changes in this PR and could make switching from the title component to the heading component, once the context variant has been added, a bit easier. What do you think about this idea/approach?
|
@MartinJJones happy for your PR to be merged first although I'd suggest if you're adding the |
|
@andysellick I've merged the other PR now which makes better use of govuk-main-wrapper, replacing the title component with the heading component where required. You should now be able to remove the change made to |
What
Why
We're updating the component spacing model to make it more consistent and having a margin_top option isn't part of the intended outcome.
Visual changes
For the coronavirus landing page the title component used the
margin_top: 0option, so we replace the component with the heading component, which means no visual change (app/views/coronavirus_landing_page/components/landing_page/_page_header.html.erb).For the development index page we replace the title component with the heading component. This results in a small visual change, but this is a non-public facing page so isn't a concern (app/views/development/index.html.erb).
For the taxon pages the title component used the
margin_top: 0option, so we replace the component with the heading component, which means no visual change (app/views/taxons/_page_header.html.erb).For the world location news pages the title component used the
margin_top: 0option, but requires the 'context' feature of the title component (not available in the heading component) so we keep it as it is, but remove the option (app/views/world_location_news/show.html.erb). This currently will not produce a visual difference until we merge alphagov/govuk_publishing_components#4508 at which point it will change as shown below (only on desktop and tablet, mobile remains unchanged). I think this change is acceptable, as the title seems quite squashed against the breadcrumb in this instance.