Remove is_page_heading parameter from radio component
#2061
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
danacotoran
changed the title
is_page_heading param from radio component
danacotoran
changed the title
is_page_heading param from radio componentis_page_heading parameter from radio component
danacotoran
force-pushed
the
retire_is_page_heading_param_2
branch
from
f5e425d to
4ac1ce1
Compare
|
Nitpick before I dive into review: I can see that you've been dashing around our codebase removing instances of this buuuuut should this be labelled as a breaking change as we're mutating our API? |
owenatgov
approved these changes
May 17, 2021
danacotoran
force-pushed
the
retire_is_page_heading_param_2
branch
from
4ac1ce1 to
118c1aa
Compare
|
@owenatgov good point Owen. |
alex-ju
added
the
breaking change
danacotoran
force-pushed
the
retire_is_page_heading_param_2
branch
3 times, most recently
from
9a2525b to
a878188
Compare
Recently, the ability to pass a custom heading level to the radio component was introduced. Since a custom heading level is now an option, the is_page_heading parameter became redundant. This parameter simply dictates that the radio component legend heading should be a h1 of size xl by default. It is possible to achieve the same result using heading_level and/or heading_size instead. This removes all mentions of is_page_heading from the radio component. Whenever we need this component to have a h1 in the future we can simply use the heading_level option (in combination with heading_size, if necessary)
danacotoran
force-pushed
the
retire_is_page_heading_param_2
branch
from
a878188 to
8694e7e
Compare
Merged
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
This removes all mentions of the
is_page_headingoption from the radio component.Why
Recently, the ability to pass a custom heading level to the radio component was introduced: #1951
Since specifying a custom heading level is now an option, the
is_page_headingparameter became redundant. This parameter simply dictates that the radio component legend heading should be ah1of sizexlby default.It is possible to achieve the same result using
heading_leveland/orheading_sizeinstead.Whenever we need this component to have a h1 in the future, we can simply use the
heading_leveloption (in combination withheading_size, if necessary).This issue is recorded here #1953
Visual changes
There should be no visual changes as a result of this update, as work has already been done to update all references of the radio component to use
heading_level/heading_sizeinstead ofis_page_heading: true– for instance hereThere are still some references to the
is_page_headingoption in some older, read-only repositories which are using older versions of the gem (Example). In the hypothetical situation where one of these repos is unarchived and updated to the latest version ofgovuk_publishing_components, this would be a breaking change in those repos.