Bugfix/issue 446 inconsistent active tab

example/config.rb

@@ -7,3 +7,18 @@
 proxy "/a-proxied-page.html", "templates/proxy_template.html", locals: {
   title: "I am a title",
 }
+
+gem_root = File.expand_path("..", __dir__)
+files.watch :reload, path: File.join(gem_root, "lib")
+
+configure :development do
+  ready do
+    files.on_change :reload do |changed|
+      changed.each do |file|
+        next unless file.full_path.extname == ".rb"
+
+        load file.full_path.to_s
+      end
+    end
+  end
+end

example/config/tech-docs.yml

@@ -14,6 +14,7 @@ header_links:
   Expired with owner: /expired-page-with-owner.html
   Not expired page: /not-expired-page.html
   Single page nav: /single-page-nav.html
+  Active pages: /active-pages/
 
 footer_links:
   Accessibility: /hidden-page.html

example/source/active-pages/index.html.md.erb

@@ -0,0 +1,7 @@
+---
+title: Active page section root
+---
+
+# This is a section landing page, under the "Active pages" navigation link.
+
+The "Active pages" navigation link should be active.

example/source/active-pages/sub-section/index.html.md.erb

@@ -0,0 +1,7 @@
+---
+title: Active page sub section root
+---
+
+# This is a section sub section, under the "Active pages" navigation link.
+
+The "Active pages" navigation link should STILL BE active.

lib/govuk_tech_docs.rb

@@ -101,11 +101,35 @@ def format_date(date)
       end
 
       def active_page(page_path)
-        [
-          page_path == "/" && current_page.path == "index.html",
-          "/#{current_page.path}" == page_path,
-          !current_page.data.parent.nil? && current_page.data.parent.to_s == page_path,
-        ].any?
+        # if we are on the homepage e.g  Documentation: /
+        if page_path == "/" && current_page.url == "/"
+          return true
+        end
+
+        # if we are on a single page the header links point to e.g Expired page: /expired-page.html
+        if "/#{current_page.path}" == page_path
+          return true
+        end
+
+        # If the doc maintainer has set the parent data in the frontmatter for the current page,
+        # and it matches the value in the config e.g
+        # Expired page: /expired-page.html (config) and parent: /expired-page.html (child-of-expired-page.html)
+        if !current_page.data.parent.nil? && current_page.data.parent.to_s == page_path
+          return true
+        end
+
+        # If we have a nested directory structure that matches a navigation link e.g  Active pages: /active-pages/
+        if current_page.url.start_with?(page_path)
+          # feels like a weird check, but stops false positive where "/" for root is also the start of "/active-pages"
+          if page_path == "/"
+            return current_page.url == "/"
+          end
+
+          # safe to return true, since we aren't in the situation above
+          return true
+        end
+        # no other reason to believe we're active
+        false
       end
     end
 

spec/features/header_links.rb

