Suppress subscription components heading #804
Conversation
aliuk2012
force-pushed
the
suppress-subscription-heading
branch
from
ad02897
to
ead4098
Compare
|
I plan to do a release once this has been approved & merged |
For some unknown reason this component includes a h2 heading by default. This means that if a service uses it unexpectly it will include the heading. It also stops a team from using this component anywhere else on a page. By using the 'hide_heading' option, it will suppress the heading when the component is being rendered.
aliuk2012
force-pushed
the
suppress-subscription-heading
branch
from
ead4098
to
3fa4132
Compare
| @@ -11,10 +11,13 @@ | |||
| css_classes << (shared_helper.get_margin_bottom) unless local_assigns[:margin_bottom] == 0 | |||
| css_classes << brand_helper.brand_class | |||
| data = {"module": "gem-toggle"} if sl_helper.feed_link_box_value | |||
| hide_heading ||= false | |||
There was a problem hiding this comment.
I was wondering if we could turn this into a positive flag show_heading ||= true which can be turned off. It's a matter of flavour, but I always find confusing setting things to true to disable something. Especially because we don't know why we set this <h2> implicitly on this component and we may consider dropping it at some point.
There was a problem hiding this comment.
I've tried using positive flags before but it doesn't work with the way we set the defaults using ||= operator.
TBH, I would have liked to have simply remove the h2 but I just don't have time to update and test the other repos.
There was a problem hiding this comment.
It could be a positive flag, but we'd have to go and change every existing use of the component.
There was a problem hiding this comment.
@andysellick: I don't think so if we're setting the default to true.
I'm not very opinionated, whatever does the job 👍
There was a problem hiding this comment.
@alex-ju that's the problem Al was talking about. If you set the default to true, like this: show_heading ||= true when you pass through 'false' it defaults to true.
There was a problem hiding this comment.
yep, familiar with this situation, but we dealt with it in other components with a conditional statement.
|
@andysellick Good idea, I've added a some copy. I'll merge it now. |
|
@aliuk2012 would be great if we can get #805 in the same release |
For some unknown reason this component includes a h2 heading by default.
This means that if a service uses it unexpectedly it will include the heading.
It also stops a team from using this component anywhere else on a page.
By using the 'hide_heading' option, it will suppress the heading when
the component is being rendered.
Component: https://govuk-publishing-compon-pr-804.herokuapp.com/component-guide/subscription-links
Demo: https://govuk-publishing-compon-pr-804.herokuapp.com/component-guide/subscription-links/without_heading
Ticket: https://trello.com/c/nMB9EUx0/581-update-top-section-of-results-layout