Update design for "how government works" #6283

jon-kirwan · 2021-09-15T11:31:04Z

What

https://trello.com/c/vgUN7cPF/631-update-how-government-works-template

Page needs to be updated to use modern layout classes

Why

Consistent look with other pages
Use design system styles and component guide components
Fix document outline issues

Visual changes

Before	After

chris-gds · 2021-09-16T10:18:56Z

app/views/home/how_government_works.html.erb

+<div class="govuk-width-container">
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+      <header>


There is probably debate to be had about the pros / cons of multiple <header>s on the page - however, following convention of other pages + as there's the id="global-header" on the page - I don't think this is needed.

chris-gds · 2021-09-16T10:41:33Z

app/views/home/how_government_works.html.erb

      </div>

-      <div class="full-width-section ministers">
+      <section>


I'm glad you used this. I'm not sure we have consistent, recommended agreement on <section> use.

The Design System layout doesn't mention it however it is used here but in a different way. I've raised a "?" about this previously as I'm a bit unclear.

My stance at the moment is that we should be using it as it benefits some users but how we use it I think we need to document a little.

I would imagine it's OK to use in layout like this but the use above is slightly different and I'm not 100% it's supported. Trying to find out.

<section class="govuk-grid-row"> <div class="govuk-grid-column-two-thirds"> <h1 class="govuk-heading-xl">Two-thirds column</h1> </div> </section>

Thanks, will have a look around to see if I can find anything helpful to add to the conversation too

I'd tend towards divs until we have a proper strategy around when to use section, but it's probably not worth changing at this point unless it seems to cause any issues.

chris-gds · 2021-09-16T10:53:17Z

app/views/home/how_government_works.html.erb

+<div class="govuk-width-container">
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+      <header>
        <%= render "govuk_publishing_components/components/title", {
          title: "How government works",
        } %>
        <p class="govuk-body-l">In the UK, the Prime Minister leads the government with the support of the Cabinet and ministers. You can find out <%= link_to("who runs government", "#who-runs-government", class: "govuk-link") %> and <%= link_to("how government is run", "#how-government-is-run", class: "govuk-link") %>, as well as learning about the <%= link_to("history of government", "#history-uk-government", class: "govuk-link") %>.</p>


This could be swapped for a leading paragraph Component (It might need to be extended to allow links)

Looks like you can use a sanitize function with escaped quotes to include html in the text property.

chris-gds · 2021-09-16T10:55:51Z

app/views/home/how_government_works.html.erb

+         <ul class="govuk-list">
+          <li class="feature-value">
+            <a href="<%= organisations_path(anchor: 'departments') %>" class="feature-value__link">
+              <span class="feature-value__count"><%= @ministerial_department_count %></span>


Ah this whole part can be swapped for the newly created Component called "big numbers"

Be careful doing this - we previously had to implement code to prevent it overlapping other stuff when users either zoomed in or increased their font size (I don't remember the exact scenario). Test thoroughly for this.

nods - PR Ref FYI

Okay i've now updated the page to use the big number component. This was a little tricky due to the font size of the number in the component which has meant design changes following a review with Nik.

Zoomed in this looks fine and I can't see any overflow problems on this page.

Attached a couple of screenshots.

Infographic 1

Infographic 2

chris-gds · 2021-09-16T10:58:22Z

app/views/home/how_government_works.html.erb

-              heading_level: 3,
-              margin_bottom: 5,
-            } %>
+        <ul class="govuk-list govuk-list--bullet govuk-list--spaced">


This looks like it could be swapped to use List Component

chris-gds · 2021-09-16T10:58:44Z

app/views/home/how_government_works.html.erb

-              heading_level: 3,
-              margin_bottom: 5,
-            } %>
+        <ul class="govuk-list govuk-list--bullet govuk-list--spaced">


Same here with the list

chris-gds · 2021-09-16T11:03:33Z

app/views/home/how_government_works.html.erb

+        <p class="govuk-body">Read about past Prime Ministers, Chancellors and Foreign Secretaries in <%= link_to "notable people", "/government/history#notable-people", class: "govuk-link" %>. Learn more about historic <%= link_to "government buildings", "/government/history#government-buildings", class: "govuk-link" %> on Whitehall and around the UK.</p>
+        <p class="govuk-body govuk-!-margin-bottom-8">You can also find links to <%= link_to "historical research", "/government/history#historical-research", class: "govuk-link" %>, documents and records.</p>
+
+        <%= image_tag 'how-gov-works/Churchill_01.jpg', alt: 'History of government' %>


