search

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

Update menu html structure so it's one single hierarchical list #240

Closed colinbm closed 3 years ago

colinbm commented 3 years ago

⚠️ Don't forget to update the gem version in the CHANGELOG before merging! When you're ready to release bump version file and generate a tag. ⚠️

What

Updates the navigation to use a single top level ul and nested subnav.

Why

This is a follow up to https://github.com/alphagov/tech-docs-gem/pull/237 with a better solution.

The previous markup was like this:

<ul>
  <li>
    <a href="">Top level link</a>
  </li>
</ul>

<ul>
  <li>
    <a href="">Top level link</a>
    <ul>
      <li>indented nav</li>
    </ul>
  </li>
</ul>

This separates each top level nav item into a separate list, where the nav should be a single component.

New markup is:

<ul>
  <li>
    <a href="">Top level link</a>
  </li>
  <li>
    <a href="">Top level link</a>
    <ul>
      <li>indented nav</li>
    </ul>
  </li>
</ul>
36degrees commented 3 years ago

I don't think b35334d93bebdd65f34d8d5b3e3d6fca46cdc445 belongs in this PR, but otherwise this seems to make sense to me.

colinbm commented 3 years ago

@36degrees It's more adjacent... would you prefer it be in a separate PR? Or I can edit the title to include it explicitly?

36degrees commented 3 years ago

I think it's more or less the same commit as https://github.com/alphagov/tech-docs-gem/pull/238/commits/f46aee7312d58e606cf81ac96018a1774fa73166 in #238? At the minute it'll conflict with that PR because it's changing the same lines of code but in slightly different ways.

colinbm commented 3 years ago

Ah so it is - thanks for clarifying - will remove it here.