@@ -0,0 +1,162 @@
+require "rack/files"
+require "capybara/rspec"
+
+# want to be able to test directory indexes without the .html extension
+Capybara.app = Rack::Builder.new do
+  use Rack::Static, urls: [""], root: "example/build", index: "index.html"
+  run Rack::Files.new("example/build")
+end
+
+#   Documentation: /
+#   Expired page: /expired-page.html
+#   Expired with owner: /expired-page-with-owner.html
+#   Not expired page: /not-expired-page.html
+#   Single page nav: /single-page-nav.html
+#   Active pages: /active-pages/
+
+describe "Active header links " do
+  include Capybara::DSL
+
+  describe "are not visible on " do
+    it "the homepage when there are none provided to the config" do
+      given_the_site_is_created_without_links_in_the_config
+      when_i_visit_the_url_provided "/"
+      when_the_page_has_finished_loading
+      then_no_navigation_links_are_present
+      then_no_active_link_class_is_present
+    end
+
+    it "nested pages when there are none provided to the config" do
+      given_the_site_is_created_without_links_in_the_config
+      when_i_visit_the_url_provided "/nested-page/another-nested-page/"
+      when_the_page_has_finished_loading
+      then_no_navigation_links_are_present
+      then_no_active_link_class_is_present
+    end
+
+    it "full urls when there are none provided to the config" do
+      given_the_site_is_created_without_links_in_the_config
+      when_i_visit_the_url_provided "/expired-page-with-owner.html"
+      when_the_page_has_finished_loading
+      then_no_navigation_links_are_present
+      then_no_active_link_class_is_present
+    end
+  end
+
+  describe "are visible" do
+    it "when header_links are present in the config" do
+      given_the_site_is_created_with_links_in_the_config
+      when_i_visit_the_url_provided "/"
+      when_the_page_has_finished_loading
+      then_the_header_links_in_the_config_are_visible
+    end
+
+    describe "are active on " do
+      it "the homepage only when visiting the home page" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_only_the_expected_header_link_is_active "Documentation"
+      end
+
+      it "the expected link when the header_link is a specific .html page" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/expired-page.html"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_only_the_expected_header_link_is_active "Expired page"
+      end
+
+      it "the expected link when the page has a parent specified that is a header_link" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/child-of-expired-page.html"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_only_the_expected_header_link_is_active "Expired page"
+      end
+
+      it "the expected link when the page is the root of a nested folder section" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/active-pages/"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_the_page_should_have_the_expected_content "This is a section landing page, under the “Active pages” navigation link."
+        then_only_the_expected_header_link_is_active "Active pages"
+      end
+
+      it "the expected link when the page is a child of a nested folder section" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/active-pages/sub-section/"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_the_page_should_have_the_expected_content "This is a section sub section, under the “Active pages” navigation link."
+        then_only_the_expected_header_link_is_active "Active pages"
+      end
+    end
+
+    describe "are not active on" do
+      it "a page that has no header link or parent associated with it" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/api-path.html"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_no_active_link_class_is_present
+      end
+
+      it "a nested page that has no header link or parent associated with it" do
+        given_the_site_is_created_with_links_in_the_config
+        when_i_visit_the_url_provided "/nested-page/another-nested-nested-page/"
+        when_the_page_has_finished_loading
+        then_the_header_links_in_the_config_are_visible
+        then_no_active_link_class_is_present
+      end
+    end
+  end
+
+  def given_the_site_is_created_without_links_in_the_config
+    rebuild_site! overrides: { "header_links" => {} }
+  end
+
+  def then_only_the_expected_header_link_is_active(link_text)
+    active_links = all(
+      ".govuk-header__navigation-item.govuk-header__navigation-item--active",
+    )
+    expect(active_links.length).to eq(1)
+    expect(active_links[0].text).to eq(link_text)
+  end
+
+  def given_the_site_is_created_with_links_in_the_config
+    rebuild_site!
+  end
+
+  def when_i_visit_the_url_provided(page_path)
+    visit page_path
+  end
+
+  def when_the_page_has_finished_loading
+    assert_text("My First Service") # tech_docs_config["service_name"]
+  end
+
+  def then_the_page_should_have_the_expected_content(content_string)
+    assert_text(content_string)
+  end
+
+  def then_no_navigation_links_are_present
+    expect(page).not_to have_css("li.govuk-header__navigation-item")
+  end
+
+  def then_no_active_link_class_is_present
+    expect(page).not_to have_css("li.govuk-header__navigation-item--active")
+  end
+
+  def then_the_header_links_in_the_config_are_visible
+    tech_docs_config["header_links"].each do |link_text, path|
+      link = page.find(
+        "li.govuk-header__navigation-item a.govuk-header__link",
+        exact_text: link_text,
+      )
+      expect(link[:href]).to eq(path)
+    end
+  end
+end

spec/site_helper.rb

@@ -6,13 +6,17 @@ module Helpers
 
   # Build the example site using middleman
 
+  def tech_docs_config
+    @tech_docs_config
+  end
+
   def rebuild_site!(config: "config/tech-docs.yml", overrides: {})
     config_file = Tempfile.new("tech_docs_config")
 
     begin
       Dir.chdir("example") do
-        new_config = YAML.load_file(config).merge(overrides)
-        config_file.write(YAML.dump(new_config))
+        @tech_docs_config = YAML.load_file(config).merge(overrides).freeze
+        config_file.write(YAML.dump(tech_docs_config.to_h))
         config_file.close
         command = [
           "rm -rf build",