In the context of this page, I'm not sure about image use in general. However, keeping this in scope of this PR - I think the alt of this needs to change to convey the meaning of this image. I would go with something like "Winston Churchill" or even "Sir Winston Leonard Spencer Churchill"

I'd say in this case it's entirely decorative and not conveying any information whatsoever, so set the alt text to an empty string.

TL;DR - on review I agree. Let's remove alt for all these images on this page.

Provided empty alt text for the following images -

<%= image_tag @prime_minister.image_url(:s465), { alt: '' } %> <%= image_tag 'how-gov-works/10_Downing_Street.jpg', alt: '' %> <%= image_tag 'how-gov-works/Cabinet_01.jpg', alt: '' %> <%= image_tag 'how-gov-works/Churchill_01.jpg', alt: '' %>

chris-gds · 2021-09-16T11:03:57Z

app/views/home/how_government_works.html.erb

-        <div class="one-half">
-          <div class="inner-block">
-            <p class="govuk-body">Britain has one of the oldest governments in the world. Find out more about how it has worked and who has shaped it in the <%= link_to "history", "/government/history/", class: "govuk-link" %> section.</p>
+        <ul class="govuk-list govuk-list--bullet govuk-list--spaced">


Another list

chris-gds · 2021-09-16T11:12:44Z

app/views/home/how_government_works.html.erb

+          <p class="govuk-body">The Cabinet is made up of the senior members of government. Every week during Parliament, members of the Cabinet (Secretaries of State from all departments and some other ministers) meet to discuss the most important issues for the government.</p>
+          <p class="govuk-body govuk-!-margin-bottom-8"><%= link_to("See who is in the Cabinet", "/government/ministers", class: "govuk-link") %></p>
+
+          <%= image_tag 'how-gov-works/Cabinet_01.jpg', alt: 'The Cabinet' %>


I don't think this Content is apt, something like "The Cabinet Room at No. 10 Downing Street Cabinet table" - I think is more descriptive of what this image is.

Provided empty) alt text for the following images -

<%= image_tag @prime_minister.image_url(:s465), { alt: '' } %> <%= image_tag 'how-gov-works/10_Downing_Street.jpg', alt: '' %> <%= image_tag 'how-gov-works/Cabinet_01.jpg', alt: '' %> <%= image_tag 'how-gov-works/Churchill_01.jpg', alt: '' %>

maxgds · 2021-09-22T07:10:08Z

app/views/home/how_government_works.html.erb

+          <% if @prime_minister.present? && @prime_minister.image_url(:s465).present? %>
+            <%= image_tag @prime_minister.image_url(:s465), { alt: '', class: 'govuk-!-width-full', width: '465', height: '310' } %>
+          <% else %>
+            <%= image_tag 'how-gov-works/10_Downing_Street.jpg', alt: '', class: 'govuk-!-width-full', width: '465', height: '310' %>


These dimensions need attention - I've pushed your branch onto integration to preview it and the images are being stretched too wide on desktop and pushed too narrow on mobile. They seem ok at tablet size. I'm not sure what the interaction causing that is but it definitely needs a fix before we can merge.

Yep I see the issue - I've made a few changes to remove the govuk-!-width-full class and have replaced with some fluid image styles -

img { max-width: 100%; height: auto; }

which combined with the dimensions seem to work okay now. i've updated the screenshot in the description and also attached a mobile one here

maxgds · 2021-09-22T07:11:04Z

app/views/home/how_government_works.html.erb

      </div>

-      <div class="full-width-section ministers">
+      <section>


I'd tend towards divs until we have a proper strategy around when to use section, but it's probably not worth changing at this point unless it seems to cause any issues.

maxgds · 2021-09-22T07:16:08Z

test/functional/home_controller_test.rb

@@ -18,14 +18,14 @@ class HomeControllerTest < ActionController::TestCase
  view_test "how government works shows the current prime minister" do
    get :how_government_works

-    assert_select ".prime-minister p a", "Firstname Lastname"
+    assert_select ".prime-minister > p:nth-child(5) [href]", "Firstname Lastname"


Not worth changing for this PR but a thought for future: this is not a particularly robust test. If we start adding extra children or removing some of the current content then that target of 5th child becomes invalid and the test needs an update. Probably better to tie things together more with classes generally, I'd have gone for BEM classes across the prime minister metadata myself. At that point you can both style and test the content more directly.

Thanks Max - good point. I'll bear that in mind for future.

