Add callouts to Govspeak component; Remove information-block #899

36degrees · 2017-02-10T10:12:15Z

Background

We would like to be able to use callouts in the Service Manual. The CSS for the callouts is currently only defined in helpers/_text.scss, which is not included when using the core_layout template in static.

This adds styling for the various Govspeak callouts to the Govspeak component, which is included in core_layout.

As part of this work, I have also removed the information-block class – after a fair amount of digging we can't find anywhere this is used. See the commit message for more details.

Design

I worked with Mia on the template consolidation team, as she's been doing some work to rationalise the different Govspeak callouts we have.

The image with the blue m’s shows the spacing (in ems!) that I am proposing.
The address callout uses a box the same width as the other callout borders, with a grey left border (the box and the other borders are white or transparent).

I have not touched the address element, as it's a fairly complicated chunk of CSS and I think it's distinct enough to be out of scope for this work.

Result

The template consolidation team have been working on Govspeak and have produced a rationalised design for the various Govspeak callouts. This modifies the ‘call to action’ callout to use 2em padding. It also removes a redundant declaration - ordered lists already have a list-style-position of outside as set in the govspeak typography file.

The template consolidation team have been working on Govspeak and have produced a rationalised design for the various Govspeak callouts. This modifies the border width and padding, and removes the bottom margin from the last paragraph or list.

The information-block class was originally added when call-to-action was added to whitehall, and a specialist guide was refactored so that `call-to-action` and `information-block` shared some common CSS[1]. The last reference to the information-block within templates in whitehall was removed when 'detailed guides' were moved to government-frontend[2]. `information-block` itself does not appear to have ever been used in HTML generated by the Govspeak engines in whitehall nor in the standalone gem. Searching the commit history for whitehall returns only mentions of “information-block” within templates, CSS and tests, and searching the history for alphagov/govspeak returns nothing. At time of commiting, according to GitHub’s search, there are only 11 files mentioning information-block across the entire alphagov organisation. Ten of these are CSS files, the other is the govspeak.yml file in this repo.[3] Lastly, we grepped the mirrors for mentions of information-block and found only three results: ``` ./guidance/animal-welfare-in-severe-weather.html: <aside class="related-mainstream-content information-block" role="complementary"> ./guidance/disclosure-and-barring-service-criminal-record-checks-referrals-and-complaints.html: <aside class="related-mainstream-content information-block" role="complementary"> ./guidance/pet-travel-quarantine.html: <aside class="related-mainstream-content information-block" role="complementary"> ``` However these all appear to be outdated as the class is not present when viewing the corresponding webpages in production. [1]: alphagov/whitehall@d658664 [2]: alphagov/whitehall@08b9056 [2]: https://github.com/search?q=org%3Aalphagov+information-block&type=Code

For now, use 2em to reflect the padding used. This will be revisited when the type styles are rationalised, as that will affect the overall vertical rhythm.

dsingleton

I'm not going to mark as approved, someone from GOV.UK Publishing should have last day, but this looks good to me (pending one query).

And 👏 to 943d1e1, that's a really well written commit, and this branch is really readable in general.

dsingleton · 2017-02-10T10:37:19Z

app/views/govuk_component/docs/govspeak.yml

@@ -121,15 +121,12 @@ fixtures:
        <p>My quote</p>
        <p class="last-child">about things</p>
      </blockquote>
-  call_out_blocks:
+  information_block:


As .information-block was removed in 943d1e1, can the fixture be removed now?

fofr · 2017-02-10T10:44:57Z

app/assets/stylesheets/govuk-component/govspeak/_call-to-action.scss

