From b1da1974729332a7abcea81a34c39400c574ebfb Mon Sep 17 00:00:00 2001 From: Jon Kirwan Date: Mon, 11 Nov 2024 12:39:48 +0000 Subject: [PATCH 1/2] Remove important metadata component --- app/assets/stylesheets/application.scss | 5 ++ .../components/_important-metadata.scss | 52 ----------------- .../components/_important_metadata.html.erb | 21 ------- .../components/docs/important_metadata.yml | 36 ------------ .../document_collection.html.erb | 11 +++- .../content_items/fatality_notice.html.erb | 12 +++- .../specialist_document.html.erb | 11 +++- app/views/content_items/speech.html.erb | 11 +++- .../statistical_data_set.html.erb | 10 +++- .../statistics_announcement.html.erb | 31 ++++++---- config/initializers/dartsass.rb | 1 - test/components/important_metadata_test.rb | 56 ------------------- 12 files changed, 71 insertions(+), 186 deletions(-) delete mode 100644 app/assets/stylesheets/components/_important-metadata.scss delete mode 100644 app/views/components/_important_metadata.html.erb delete mode 100644 app/views/components/docs/important_metadata.yml delete mode 100644 test/components/important_metadata_test.rb diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 1b1c78da7..a5bb80016 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -54,3 +54,8 @@ $govuk-include-default-font-face: false; .constrained-image { max-width: 100%; } + +.inverse-background { + background: $govuk-brand-colour; + color: govuk-colour("white"); +} diff --git a/app/assets/stylesheets/components/_important-metadata.scss b/app/assets/stylesheets/components/_important-metadata.scss deleted file mode 100644 index 3b3bb9fcd..000000000 --- a/app/assets/stylesheets/components/_important-metadata.scss +++ /dev/null @@ -1,52 +0,0 @@ -@import "govuk_publishing_components/individual_component_support"; -@import "../mixins/margins"; - -.app-c-important-metadata { - background: $govuk-brand-colour; - color: govuk-colour("white"); - padding: govuk-spacing(3); - - @include govuk-media-query($from: tablet) { - overflow: auto; - padding: govuk-spacing(4) govuk-spacing(6); - } - - @include govuk-media-query($media-type: print) { - padding: 0; - color: $govuk-text-colour; - - .govuk-link { - color: $govuk-text-colour; - } - - .app-c-important-metadata__definition { - padding-bottom: govuk-spacing(2); - } - } -} - -// this will be moved and extended into a model for general component spacing -// once this has been decided upon and other work completed, see: -// https://trello.com/c/KEkNsxG3/142-3-implement-customisable-spacing-for-components -.app-c-important-metadata--bottom-margin { - @include responsive-bottom-margin; -} - -.app-c-important-metadata__title { - font-weight: bold; - margin-bottom: 5px; -} - -.app-c-important-metadata__list { - margin: 0; -} - -.app-c-important-metadata__term { - float: left; - padding-right: govuk-spacing(1); -} - -.app-c-important-metadata__definition { - margin: 0; - font-weight: bold; -} diff --git a/app/views/components/_important_metadata.html.erb b/app/views/components/_important_metadata.html.erb deleted file mode 100644 index e136a55e4..000000000 --- a/app/views/components/_important_metadata.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<% add_app_component_stylesheet("important-metadata") %> -<% - title = local_assigns[:title] - items = local_assigns[:items] || {} - items = items.reject { |k,v| v.nil? } - items = items.merge(items) { |k,v| Array(v).join(", ") } - margin_bottom_class = " app-c-important-metadata--bottom-margin" unless local_assigns[:margin_bottom] --%> -<% if items.any? %> -
- <% if title %> -

<%= sanitize title %>

