alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

Sidebar navigation is missing outer `<ul>` if `multipage_nav` is disabled #250

Closed 36degrees closed 3 years ago

36degrees commented 3 years ago

There’s no outer <ul> when multipage_nav is false – it jumps straight from the <nav> to the <li>.

<nav id="toc" class="js-toc-list toc__list" aria-labelledby="toc-heading" data-module="collapsible-navigation">
                        <li>
    <a href="#the-title" class="toc-link--in-view"><span>The title</span></a>
    <ul>
      <li>
        <a href="#a-subheader"><span>A subheader</span></a>
        <ul>
          <li>
            <a href="#a-h3-subheader"><span>A h3 subheader</span></a>
          </li>
        </ul>
      </li>
      <li>
        <a href="#another-subheader"><span>Another subheader</span></a>
      </li>
    </ul>
  </li>
</nav>

This causes rendering issues, as seen in https://github.com/alphagov/gds-way/pull/632.

Before

current (1)

After

new

36degrees commented 3 years ago

The issue is visible in the spec:

https://github.com/alphagov/tech-docs-gem/blob/8375d932d797bc8fc59e5e0217f020fbf86a1455/spec/table_of_contents/helpers_spec.rb#L20-L32

https://github.com/alphagov/tech-docs-gem/blob/8375d932d797bc8fc59e5e0217f020fbf86a1455/spec/table_of_contents/helpers_spec.rb#L146-L169

The render_page_tree method called by multi_page_table_of_contents manually adds the outer <ul> so possibly the single_page_table_of_contents method just needs to do the same…?

We should probably add an example of a non multipage nav to the 'example' docs, although at the minute this is configured at the site level rather than per page.