maxgds · 2021-09-22T10:36:28Z

I think this is looking ok now - if you can kill the temporary commit and rebase against master I'll re-review.

jon-kirwan · 2021-09-22T11:41:20Z

I think this is looking ok now - if you can kill the temporary commit and rebase against master I'll re-review.

Sorry one final unexpected thing following the rebase is that one of the tests failed.

expected not to find visible css ".feature-ministers", found 1 match: "0   Ministerial departments\n0   Non-ministerial departments\n300+   Agencies other public bodies" (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/sitewide_settings_steps.rb:24:in `/^I should not see the minister counts$/'
./features/support/admin_taxonomy_helper.rb:31:in `block in <top (required)>'
features/sitewide_settings.feature:6:in `I should not see the minister counts'

This is because I deleted the .featured-ministers selector (it's no longer used for any style purpose but is used by the test). I've now added this back in to the reshuffle / ministers section which has fixed the issue... but just wanted to mention it.

jon-kirwan force-pushed the Update-how-government-works-template branch 6 times, most recently from 9151c72 to 15f8228 Compare September 15, 2021 19:39

jon-kirwan requested review from maxgds and chris-gds September 16, 2021 07:33

chris-gds reviewed Sep 16, 2021

View reviewed changes

jon-kirwan force-pushed the Update-how-government-works-template branch 2 times, most recently from ffae15d to e59845b Compare September 21, 2021 16:15

jon-kirwan changed the title ~~Update Design - How Government Works~~ Update design for "How government works" Sep 21, 2021

jon-kirwan force-pushed the Update-how-government-works-template branch 2 times, most recently from b17cf13 to 86e9b57 Compare September 21, 2021 16:52

jon-kirwan changed the title ~~Update design for "How government works"~~ Update design for "how government works" Sep 21, 2021

maxgds requested changes Sep 22, 2021

View reviewed changes

jon-kirwan force-pushed the Update-how-government-works-template branch from 86e9b57 to 218e969 Compare September 22, 2021 07:21

jon-kirwan force-pushed the Update-how-government-works-template branch 2 times, most recently from 823146e to 590adbd Compare September 22, 2021 11:00

Update design for "how government works"

c366163

jon-kirwan force-pushed the Update-how-government-works-template branch from 590adbd to c366163 Compare September 22, 2021 11:17

maxgds approved these changes Sep 22, 2021

View reviewed changes

maxgds merged commit 708eba2 into main Sep 22, 2021

maxgds deleted the Update-how-government-works-template branch September 22, 2021 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update design for "how government works" #6283

Update design for "how government works" #6283

jon-kirwan commented Sep 15, 2021 •

edited

Loading

chris-gds Sep 16, 2021

chris-gds Sep 16, 2021

jon-kirwan Sep 17, 2021

maxgds Sep 22, 2021

chris-gds Sep 16, 2021 •

edited

Loading

jon-kirwan Sep 17, 2021

chris-gds Sep 16, 2021 •

edited

Loading

maxgds Sep 16, 2021

chris-gds Sep 16, 2021

jon-kirwan Sep 21, 2021

chris-gds Sep 16, 2021

chris-gds Sep 16, 2021

chris-gds Sep 16, 2021

maxgds Sep 16, 2021

chris-gds Sep 16, 2021

jon-kirwan Sep 17, 2021 •

edited

Loading

chris-gds Sep 16, 2021

chris-gds Sep 16, 2021 •

edited

Loading

jon-kirwan Sep 17, 2021

maxgds Sep 22, 2021

jon-kirwan Sep 22, 2021

maxgds Sep 22, 2021

maxgds Sep 22, 2021

jon-kirwan Sep 22, 2021

maxgds commented Sep 22, 2021

jon-kirwan commented Sep 22, 2021

Update design for "how government works" #6283

Update design for "how government works" #6283

Conversation

jon-kirwan commented Sep 15, 2021 • edited Loading

What

Why

Visual changes

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

chris-gds Sep 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-gds Sep 16, 2021 • edited Loading

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

jon-kirwan Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-gds Sep 16, 2021 • edited Loading

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

maxgds commented Sep 22, 2021

jon-kirwan commented Sep 22, 2021

jon-kirwan commented Sep 15, 2021 •

edited

Loading

chris-gds Sep 16, 2021 •

edited

Loading

chris-gds Sep 16, 2021 •

edited

Loading

jon-kirwan Sep 17, 2021 •

edited

Loading

chris-gds Sep 16, 2021 •

edited

Loading