Merge pull request #706 from alphagov/move-detailed-guides-to-new-layout

Move detailed guides to new layout
alphagov · Jan 29, 2018 · 0886f79 · 0886f79
2 parents c8e1c12 + 97c5a5a
commit 0886f79
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 155 deletions.
diff --git a/app/assets/stylesheets/views/_detailed-guide.scss b/app/assets/stylesheets/views/_detailed-guide.scss
@@ -7,19 +7,6 @@
     text-align: start;
   }
 
-  .related-mainstream-content {
-    background: $panel-colour;
-    padding: $gutter-half;
-    margin-bottom: $gutter;
-
-    @include media(desktop) {
-      width: percentage(1 / 3);
-      float: right;
-      margin-bottom: $gutter-half;
-      margin-left: $gutter-half;
-    }
-  }
-
   // To allow .back-to-content to settle in at the bottom after scrolling past.
   .sidebar-with-body {
     position: relative;
@@ -29,8 +16,8 @@
     max-width: 70%;
 
     @include media(tablet) {
-      max-width: 100%;
       margin-top: 45px;
+      max-width: 100%;
     }
   }
 }
diff --git a/app/presenters/detailed_guide_presenter.rb b/app/presenters/detailed_guide_presenter.rb
@@ -12,14 +12,6 @@ def title_and_context
     end
   end
 
-  def related_guides
-    links("related_guides")
-  end
-
-  def related_mainstream_content
-    links("related_mainstream_content")
-  end
-
   def image
     content_item["details"]["image"]["url"] if content_item["details"]["image"]
   end
@@ -30,8 +22,28 @@ def document_footer
     end
   end
 
+  def related_navigation
+    nav = super
+    nav[:related_items] += related_links("related_mainstream_content")
+    nav[:related_guides] = related_links("related_guides")
+    nav
+  end
+
+  # FIXME: This is a temporary removal of National Applicability
+  # Once all formats have moved to new publisher/important metadata
+  # components, we can remove here: app/presenters/content_item/national_applicability.rb:33
+  def publisher_metadata
+    super.tap { |m| m[:other].delete(:"Applies to") }
+  end
+
 private
 
+  def related_links(key)
+    raw_links = content_item["links"]
+    guides = raw_links.fetch(key, [])
+    guides.map { |g| { text: g["title"], path: g["base_path"] } }
+  end
+
   def related_guides_title
     I18n.t('detailed_guide.related_guides')
   end

diff --git a/app/views/components/_related-navigation.html.erb b/app/views/components/_related-navigation.html.erb
@@ -1,8 +1,10 @@
 <%
   max_section_length = 5
