[DO NOT MERGE] Remove title component margin_top option #4508
+6
−23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andysellick
force-pushed
the
title-margin
branch
from
8ec358a to
e2b7968
Compare
andysellick
added a commit
to alphagov/collections
that referenced
this pull request
Dec 19, 2024
- 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
- our spacing model is to apply margin bottom, so this is an anomaly - replace margin_top with default padding on the component, so no visual changes - margin_top option isn't widely used and can be replaced with alternatives, separate PRs will be raised
andysellick
force-pushed
the
title-margin
branch
from
e2b7968 to
62b059e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Applications that use this component, and their use of the
margin_topoption are:collections(changes covered in Update usage of the title component collections#3918)datagovuk_find(no use ofmargin_topso no change required)email-alert-frontend(no use ofmargin_topso no change required)feedback(changes covered in Use heading component instead of title feedback#2042)finder-frontend(no use ofmargin_topso no change required)frontend(changes covered in Page title component changes frontend#4550)government-frontendis a bit problematic, need to get this change deployed so we can switch from the title to heading component: Add context option to heading component #4510)manuals-publisher(changes covered in Replace title component with heading manuals-publisher#2595)releasesearch-adminsmart-answerswhitehallNote that this PR will have to wait for the ones mentioned above and further work to check it is okay before merging
Why
This is part of a wider piece of work to move the margin options from the shared helper into the component wrapper helper and standardise our component spacing model. This is a step towards being able to remove the
margin_topoption from the shared helper.Visual Changes
None.
Trello card: https://trello.com/c/Y0pDWbHw/390-move-some-shared-helper-options-into-component-wrapper