Merge pull request #853 from alphagov/use-b-variant-of-component

Always use the non-numbered contents list
alphagov · Mar 28, 2018 · 9120ddc · 9120ddc
2 parents 77d65e1 + b395d05
commit 9120ddc
Show file tree

Hide file tree

Showing 12 changed files with 23 additions and 215 deletions.
diff --git a/app/controllers/concerns/guide_nav_ab_testable.rb b/app/controllers/concerns/guide_nav_ab_testable.rb
diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb
@@ -1,6 +1,4 @@
 class ContentItemsController < ApplicationController
-  include GuideNavAbTestable
-
   rescue_from GdsApi::HTTPForbidden, with: :error_403
   rescue_from GdsApi::HTTPNotFound, with: :error_notfound
   rescue_from GdsApi::HTTPGone, with: :error_410

diff --git a/app/presenters/guide_presenter.rb b/app/presenters/guide_presenter.rb
@@ -26,10 +26,6 @@ def multi_page_guide?
     parts.size > 1
   end
 
-  def current_part_title_with_index
-    "#{current_part_index + 1}. #{current_part_title}"
-  end
-
   def print_link
     "#{base_path}/print"
   end

diff --git a/app/views/content_items/guide.html.erb b/app/views/content_items/guide.html.erb
@@ -4,19 +4,15 @@
     <%= render 'govuk_component/title', { title: @content_item.title } %>
     <% if @content_item.multi_page_guide? %>
       <aside class="part-navigation-container" role="complementary">
-        <%= render 'shared/parts_navigation', content_item: @content_item, navigation_label: 'Pages in this guide' %>
+        <%= render "components/contents-list", aria_label: 'Pages in this guide', contents: @content_item.part_link_elements %>
       </aside>
     <% end %>
   </div>
   <div class="column-two-thirds">
     <% if @content_item.has_parts? %>
       <% if @content_item.multi_page_guide? %>
         <h1 class="part-title">
-          <% if show_alternate_guide_chapters? %>
-            <%= @content_item.current_part_title %>
-          <% else %>
-            <%= @content_item.current_part_title_with_index %>
-          <% end %>
+          <%= @content_item.current_part_title %>
         </h1>
       <% end %>
       <%= render 'govuk_component/govspeak',

diff --git a/app/views/content_items/travel_advice.html.erb b/app/views/content_items/travel_advice.html.erb
@@ -8,7 +8,25 @@
   <div class="column-two-thirds">
     <%= render 'govuk_component/title', @content_item.title_and_context %>
     <aside class="part-navigation-container" role="complementary">
-      <%= render 'shared/parts_navigation', content_item: @content_item, navigation_label: 'Travel advice pages' %>
+
+      <nav role="navigation" class="grid-row part-navigation" aria-label="Travel advice pages" data-module="track-click">
+        <%  @content_item.parts_navigation.each_with_index do |part_group, i| %>
+          <ol
+            class="column-half"
+            <% if i == 1 %>
+              start="<%=  @content_item.parts_navigation_second_list_start %>"
+            <% end %>
+          >
+            <% part_group.each do |part_nav_item| %>
+              <li>
+                <%= part_nav_item %>
+              </li>
+            <% end %>
+          </ol>
+        <% end %>
+      </nav>
+
+
       <%= render 'components/subscription-links', email_signup_link: @content_item.email_signup_link, feed_link: @content_item.feed_link %>
     </aside>
   </div>

diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -24,7 +24,6 @@
     <meta name="description" content="<%= strip_tags(@content_item.description) %>" />
   <% end %>
   <meta name="govuk:content-id" content="<%= @content_item.content_id %>" />
-  <%= guide_test_variant.analytics_meta_tag.html_safe if is_tested_guide? %>
   <%= yield :extra_head_content %>
 </head>
 <body>

diff --git a/app/views/shared/_parts_navigation.html.erb b/app/views/shared/_parts_navigation.html.erb
diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb
@@ -3,7 +3,6 @@
 class ContentItemsControllerTest < ActionController::TestCase
   include GdsApi::TestHelpers::ContentStore
   include GdsApi::TestHelpers::Rummager