-  defined_sections = %w(topics collections policies topical_events world_locations statistical_data_sets)
+  defined_sections = %w(related_guides topics collections policies topical_events
+                        world_locations statistical_data_sets)
   related_content = [
     {"related_items" => related_items ||= [] },
+    {"related_guides" => related_guides ||= [] },
     {"topics" => topics ||= [] },
     {"collections" => collections ||= [] },
     {"policies" => policies ||= [] },

diff --git a/app/views/content_items/detailed_guide.html+taxonomy_navigation.erb b/app/views/content_items/detailed_guide.html+taxonomy_navigation.erb
diff --git a/app/views/content_items/detailed_guide.html+universal_navigation.erb b/app/views/content_items/detailed_guide.html+universal_navigation.erb
diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb
@@ -2,44 +2,37 @@
   <div class="column-two-thirds">
     <%= render 'govuk_component/title', @content_item.title_and_context %>
   </div>
-  <div class="column-third">
-    <%= render 'components/translation-nav', translations: @content_item.available_translations %>
-    <% if @content_item.image.present? %>
-      <%= image_tag @content_item.image, class: "logo-image" %>
-    <% end %>
+  <% if @content_item.available_translations.length > 1 %>
+    <div class="column-third">
+      <%= render 'components/translation-nav',
+          translations: @content_item.available_translations %>
+    </div>
+  <% end %>
+  <div class="column-two-thirds">
+    <%= render 'govuk_component/lead_paragraph', text: @content_item.description %>
   </div>
 </div>
-<%= render 'components/notice', @content_item.withdrawal_notice_component  %>
-<%= render 'shared/metadata', content_item: @content_item %>
+
+<%= render 'shared/publisher_metadata_with_logo' %>
 <%= render 'shared/history_notice', content_item: @content_item %>
-<%= render 'govuk_component/lead_paragraph', text: @content_item.description %>
+<%= render 'components/notice', @content_item.withdrawal_notice_component  %>
 
-<div
-  class="grid-row sidebar-with-body"
-  data-module="sticky-element-container"
-  id="contents"
->
-  <% if @content_item.contents.any? %>
-    <div class="column-third">
-      <%= render 'components/contents-list', contents: @content_item.contents %>
-    </div>
-  <% end %>
-  <div class="column-two-thirds <% unless @content_item.contents.any? %>offset-one-third<% end %>">
-    <% if @content_item.related_mainstream_content.any? %>
-      <aside class="related-mainstream-content" role="complementary">
-        <h4>
-          <%= raw( t('detailed_guide.related_mainstream_content') ) %>
-        </h4>
-        <% @content_item.related_mainstream_content.each do |link| %>
-          <%= link %><br />
-        <% end %>
-      </aside>
+<div class="grid-row">
+  <div class="column-two-thirds">
+    <%= render 'components/important-metadata',
+        items: @content_item.metadata[:other] %>
+
+    <%= render "components/contents-list-with-body", contents: @content_item.contents do %>
+      <%= render 'govuk_component/govspeak', @content_item.govspeak_body %>
+
+      <div class="responsive-bottom-margin">
+        <%= render 'components/published-dates', {
+            published: @content_item.published,
+            last_updated: @content_item.updated,
+            history: @content_item.history
+          } %>
+      </div>
     <% end %>
-    <%= render 'govuk_component/govspeak', @content_item.govspeak_body %>
-  </div>
-  <div data-sticky-element class="govuk-sticky-element">
-    <%= render 'components/back-to-top', href: "#contents" %>
   </div>
+  <%= render "shared/sidebar_navigation" %>
 </div>
-
-<%= render 'shared/footer', @content_item.document_footer %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -33,6 +33,7 @@ en:
       policies: "Policy"
       publishers: "Published by"
       related_content: "Related content"
+      related_guides: "Detailed guidance"
       statistical_data_sets: "Statistical data set"
       topics: "Explore the topic"
       topical_events: "Topical event"

diff --git a/test/components/related_navigation_test.rb b/test/components/related_navigation_test.rb
@@ -23,6 +23,20 @@ def component_name
     assert_select ".app-c-related-navigation__section-link--other[href=\"/apprenticeships\"]", text: 'Apprenticeships'
   end
 
+  test "renders related guides section when passed related guides" do
+    render_component(
+      related_guides: [
+        {
+          text: "Some other guidance",
+          path: '/something-a-bit-like-this'
+        }
+      ]
+    )
+
+    assert_select ".app-c-related-navigation__sub-heading", text: 'Detailed guidance'
+    assert_select ".app-c-related-navigation__section-link[href=\"/something-a-bit-like-this\"]", text: 'Some other guidance'
+  end
+
   test "renders topics section when passed topic items" do
     render_component(
       topics: [

diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb
@@ -11,21 +11,27 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
     assert_has_component_title(@content_item["title"])
     assert page.has_text?(@content_item["description"])
 
-    assert_has_component_metadata_pair("first_published", "12 June 2014")
-    assert_has_component_metadata_pair("last_updated", "18 February 2016")
-    link1 = "<a href=\"/topic/business-tax/paye\">PAYE</a>"
-    link2 = "<a href=\"/topic/business-tax\">Business tax</a>"
-    assert_has_component_metadata_pair("part_of", [link1, link2])
-    assert_has_component_document_footer_pair("part_of", [link1, link2])
+    assert_has_publisher_metadata(
+      published: "Published 12 June 2014",
+      last_updated: "Last updated 18 February 2016",
+      history_link: true,
+      metadata: {
+        "From:": {
+          "HM Revenue & Customs": "/government/organisations/hm-revenue-customs"
+        }
+      }
+    )
   end
 
   test "renders back to contents elements" do
     setup_and_visit_content_item('detailed_guide')
+
     assert page.has_css?(".app-c-back-to-top[href='#contents']")
   end
 
   test "withdrawn detailed guide" do
     setup_and_visit_content_item('withdrawn_detailed_guide')
+
     assert page.has_css?('title', text: "[Withdrawn]", visible: false)
 
     within ".app-c-notice" do
@@ -35,19 +41,48 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "renders related topics" do
+    setup_and_visit_content_item('detailed_guide')
+
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-topics",
+        section_text: "Explore the topic",
+        links: {
+          "PAYE": "/topic/business-tax/paye",
+          "Business tax": "/topic/business-tax",
+        }
+      }
+    ])
+  end
+
   test "shows related detailed guides" do
     setup_and_visit_content_item('political_detailed_guide')
-    assert_has_component_document_footer_pair("Related guides", ['<a href="/guidance/offshore-wind-part-of-the-uks-energy-mix">Offshore wind: part of the UK&#39;s energy mix</a>'])
+
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-related_guides",
+        section_text: "Detailed guidance",
+        links: {
+          "Offshore wind: part of the UK's energy mix":
+            "/guidance/offshore-wind-part-of-the-uks-energy-mix"
+        }
+      }
+    ])
   end
 
-  test "shows related mainstream content" do
+  test "shows related mainstream links" do
     setup_and_visit_content_item('related_mainstream_detailed_guide')
 
-    within ".related-mainstream-content" do
-      assert page.has_text?('Too much detail?')
-      assert page.has_css?('a[href="/overseas-passports"]', text: 'Overseas British passport applications')
-      assert page.has_css?('a[href="/report-a-lost-or-stolen-passport"]', text: 'Cancel a lost or stolen passport')
-    end
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-related_items",
+        links: {
+          "Overseas British passport applications": "/overseas-passports",
+          "Cancel a lost or stolen passport": "/report-a-lost-or-stolen-passport",
+        }
+      }
+    ])
   end
 
   test "historically political detailed guide" do
@@ -60,16 +95,15 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
 
   test 'detailed guide that only applies to a set of nations' do
     setup_and_visit_content_item('national_applicability_detailed_guide')
-
-    assert_has_component_metadata_pair('Applies to', 'England')
+    assert_has_important_metadata("Applies to:": "England")
   end
 
   test 'detailed guide that only applies to a set of nations, with alternative urls' do
     setup_and_visit_content_item('national_applicability_alternative_url_detailed_guide')
 
-    assert_has_component_metadata_pair(
-      'Applies to',
-      'England, Scotland, and Wales (see guidance for <a rel="external" href="http://www.dardni.gov.uk/news-dard-pa022-a-13-new-procedure-for">Northern Ireland</a>)'
+    assert_has_important_metadata(
+      'Applies to:':
+        'England, Scotland, and Wales (see guidance for Northern Ireland)'
     )
   end