Skip to content

Commit

Permalink
Fix: Don't break TOC when OpenAPI description includes headers
Browse files Browse the repository at this point in the history
As noted in [0], the ability to render a CommonMark document through
OpenAPI specifications' `info.description` is breaking due to how the
Table of Contents code works.

We'd assumed that the `id` would always be set on headers - by Middleman
- but as it's possible for an OpenAPI description to include headers, we
won't always have one.

To solve this, we can make it possible for a link to not be present for
a heading, instead returning just a `<span> for the table of contents.

This allows the headings to still be present, but won't be actionable.

Until the `openapi3_parser` gem supports making this configurable, i.e.
be allowing us to inject in headers, this is a good middle ground.

[0]: alphagov/tdt-documentation#156
  • Loading branch information
jamietanna committed Nov 10, 2021
1 parent ee6b8aa commit a6df7c1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 2 deletions.
4 changes: 3 additions & 1 deletion lib/govuk_tech_docs/table_of_contents/heading.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ def size
def href
if @page_url != "" && size == 1
@page_url
else
elsif @attributes["id"]
@page_url + "#" + @attributes["id"]
else
nil
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ def render_tree(tree, indentation: DEFAULT_INDENTATION, level: nil)
output = ""

if tree.heading
output += indentation + %{<a href="#{tree.heading.href}"><span>#{tree.heading.title}</span></a>\n}
if tree.heading.href
output += indentation + %{<a href="#{tree.heading.href}"><span>#{tree.heading.title}</span></a>\n}
else
output += indentation + %{<span>#{tree.heading.title}</span>\n}
end
end

if tree.children.any? && level < @max_level
Expand Down
28 changes: 28 additions & 0 deletions spec/table_of_contents/helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,34 @@ class Subject
expect(subject.single_page_table_of_contents(html).strip).to eq(expected_single_page_table_of_contents.strip)
end

it "builds a table of contents from html, when items do not include an `id`, for instance if included as Markdown in an OpenAPI document" do
html = %{
<h1>Fruit</h1>
<h1 id="apples">Apples</h1>
<p>A fruit</p>
<h2 id="apple-recipes">Apple recipes</h2>
<p>Get some apples..</p>
}

expected_single_page_table_of_contents = %{
<ul>
<li>
<span>Fruit</span>
</li>
<li>
<a href="#apples"><span>Apples</span></a>
<ul>
<li>
<a href="#apple-recipes"><span>Apple recipes</span></a>
</li>
</ul>
</li>
</ul>
}

expect(subject.single_page_table_of_contents(html).strip).to eq(expected_single_page_table_of_contents.strip)
end

it "builds a table of contents from html when headings suddenly change by more than one size" do
html = %{
<h1 id="fruit">Fruit</h1>
Expand Down

0 comments on commit a6df7c1

Please sign in to comment.