-  include GovukAbTesting::MinitestHelpers
 
   test 'routing handles paths with no format or locale' do
     assert_routing(

diff --git a/test/controllers/guide_chapter_nav_ab_test_controller_test.rb b/test/controllers/guide_chapter_nav_ab_test_controller_test.rb
diff --git a/test/controllers/step_navigation_controller_test.rb b/test/controllers/step_navigation_controller_test.rb
@@ -2,7 +2,6 @@
 
 class ContentItemsControllerTest < ActionController::TestCase
   include GdsApi::TestHelpers::ContentStore
-  include GovukAbTesting::MinitestHelpers
 
   %w(guide answer publication).each do |schema_name|
     test "#{schema_name} shows step by step navigation where relevant" do

diff --git a/test/integration/guide_test.rb b/test/integration/guide_test.rb
@@ -11,44 +11,21 @@ class GuideTest < ActionDispatch::IntegrationTest
     assert page.has_css?("title", visible: false, text: @content_item['title'])
     assert_has_component_title(@content_item['title'])
 
-    assert page.has_css?('.part-navigation ol', count: 2)
-    assert page.has_css?('.part-navigation li', count: @content_item['details']['parts'].size)
-
-    @content_item["details"]["parts"].each_with_index do |part, i|
-      if i.zero?
-        assert page.has_css?('.part-navigation li', text: part['title'])
-        refute page.has_css?('.part-navigation li a', text: part['title'])
-      else
-        assert page.has_css?(".part-navigation li a[href*=\"#{part['slug']}\"]", text: part['title'])
-      end
-    end
-
-    assert page.has_css?('h1', text: "1. #{@content_item['details']['parts'].first['title']}")
+    assert page.has_css?('h1', text: @content_item['details']['parts'].first['title'])
     assert page.has_css?(shared_component_selector("previous_and_next_navigation"))
     assert page.has_css?('.app-c-print-link a[href$="/print"]')
-
-    assert page.has_css?(".part-navigation[data-module='track-click']")
-    assert_tracking_link("category", "contentsClicked", (@content_item['details']['parts'].size - 1))
-    assert_tracking_link("action", "content_item 2")
-    assert_tracking_link("label", "/national-curriculum/key-stage-1-and-2")
-    assert_tracking_link("options", { dimension29: "Key stage 1 and 2" }.to_json)
   end
 
   test "draft access tokens are appended to part links within navigation" do
     setup_and_visit_content_item('guide', '?token=some_token')
 
-    assert page.has_css?('.part-navigation a[href$="?token=some_token"]')
+    assert page.has_css?('.app-c-contents-list a[href$="?token=some_token"]')
   end
 
   test "does not show part navigation, print link or part title when only one part" do
     setup_and_visit_content_item('single-page-guide')
 
     refute page.has_css?('h1', text: @content_item['details']['parts'].first['title'])
-    refute page.has_css?('.part-navigation')
     refute page.has_css?('.app-c-print-link')
   end
-
-  def assert_tracking_link(name, value, total = 1)
-    assert page.has_css?("a[data-track-#{name}='#{value}']", count: total)
-  end
 end
diff --git a/test/presenters/guide_presenter_test.rb b/test/presenters/guide_presenter_test.rb
@@ -17,11 +17,6 @@ def schema_name
       end
     end
 
-    test "presents part titles with their index" do
-      first_part_title = schema_item['details']['parts'].first['title']
-      assert_equal presented_item.current_part_title_with_index, "1. #{first_part_title}"
-    end
-
     test 'presents withdrawn in the title for withdrawn content' do
       presented_item = presented_item(schema_name, nil, "withdrawn_notice" => { "explanation": "Withdrawn", "withdrawn_at": "2014-08-22T10:29:02+01:00" })
       assert_equal "[Withdrawn] The national curriculum", presented_item.page_title