+
+.govuk-govspeak {
+  .call-to-action {
+    margin: 2em 0;


Do left and right margins need to be set to 0?

I guess not as that should be the default… this is a bit more defensive I guess. And I think I just prefer it to setting margin-top, margin-bottom and leaving the rest to chance?

fofr · 2017-02-10T10:46:19Z

app/assets/stylesheets/govuk-component/govspeak/_information-callout.scss

+    margin: 2em 0;
+
+    // Remove margin from the last element
+    p:last-child,


Feels like this could be a mixin and shared between the callouts.

Yeah, I was debating it. I kinda feel it's in few enough places that it doesn't warrant hiding things behind abstractions, but I could be persuaded.

Does this need to be qualified? Would :last-child {} work?

Good question. Probably not? It's hard to know exactly what elements are going to be used inside the callout and so what this could affect, but at the same time that's also a good argument for making it a generic :last-child…

@36degrees When I use comments to explain something it usually makes sense to refactor into a named function:

eg // Remove margin from the last element
becomes @include remove-last-element-margin;

I prefer the selector to be qualified. It's faster and less risky (eg otherwise it would apply to all of the last list items in a list).

If we use a mixin it's easy to add another case if found.

36degrees · 2017-02-10T10:46:29Z

I've messed around with the service manual frontend to get it to render the Govspeak styles using the 'old' wrapper layout, so you can see what it would look like:

Old	New

Note that there are no styles for call-to-action in helpers/text – that's something that only existed in the Govspeak component.

Using 'Call to Action' ($CTA) also isn't documented in the GovSpeak guide.

For that matter, neither are advisories (@) – another present in the Govspeak gem which has no styles in this component and is not documented.

NickColley

This looks great, just a few comments.

NickColley · 2017-02-10T10:49:11Z

app/assets/stylesheets/govuk-component/govspeak/_information-callout.scss

+    margin: 2em 0;
+
+    // Remove margin from the last element
+    p:last-child,


Does this need to be qualified? Would :last-child {} work?

NickColley · 2017-02-10T10:52:03Z

app/assets/stylesheets/govuk-component/govspeak/_example.scss

-    padding-left: 1.5em;
-    margin: 2.5em 0;
+    border-left: 1em solid $panel-colour;
+    padding: 1em 0 1em 1em;


I'm 👍 for using em in general but is this consistent with our spacing as defined here: https://govuk-elements.herokuapp.com/layout/?

spacing between elements is usually 5px, 10px, 15px or multiples of 15px

I'd prefer we redefine our spacing system to use em rather than be inconsistent.

^ @miaallers thoughts?

I'd prefer we redefine our spacing system to use em rather than be inconsistent.

Is this a recommendation for GOV.UK Publishing, or to all services?

I'd prefer we redefine our spacing system to use em rather than be inconsistent.

That's a big change.

For now, using ems in this case improves legibility and makes spacing consistent with other content.

We're using the grid classes to define vertical margins on paragraphs etc though:
https://github.com/alphagov/static/blob/master/app/assets/stylesheets/govuk-component/govspeak/_typography.scss#L16-L17

We could move to using multiples of $gutter-half (it’s essentially a difference of 15px vs a computed 16px).

It'd be good to better understand Mia's intentions. I'm aware Mia's out of the office today, so let's hold merging this till Monday, and discuss it then.

I thought we were using those spacing rules but it seems like we're not so I'm 👍 to merge, we can have a conversation about consistency another time.

fofr

I'm pleased to see this, it's a feature Template Consolidation would have needed to add to render mainstream content using the component.

Good PR, good commits 💯

NickColley · 2017-02-10T11:42:17Z

Thanks @36degrees and @miaallers, this is top 👍

Information blocks were removed in 943d1e1 See: #899

This feature was removed in #899. https://trello.com/c/ZMVwIyj5

36degrees added 8 commits February 10, 2017 08:13

Split information-block and call-to-action

76ae1a7

Apply rationalised design to ‘example’

77b0bac

The template consolidation team have been working on Govspeak and have produced a rationalised design for the various Govspeak callouts. This modifies the border width and padding, and removes the bottom margin from the last paragraph or list.

Add information callout

165a145

Add Govspeak warning callout

69bf6da

Use a consistent margin for callouts

e3f237c

For now, use 2em to reflect the padding used. This will be revisited when the type styles are rationalised, as that will affect the overall vertical rhythm.

Fix linting errors

c2b7820

dsingleton reviewed Feb 10, 2017

View reviewed changes

Remove information_block fixture

63cc97e

fofr reviewed Feb 10, 2017

View reviewed changes

NickColley approved these changes Feb 10, 2017

View reviewed changes

fofr approved these changes Feb 10, 2017

View reviewed changes

NickColley merged commit f87ba63 into master Feb 10, 2017

NickColley deleted the add-callouts branch February 10, 2017 11:41

fofr added a commit that referenced this pull request Mar 6, 2017

Remove information-block reference

d474d1f

Information blocks were removed in 943d1e1 See: #899

tijmenb added a commit that referenced this pull request May 18, 2018

Remove unused information-block image

c4c9d14

This feature was removed in #899. https://trello.com/c/ZMVwIyj5

tijmenb mentioned this pull request May 18, 2018

Remove unused information-block image #1415

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add callouts to Govspeak component; Remove information-block #899

Add callouts to Govspeak component; Remove information-block #899

36degrees commented Feb 10, 2017 •

edited

Loading

dsingleton left a comment

dsingleton Feb 10, 2017

fofr Feb 10, 2017

36degrees Feb 10, 2017

fofr Feb 10, 2017

36degrees Feb 10, 2017

NickColley Feb 10, 2017

36degrees Feb 10, 2017

fofr Feb 10, 2017

36degrees commented Feb 10, 2017 •

edited by dsingleton

Loading

NickColley left a comment

NickColley Feb 10, 2017

NickColley Feb 10, 2017

36degrees Feb 10, 2017

dsingleton Feb 10, 2017

fofr Feb 10, 2017 •

edited

Loading

36degrees Feb 10, 2017

NickColley Feb 10, 2017

fofr left a comment

NickColley commented Feb 10, 2017

Add callouts to Govspeak component; Remove information-block #899

Add callouts to Govspeak component; Remove information-block #899

Conversation

36degrees commented Feb 10, 2017 • edited Loading

Background

Design

Result

dsingleton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

36degrees commented Feb 10, 2017 • edited by dsingleton Loading

NickColley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fofr Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fofr left a comment

Choose a reason for hiding this comment

NickColley commented Feb 10, 2017

36degrees commented Feb 10, 2017 •

edited

Loading

36degrees commented Feb 10, 2017 •

edited by dsingleton

Loading

fofr Feb 10, 2017 •

edited

Loading