Update title component to meet chosen conventions #1117
Conversation
vanitabarrett
force-pushed
the
refactor-title-component
branch
2 times, most recently
from
5469148 to
71356f9
Compare
| .context { | ||
| @include core-24; | ||
| color: $secondary-text-colour; | ||
| &--long .pub-c-title__text { |
There was a problem hiding this comment.
We try to avoid nesting if it's not required see: https://github.com/alphagov/govuk-frontend/blob/master/docs/coding-standards/css.md#nesting
There was a problem hiding this comment.
Seems as if here, since the long modifier only impacts the text element, we could have a modifier class on .pub-c-title_text instead?
| @@ -2,9 +2,9 @@ | |||
| average_title_length ||= :average | |||
| context ||= false | |||
| %> | |||
| <div class="govuk-title length-<%= average_title_length %>"> | |||
| <div class="pub-c-title pub-c-title--<%= average_title_length %>"> | |||
There was a problem hiding this comment.
I think this means if the long title length is not active we'll have a class of "pub-c-title pub-c-title--" which is not a big deal but seems like it could be avoided?
There was a problem hiding this comment.
It defaults to 'average', no?
There was a problem hiding this comment.
Yeah think you're right, not sure why though?
| assert_select ".pub-c-title__context", text: "Format" | ||
| end | ||
|
|
||
| test "applies title length if supplied" do |
vanitabarrett
force-pushed
the
refactor-title-component
branch
from
71356f9 to
bbedd6a
Compare
|
Searching the codebase for any overrides I found https://github.com/alphagov/collections/blob/cbaae81eed6cf2dac1b4b7885e864d5f5d1e024d/app/views/taxons/_page_header.html.erb#L3 so we need to update collections to use the govuk-title component and then we can ship this change. |
Updating static title component to meet government-frontend component conventions. This includes: * Using pub-c namespace for CSS classes * Use of BEM * Adding accessibility criteria to docs
vanitabarrett
force-pushed
the
refactor-title-component
branch
from
bbedd6a to
7d3f6b1
Compare
|
@NickColley I've had a look at Collections in more detail. It looks like there's about 4 different titles being used on different pages, all with different spacing and uses, among themselves and with our component (see screenshots below, left is current collections title, right is with static title component). Spacing differences: This title also contains a description, which our title component doesn't take as a parameter. If we want to integrate the title component, we'd probably have to override some of the component CSS within collections, which doesn't feel like a good thing to be do? What are your thoughts on where to go from here? |
vanitabarrett
changed the title
|
@vanitabarrett @NickColley Should we iterate collections to not use the title component? |
|
@fofr @NickColley The collections title does feel different to the static title - perhaps in the future it should really be an app-specific component in itself (if other apps move over to our component pattern). For now we could remove the govuk-title so it doesn't rely on the static styling, and add the styling it needs into collections? |
|
I've not closely looked but the title component should be broadly applicable, seems like the main problem is spacing which is something we know we need to solve. |
|
@vanitabarrett I think we can avoid the headings that are not currently using the govuk-title classes and create a card to consolidate them separately. I've had a look at the remaining use cases. That text underneath the title, looks a lot like our lead paragraph component.
This means if we can move the lead paragraph component to static, and allow flexible spacing for the title component. We should be good to go? |
|
Screenshot showing the above hacked in |
|
To get this story completed more quickly, we've decided to (for now) remove the use of the govuk-title class in Collections and simply copy over the styling. This has been done in alphagov/collections#373 Have added stories to the Planning board to move over lead paragraph to static and update collection titles to use static components |
|
We need to deploy Collections before merging this. |
We need to add the old gov-uk styling for deployment so any cached HTML is still styled correctly. Will remove after deployment.
vanitabarrett
changed the title
Updating static title component to meet government-frontend component conventions.
This includes: