Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ID shared by different pages being considered duplicates #311

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions example/source/shared-subheadings/page-a.html.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
layout: layout
---

# Shared subheadings - Page A

A page that shares headings with the other pages in this folder,
to check that the heading IDs are computed properly

## A subheading

### A h3 subheading

## Another subheading
14 changes: 14 additions & 0 deletions example/source/shared-subheadings/page-b.html.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
layout: layout
---

# Shared subheadings - Page B

A page that shares headings with the other pages in this folder,
to check that the heading IDs are computed properly

## A subheading

### A h3 subheading

## Another subheading
2 changes: 0 additions & 2 deletions lib/govuk_tech_docs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
require "govuk_tech_docs/page_review"
require "govuk_tech_docs/pages"
require "govuk_tech_docs/tech_docs_html_renderer"
require "govuk_tech_docs/unique_identifier_extension"
require "govuk_tech_docs/unique_identifier_generator"
require "govuk_tech_docs/warning_text_extension"
require "govuk_tech_docs/api_reference/api_reference_extension"
Expand Down Expand Up @@ -62,7 +61,6 @@ def self.configure(context, options = {})

config_file = ENV.fetch("CONFIG_FILE", "config/tech-docs.yml")
context.config[:tech_docs] = YAML.load_file(config_file).with_indifferent_access
context.activate :unique_identifier
context.activate :warning_text
context.activate :api_reference

Expand Down
9 changes: 8 additions & 1 deletion lib/govuk_tech_docs/tech_docs_html_renderer.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "middleman-core/renderers/redcarpet"
require "govuk_tech_docs/unique_identifier_generator"

module GovukTechDocs
class TechDocsHTMLRenderer < Middleman::Renderers::MiddlemanRedcarpetHTML
Expand All @@ -10,12 +11,18 @@ def initialize(options = {})
super
end

def preprocess(document)
@unique_identifier_generator = UniqueIdentifierGenerator.new

document
end

def paragraph(text)
@app.api("<p>#{text.strip}</p>\n")
end

def header(text, level)
anchor = UniqueIdentifierGenerator.instance.create(text, level)
anchor = @unique_identifier_generator.create(text, level)
%(<h#{level} id="#{anchor}">#{text}</h#{level}>\n)
end

Expand Down
13 changes: 0 additions & 13 deletions lib/govuk_tech_docs/unique_identifier_extension.rb

This file was deleted.

10 changes: 1 addition & 9 deletions lib/govuk_tech_docs/unique_identifier_generator.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
require "singleton"

module GovukTechDocs
class UniqueIdentifierGenerator
include Singleton

Anchor = Struct.new(:id, :level)

attr_reader :anchors

def initialize
reset
@anchors = []
end

def create(id, level)
Expand All @@ -28,10 +24,6 @@ def create(id, level)
anchor
end

def reset
@anchors = []
end

private

def prefixed_by_parent(anchor, level)
Expand Down
47 changes: 47 additions & 0 deletions spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,51 @@ def hello_world
end
end
end

describe "#render unique heading IDs" do
it "Automatically assigns an ID to the heading" do
output = processor.render <<~MARKDOWN
# A heading
## A subheading
MARKDOWN

expect(output).to include('<h1 id="a-heading">A heading</h1>')
expect(output).to include('<h2 id="a-subheading">A subheading</h2>')
end

it "Ensures IDs are unique among headings in the page" do
output = processor.render <<~MARKDOWN
# A heading
## A subheading
### A shared heading
### A shared heading
### A shared heading
## Another subheading
### A shared heading
MARKDOWN

# Fist heading will get its ID as a normal heading
expect(output).to include('<h3 id="a-shared-heading">A shared heading</h3>')
# Second occurence will be prefixed by the previous heading to ensure uniqueness
expect(output).to include('<h3 id="a-subheading-a-shared-heading">A shared heading</h3>')
# Third occurence will be get the prefix, as well as a numeric suffix, to keep ensuring uniqueness
expect(output).to include('<h3 id="a-subheading-a-shared-heading-2">A shared heading</h3>')
# Finally the last occurence will get a prefix, but no number as it's in a different section
expect(output).to include('<h3 id="another-subheading-a-shared-heading">A shared heading</h3>')
end

it "Does not consider unique IDs across multiple renders" do
processor.render <<~MARKDOWN
# A heading
## A subheading
MARKDOWN

second_output = processor.render <<~MARKDOWN
# A heading
## A subheading
MARKDOWN

expect(second_output).to include('<h2 id="a-subheading">A subheading</h2>')
end
end
end
12 changes: 1 addition & 11 deletions spec/govuk_tech_docs/unique_identifier_generator_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
RSpec.describe GovukTechDocs::UniqueIdentifierGenerator do
subject { described_class.instance }
before { subject.reset }
subject { described_class.new }

describe "#create" do
it "lower-cases the text" do
Expand Down Expand Up @@ -66,13 +65,4 @@
end
end
end

describe "#reset" do
it "clears the list of existing anchors" do
subject.create("An Anchor", 1)
expect(subject.anchors).to_not be_empty
subject.reset
expect(subject.anchors).to be_empty
end
end
end