- <% end %> -
- <% items.each do |title, definition| %> -
<%= sanitize title.to_s %>:
-
<%= sanitize definition %>
- <% end %> -
-
-<% end %> diff --git a/app/views/components/docs/important_metadata.yml b/app/views/components/docs/important_metadata.yml deleted file mode 100644 index 49f87725d..000000000 --- a/app/views/components/docs/important_metadata.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Important metadata -description: List of document-type specific metadata -body: | - A replacement for the [metadata component](https://govuk-static.herokuapp.com/component-guide/metadata) with only the format specific details. - - Part of the universal navigation design. - -shared_accessibility_criteria: -- link - -accessibility_criteria: | - Important metadata must: - - - have a text contrast ratio higher than 4.5:1 against the background colour to meet [WCAG AA](https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast) - -examples: - default: - data: - items: - Date of occurrence: 29 November 2013 - Aircraft category: General Aviation - Report type: Formal Report - with_multiple_items_per_thing: - data: - items: - Many things: - - First thing - - Second thing - - Third thing - with_title: - description: Used on statistics announcements to display release date changed information, [see example](https://gov.uk/government/statistics/announcements/museums-and-galleries-monthly-visits--8) - data: - title: "The release date has been changed" - items: - Previous Date: 4 February 2016 9:00pm - Reason for change: Incorrectly input as 2015 instead of 2016 diff --git a/app/views/content_items/document_collection.html.erb b/app/views/content_items/document_collection.html.erb index 3fa950e6f..519f79e9c 100644 --- a/app/views/content_items/document_collection.html.erb +++ b/app/views/content_items/document_collection.html.erb @@ -29,8 +29,15 @@
- <%= render 'components/important_metadata', - items: @content_item.important_metadata %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> <%= render "components/contents_list_with_body", contents: @content_item.contents do %>
diff --git a/app/views/content_items/fatality_notice.html.erb b/app/views/content_items/fatality_notice.html.erb index 51f32a14d..52b86ab46 100644 --- a/app/views/content_items/fatality_notice.html.erb +++ b/app/views/content_items/fatality_notice.html.erb @@ -22,8 +22,16 @@
- <%= render 'components/important_metadata', - items: @content_item.important_metadata %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> + <%= render 'components/figure', src: @content_item.image["url"], alt: @content_item.image["alt_text"], diff --git a/app/views/content_items/specialist_document.html.erb b/app/views/content_items/specialist_document.html.erb index b07f589be..8a55cf0eb 100644 --- a/app/views/content_items/specialist_document.html.erb +++ b/app/views/content_items/specialist_document.html.erb @@ -21,8 +21,15 @@
- <%= render 'components/important_metadata', - items: @content_item.important_metadata %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> <%= render "components/contents_list_with_body", contents: @content_item.contents do %> diff --git a/app/views/content_items/speech.html.erb b/app/views/content_items/speech.html.erb index 875f0cd9a..ed4000738 100644 --- a/app/views/content_items/speech.html.erb +++ b/app/views/content_items/speech.html.erb @@ -24,8 +24,15 @@
- <%= render 'components/important_metadata', - items: @content_item.important_metadata %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> <%= render 'components/figure', src: @content_item.image["url"], diff --git a/app/views/content_items/statistical_data_set.html.erb b/app/views/content_items/statistical_data_set.html.erb index f22b390ca..52ba2b145 100644 --- a/app/views/content_items/statistical_data_set.html.erb +++ b/app/views/content_items/statistical_data_set.html.erb @@ -24,7 +24,15 @@ <%= render 'shared/publisher_metadata_with_logo' %>
- <%= render 'components/important_metadata', items: @content_item.important_metadata %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> <%= render "components/contents_list_with_body", contents: @content_item.contents do %>
diff --git a/app/views/content_items/statistics_announcement.html.erb b/app/views/content_items/statistics_announcement.html.erb index 703bfa711..040350a7f 100644 --- a/app/views/content_items/statistics_announcement.html.erb +++ b/app/views/content_items/statistics_announcement.html.erb @@ -15,20 +15,29 @@
- <%= render "components/important_metadata", - items: @content_item.important_metadata, - margin_bottom: true %> + <% if @content_item.important_metadata.any? %> + <%= content_tag :div, class: "inverse-background" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: @content_item.important_metadata, + margin_bottom: 0, + } %> + <% end %> + <% end %> <% if @content_item.release_date_changed? %>
- <%= render "components/important_metadata", - title: t("statistics_announcement.changed_date"), - items: { - t("statistics_announcement.previous_date") => @content_item.previous_release_date, - t("statistics_announcement.reason_for_change") => @content_item.release_date_change_reason, - }, - margin_bottom: true - %> + <%= content_tag :div, class: "inverse-background" do %> + <%= render "govuk_publishing_components/components/metadata", { + inverse: true, + other: { + t("statistics_announcement.previous_date") => @content_item.previous_release_date, + t("statistics_announcement.reason_for_change") => @content_item.release_date_change_reason, + }, + margin_bottom: 0, + title: t("statistics_announcement.changed_date"), + } %> + <% end %>
<% end %> diff --git a/config/initializers/dartsass.rb b/config/initializers/dartsass.rb index e2cadc570..7611ffd9b 100644 --- a/config/initializers/dartsass.rb +++ b/config/initializers/dartsass.rb @@ -5,7 +5,6 @@ "components/_contents-list-with-body.scss" => "components/_contents-list-with-body.css", "components/_download-link.scss" => "components/_download-link.css", "components/_figure.scss" => "components/_figure.css", - "components/_important-metadata.scss" => "components/_important-metadata.css", "components/_published-dates.scss" => "components/_published-dates.css", "views/_guide.scss" => "views/_guide.css", "views/_html-publication.scss" => "views/_html-publication.css", diff --git a/test/components/important_metadata_test.rb b/test/components/important_metadata_test.rb deleted file mode 100644 index 660c5d989..000000000 --- a/test/components/important_metadata_test.rb +++ /dev/null @@ -1,56 +0,0 @@ -require "component_test_helper" - -class ImportantMetadataTest < ComponentTestCase - def component_name - "important_metadata" - end - - test "does not render metadata when no data is given" do - assert_empty render_component({}) - end - - test "does not render when an 'other' object is provided without any values" do - assert_empty render_component(other: { From: [] }) - assert_empty render_component(other: { a: false, b: "", c: [], d: {}, e: nil }) - end - - test "renders a title when a title is provided" do - render_component( - title: "The release date has been changed", - items: { - "Release Date": "14 October 2016", - }, - ) - - assert_select ".app-c-important-metadata__title", text: "The release date has been changed" - end - - test "renders metadata link pairs from data it is given" do - render_component(items: { - "Opened": "14 October 2016", - "Case type": ['Mergers'], - "Case state": ['Open'], - "Market sector": ['Motor industry'], - "Outcome": ['Mergers - phase 2 clearance with remedies'], - }) - - assert_select ".app-c-important-metadata dt", text: "Opened:" - assert_select ".app-c-important-metadata dd", text: "14 October 2016" - assert_select ".app-c-important-metadata dt", text: "Case type:" - assert_select ".app-c-important-metadata dd", text: "Mergers" - assert_select ".app-c-important-metadata dd a[href='https://www.gov.uk/cma-cases?case_type%5B%5D=mergers']", - text: "Mergers" - assert_select ".app-c-important-metadata dt", text: "Case state:" - assert_select ".app-c-important-metadata dd", text: "Open" - assert_select ".app-c-important-metadata dd a[href='https://www.gov.uk/cma-cases?case_state%5B%5D=open']", - text: "Open" - assert_select ".app-c-important-metadata dt", text: "Market sector:" - assert_select ".app-c-important-metadata dd", text: "Motor industry" - assert_select ".app-c-important-metadata dd a[href='https://www.gov.uk/cma-cases?market_sector%5B%5D=motor-industry']", - text: "Motor industry" - assert_select ".app-c-important-metadata dt", text: "Outcome:" - assert_select ".app-c-important-metadata dd", text: "Mergers - phase 2 clearance with remedies" - assert_select ".app-c-important-metadata dd a[href='https://www.gov.uk/cma-cases?outcome_type%5B%5D=mergers-phase-2-clearance-with-remedies']", - text: "Mergers - phase 2 clearance with remedies" - end -end From f602401a3fbb8506f25fc6ff691870cd08444c1c Mon Sep 17 00:00:00 2001 From: Jon Kirwan Date: Tue, 12 Nov 2024 14:32:43 +0000 Subject: [PATCH 2/2] Update tests --- app/views/content_items/document_collection.html.erb | 2 +- app/views/content_items/fatality_notice.html.erb | 2 +- app/views/content_items/specialist_document.html.erb | 2 +- app/views/content_items/speech.html.erb | 2 +- app/views/content_items/statistical_data_set.html.erb | 2 +- app/views/content_items/statistics_announcement.html.erb | 2 +- test/integration/fatality_notice_test.rb | 4 ++-- test/integration/publication_test.rb | 2 +- test/integration/specialist_document_test.rb | 2 +- test/integration/speech_test.rb | 2 +- test/integration/statistical_data_set_test.rb | 3 ++- test/integration/statistics_announcement_test.rb | 4 ++-- test/test_helper.rb | 8 ++++---- 13 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/views/content_items/document_collection.html.erb b/app/views/content_items/document_collection.html.erb index 519f79e9c..9dcd15443 100644 --- a/app/views/content_items/document_collection.html.erb +++ b/app/views/content_items/document_collection.html.erb @@ -30,7 +30,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= content_tag :div, class: "important-metadata inverse-background responsive-bottom-margin" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/app/views/content_items/fatality_notice.html.erb b/app/views/content_items/fatality_notice.html.erb index 52b86ab46..95d321bc5 100644 --- a/app/views/content_items/fatality_notice.html.erb +++ b/app/views/content_items/fatality_notice.html.erb @@ -23,7 +23,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= content_tag :div, class: "important-metadata inverse-background responsive-bottom-margin" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/app/views/content_items/specialist_document.html.erb b/app/views/content_items/specialist_document.html.erb index 8a55cf0eb..83cae722e 100644 --- a/app/views/content_items/specialist_document.html.erb +++ b/app/views/content_items/specialist_document.html.erb @@ -22,7 +22,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= content_tag :div, class: "important-metadata inverse-background responsive-bottom-margin" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/app/views/content_items/speech.html.erb b/app/views/content_items/speech.html.erb index ed4000738..998ec0dc2 100644 --- a/app/views/content_items/speech.html.erb +++ b/app/views/content_items/speech.html.erb @@ -25,7 +25,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= content_tag :div, class: "important-metadata inverse-background responsive-bottom-margin" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/app/views/content_items/statistical_data_set.html.erb b/app/views/content_items/statistical_data_set.html.erb index 52ba2b145..a18564a9e 100644 --- a/app/views/content_items/statistical_data_set.html.erb +++ b/app/views/content_items/statistical_data_set.html.erb @@ -25,7 +25,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background responsive-bottom-margin" do %> + <%= content_tag :div, class: "important-metadata inverse-background responsive-bottom-margin" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/app/views/content_items/statistics_announcement.html.erb b/app/views/content_items/statistics_announcement.html.erb index 040350a7f..1c42bee98 100644 --- a/app/views/content_items/statistics_announcement.html.erb +++ b/app/views/content_items/statistics_announcement.html.erb @@ -16,7 +16,7 @@
<% if @content_item.important_metadata.any? %> - <%= content_tag :div, class: "inverse-background" do %> + <%= content_tag :div, class: "important-metadata inverse-background" do %> <%= render "govuk_publishing_components/components/metadata", { inverse: true, other: @content_item.important_metadata, diff --git a/test/integration/fatality_notice_test.rb b/test/integration/fatality_notice_test.rb index 0a8e113e7..5bcec6ac2 100644 --- a/test/integration/fatality_notice_test.rb +++ b/test/integration/fatality_notice_test.rb @@ -24,7 +24,7 @@ class FatalityNoticeTest < ActionDispatch::IntegrationTest from: { "Ministry of Defence": "/government/organisations/ministry-of-defence", }, - }) + }, context_selector: ".metadata-column") assert_has_important_metadata( "Field of operation": { "Zululand": "/government/fields-of-operation/zululand" }, @@ -60,7 +60,7 @@ class FatalityNoticeTest < ActionDispatch::IntegrationTest assert_has_metadata({ from: { "Ministry of Defence": "/government/organisations/ministry-of-defence", "The Rt Hon Sir Eric Pickles MP": "/government/people/eric-pickles", - } }) + } }, context_selector: ".metadata-column") end test "fatality notice with withdrawn notice" do diff --git a/test/integration/publication_test.rb b/test/integration/publication_test.rb index cb8cfa824..f67875101 100644 --- a/test/integration/publication_test.rb +++ b/test/integration/publication_test.rb @@ -25,7 +25,7 @@ class PublicationTest < ActionDispatch::IntegrationTest "Environment Agency": "/government/organisations/environment-agency", "The Rt Hon Sir Eric Pickles MP": "/government/people/eric-pickles", }, - }) + }, context_selector: ".metadata-column") assert_has_structured_data(page, "Article") diff --git a/test/integration/specialist_document_test.rb b/test/integration/specialist_document_test.rb index fd129e0db..1acca926b 100644 --- a/test/integration/specialist_document_test.rb +++ b/test/integration/specialist_document_test.rb @@ -24,7 +24,7 @@ class SpecialistDocumentTest < ActionDispatch::IntegrationTest "Air Accidents Investigation Branch": "/government/organisations/air-accidents-investigation-branch", }, - }) + }, context_selector: ".metadata-column") end test "renders published and updated in metadata" do diff --git a/test/integration/speech_test.rb b/test/integration/speech_test.rb index 1fd446959..545ddd261 100644 --- a/test/integration/speech_test.rb +++ b/test/integration/speech_test.rb @@ -27,7 +27,7 @@ class SpeechTest < ActionDispatch::IntegrationTest "The Rt Hon Andrea Leadsom MP": "/government/people/andrea-leadsom", }, published: "8 March 2016", - }) + }, context_selector: ".metadata-column") assert_has_important_metadata( "Delivered on": diff --git a/test/integration/statistical_data_set_test.rb b/test/integration/statistical_data_set_test.rb index 9c97372f8..6744c4509 100644 --- a/test/integration/statistical_data_set_test.rb +++ b/test/integration/statistical_data_set_test.rb @@ -15,7 +15,8 @@ class StatisticalDataSetTest < ActionDispatch::IntegrationTest assert_has_metadata({ published: "13 December 2012", from: { "Department for Transport": "/government/organisations/department-for-transport" }, - }) + }, context_selector: ".metadata-column") + assert_footer_has_published_dates("Published 13 December 2012") end diff --git a/test/integration/statistics_announcement_test.rb b/test/integration/statistics_announcement_test.rb index f240444ea..10182b502 100644 --- a/test/integration/statistics_announcement_test.rb +++ b/test/integration/statistics_announcement_test.rb @@ -17,7 +17,7 @@ class StatisticsAnnouncementTest < ActionDispatch::IntegrationTest assert page.has_text?(@content_item["description"]) assert page.has_css?('img[alt="Accredited official statistics"]') - within ".app-c-important-metadata" do + within ".important-metadata .gem-c-metadata" do assert page.has_text?(:all, "Release date: January 2016 (provisional)") end end @@ -45,7 +45,7 @@ class StatisticsAnnouncementTest < ActionDispatch::IntegrationTest assert page.has_text?(@content_item["description"]) assert page.has_text?(:all, "Release date: 20 January 2016 9:30am (confirmed)") - within ".release-date-changed .app-c-important-metadata" do + within ".release-date-changed .gem-c-metadata" do assert page.has_text?("The release date has been changed") assert page.has_text?("Previous date") assert page.has_text?("19 January 2016 9:30am") diff --git a/test/test_helper.rb b/test/test_helper.rb index a54c7c71f..f036dc4d0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -105,8 +105,8 @@ def assert_has_publisher_metadata_other(any_args) assert_has_metadata(any_args) end - def assert_has_metadata(any_args, extra_metadata_classes: nil) - within ".gem-c-metadata#{extra_metadata_classes}" do + def assert_has_metadata(any_args, context_selector: nil, extra_metadata_classes: nil) + within "#{context_selector} .gem-c-metadata#{extra_metadata_classes}" do any_args.each_value do |value| value = { value => nil } if value.is_a?(String) value.each do |text, href| @@ -138,9 +138,9 @@ def assert_has_metadata_local(metadata, term_selector, definition_selector) end def assert_has_important_metadata(metadata) - within(".app-c-important-metadata") do + within(".important-metadata .gem-c-metadata") do assert_has_metadata_local( - metadata, ".app-c-important-metadata__term", ".app-c-important-metadata__definition" + metadata, ".gem-c-metadata__term", ".gem-c-metadata__definition" ) end end