From f6c8d8440f3a0b682f24553baa356649ffa4b373 Mon Sep 17 00:00:00 2001
From: Jordan Hatch <jordan@jordanh.net>
Date: Mon, 22 Jul 2013 17:08:39 +0100
Subject: [PATCH 1/5] Separate related links into groups

---
 app/assets/stylesheets/core.scss          |  9 +++--
 app/views/root/related.raw.html.erb       | 45 ++++++++++++++---------
 test/integration/related_template_test.rb | 36 ++++++++++++++++--
 3 files changed, 65 insertions(+), 25 deletions(-)

diff --git a/app/assets/stylesheets/core.scss b/app/assets/stylesheets/core.scss
index 0fffcf508..adb41e879 100644
--- a/app/assets/stylesheets/core.scss
+++ b/app/assets/stylesheets/core.scss
@@ -168,7 +168,7 @@ h4 {
   }
 
   nav {
-    margin-bottom: 2.5em;
+    margin-bottom: 1.25em;
   }
 
   h2 {
@@ -201,8 +201,11 @@ h4 {
       list-style: none outside none;
 
       a {
-        padding: 0;
-        font-weight: bold;
+        padding: 0 0.5em;
+        display: inline;
+        text-decoration: none;
+        background-color: $light-blue-25;
+        font-weight: normal;
       }
     }
 
diff --git a/app/views/root/related.raw.html.erb b/app/views/root/related.raw.html.erb
index 362772f82..541045ab9 100644
--- a/app/views/root/related.raw.html.erb
+++ b/app/views/root/related.raw.html.erb
@@ -6,25 +6,34 @@
   <div class="related-container">
 
     <div class="related" id="related">
-      <div class="inner group">
+      <% artefact.related_artefacts.group_by(&:group).each do |group, related_artefacts| %>
+        <div class="inner group related-<%= group %>">
+          <% if group == "subsection" and section = artefact.primary_section %>
+          <h2 id="parent-subsection"><%= h section["title"] %></h2>
+          <% elsif group == "section" and section = artefact.primary_root_section %>
+          <h2 id="parent-section"><%= h section["title"] %></h2>
+          <% elsif group == "other" %>
+          <h2 id="parent-other">Elsewhere on GOV.UK</h2>
+          <% end %>
 
-        <% if root_section = artefact.primary_root_section %>
-        <h2 id="parent-section"><%= h root_section["title"] %></h2>
-        <% end %>
-        <nav role="navigation" aria-labelledby="parent-section">
-          <ul>
-            <% artefact.related_artefacts.each do |item| %>
-              <li>
-                <a href="<%= h item.web_url %>"><%= h item.title %></a>
-              </li>
-            <% end %>
-            <% if root_section = artefact.primary_root_section %>
-              <li class="related-topic"><a href="<%= h root_section["content_with_tag"]["web_url"] %>"
-                >More</a></li>
-            <% end %>
-          </ul>
-        </nav>
-        <a class="return-to-top" href="#content">Return to top &uarr;</a>
+          <nav role="navigation" aria-labelledby="parent-section">
+            <ul>
+              <% related_artefacts.each do |item| %>
+                <li>
+                  <a href="<%= h item.web_url %>"><%= h item.title %></a>
+                </li>
+              <% end %>
+              <% if section %>
+                <li class="related-topic"><a href="<%= h section["content_with_tag"]["web_url"] %>"
+                  >view all</a></li>
+              <% end %>
+            </ul>
+          </nav>
+       </div>
+     <% end %>
+
+     <div class="inner group">
+       <a class="return-to-top" href="#content">Return to top &uarr;</a>
      </div>
 
      <div id="legacy-sources">
diff --git a/test/integration/related_template_test.rb b/test/integration/related_template_test.rb
index 827dbd9a3..a394a4c78 100644
--- a/test/integration/related_template_test.rb
+++ b/test/integration/related_template_test.rb
@@ -10,9 +10,14 @@ def get_template
 
   context "with related artefacts" do
     setup do
-      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo")
-      @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar")
-      @artefact = stub("Artefact", :related_artefacts => [@related1, @related2], :primary_root_section => { "content_with_tag" => { "web_url" => "/browse/section" }})
+      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection" )
+      @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar", :group => "section")
+      @related3 = stub("Artefact", :web_url => "http://www.example.com/baz", :title => "Baz", :group => "other")
+      @artefact = stub("Artefact",
+        :related_artefacts => [@related1, @related2, @related3],
+        :primary_root_section => { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" }},
+        :primary_section => { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } }
+      )
     end
 
     should "add an item for each related item" do
@@ -39,6 +44,20 @@ def get_template
       assert_match /foo\?bar=baz&amp;id=2/, result
     end
 
+    should "add the subsection links if present" do
+      @artefact.stubs(:primary_section).returns({
+        "title" => "Subsection",
+        "content_with_tag" => {"web_url" => "http://www.example.com/browse/section/subsection"},
+      })
+      template = get_template
+      artefact = @artefact
+      result = ERB.new(template).result(binding)
+      doc = Nokogiri::HTML.parse(result)
+
+      assert doc.at_css("ul li.related-topic a[href='http://www.example.com/browse/section/subsection']")
+      assert_equal "view all", doc.at_css("ul li a[href='http://www.example.com/browse/section/subsection']").text
+    end
+
     should "add the section link if present" do
       @artefact.stubs(:primary_root_section).returns({
         "title" => "Something",
@@ -50,7 +69,16 @@ def get_template
       doc = Nokogiri::HTML.parse(result)
 
       assert doc.at_css("ul li.related-topic a[href='http://www.example.com/browse/something']")
-      assert_equal "More", doc.at_css("ul li a[href='http://www.example.com/browse/something']").text
+      assert_equal "view all", doc.at_css("ul li a[href='http://www.example.com/browse/something']").text
+    end
+
+    should "add the internal links elsewhere if present" do
+      template = get_template
+      artefact = @artefact
+      result = ERB.new(template).result(binding)
+      doc = Nokogiri::HTML.parse(result)
+
+      assert_equal "Elsewhere on GOV.UK", doc.at_css("h2#parent-other").text
     end
   end
 

From 1157aaf64ef39323c3a4d0323d4dbae19f7d9ed1 Mon Sep 17 00:00:00 2001
From: Jordan Hatch <jordan@jordanh.net>
Date: Tue, 23 Jul 2013 14:45:02 +0100
Subject: [PATCH 2/5] Tidy grouped related links template

Use a case statement to select the section
---
 app/views/root/related.raw.html.erb       | 17 ++++++++++-------
 test/integration/related_template_test.rb |  5 +++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/app/views/root/related.raw.html.erb b/app/views/root/related.raw.html.erb
index 541045ab9..99668886e 100644
--- a/app/views/root/related.raw.html.erb
+++ b/app/views/root/related.raw.html.erb
@@ -7,13 +7,16 @@
 
     <div class="related" id="related">
       <% artefact.related_artefacts.group_by(&:group).each do |group, related_artefacts| %>
+        <%
+          section = case group
+                    when 'subsection' then artefact.primary_section
+                    when 'section' then artefact.primary_root_section
+                    when 'other' then { "title" => "Elsewhere on GOV.UK" }
+                    end
+        %>
         <div class="inner group related-<%= group %>">
-          <% if group == "subsection" and section = artefact.primary_section %>
-          <h2 id="parent-subsection"><%= h section["title"] %></h2>
-          <% elsif group == "section" and section = artefact.primary_root_section %>
-          <h2 id="parent-section"><%= h section["title"] %></h2>
-          <% elsif group == "other" %>
-          <h2 id="parent-other">Elsewhere on GOV.UK</h2>
+          <% if section %>
+          <h2 id="parent-<%= group %>"><%= h section["title"] %></h2>
           <% end %>
 
           <nav role="navigation" aria-labelledby="parent-section">
@@ -23,7 +26,7 @@
                   <a href="<%= h item.web_url %>"><%= h item.title %></a>
                 </li>
               <% end %>
-              <% if section %>
+              <% if section and section["content_with_tag"] and section["content_with_tag"]["web_url"].present? %>
                 <li class="related-topic"><a href="<%= h section["content_with_tag"]["web_url"] %>"
                   >view all</a></li>
               <% end %>
diff --git a/test/integration/related_template_test.rb b/test/integration/related_template_test.rb
index a394a4c78..e45b32aad 100644
--- a/test/integration/related_template_test.rb
+++ b/test/integration/related_template_test.rb
@@ -10,13 +10,14 @@ def get_template
 
   context "with related artefacts" do
     setup do
-      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection" )
+      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection")
       @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar", :group => "section")
       @related3 = stub("Artefact", :web_url => "http://www.example.com/baz", :title => "Baz", :group => "other")
       @artefact = stub("Artefact",
         :related_artefacts => [@related1, @related2, @related3],
         :primary_root_section => { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" }},
-        :primary_section => { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } }
+        :primary_section => { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } },
+        :format => "guide"
       )
     end
 

From 32b1bfe90044cfa1fc0d35f88d327f223a3e4210 Mon Sep 17 00:00:00 2001
From: Jordan Hatch <jordan@jordanh.net>
Date: Tue, 23 Jul 2013 14:51:25 +0100
Subject: [PATCH 3/5] Use the second-level section name instead of Travel
 Advice

This isn't a very nice workaround, but there's only so much
fudging of section names which can be done in Frontend
when the breadcrumbs are set from the same data
---
 app/views/root/related.raw.html.erb       |  1 +
 test/integration/related_template_test.rb | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/app/views/root/related.raw.html.erb b/app/views/root/related.raw.html.erb
index 99668886e..56eb9b27e 100644
--- a/app/views/root/related.raw.html.erb
+++ b/app/views/root/related.raw.html.erb
@@ -13,6 +13,7 @@
                     when 'section' then artefact.primary_root_section
                     when 'other' then { "title" => "Elsewhere on GOV.UK" }
                     end
+          section = section['parent'] if artefact.format == "travel-advice" and group == "subsection"
         %>
         <div class="inner group related-<%= group %>">
           <% if section %>
diff --git a/test/integration/related_template_test.rb b/test/integration/related_template_test.rb
index e45b32aad..b3b6be87a 100644
--- a/test/integration/related_template_test.rb
+++ b/test/integration/related_template_test.rb
@@ -81,6 +81,25 @@ def get_template
 
       assert_equal "Elsewhere on GOV.UK", doc.at_css("h2#parent-other").text
     end
+
+    should "use the second-level section name for travel advice formats" do
+      @artefact.stubs(:format).returns("travel-advice")
+      @artefact.stubs(:primary_section).returns({
+        "title" => "Foreign travel advice",
+        "content_with_tag" => {"web_url" => "/foreign-travel-advice"},
+        "parent" => {
+          "title" => "Foo",
+          "content_with_tag" => {"web_url" => "/browse/abc/foo"}
+        }
+      })
+
+      template = get_template
+      artefact = @artefact
+      result = ERB.new(template).result(binding)
+      doc = Nokogiri::HTML.parse(result)
+
+      assert_equal "Foo", doc.at_css("h2#parent-subsection").text
+    end
   end
 
   should "be blank with no artefact" do

From d5dbd4132aac6d0834598051d135dff99693d2c6 Mon Sep 17 00:00:00 2001
From: Jordan Hatch <jordan@jordanh.net>
Date: Tue, 23 Jul 2013 15:21:46 +0100
Subject: [PATCH 4/5] Remove the grey bottom border from each related group

This mysteriously evaded the previous commit.
---
 app/assets/stylesheets/core.scss | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/assets/stylesheets/core.scss b/app/assets/stylesheets/core.scss
index adb41e879..7847cd982 100644
--- a/app/assets/stylesheets/core.scss
+++ b/app/assets/stylesheets/core.scss
@@ -153,8 +153,7 @@ h4 {
   }
 
   .inner {
-    background-color: $white;;
-    border-bottom: 10px solid $grey-8;
+    background-color: $white;
     padding: 0.5em 1em 0 0;
     width: auto;
 

From 05014a866a1aa926a11785c1ae12a9756236b0f1 Mon Sep 17 00:00:00 2001
From: Jordan Hatch <jordan@jordanh.net>
Date: Tue, 23 Jul 2013 16:04:07 +0100
Subject: [PATCH 5/5] Fix the aria tag for related item lists

---
 app/views/root/related.raw.html.erb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/root/related.raw.html.erb b/app/views/root/related.raw.html.erb
index 56eb9b27e..40acf0920 100644
--- a/app/views/root/related.raw.html.erb
+++ b/app/views/root/related.raw.html.erb
@@ -20,7 +20,7 @@
           <h2 id="parent-<%= group %>"><%= h section["title"] %></h2>
           <% end %>
 
-          <nav role="navigation" aria-labelledby="parent-section">
+          <nav role="navigation" aria-labelledby="parent-<%= group %>">
             <ul>
               <% related_artefacts.each do |item| %>
                 <li>