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

Bugfix: only add link if an ID attribute is present #305

Closed frankieroberto closed 2 years ago

frankieroberto commented 2 years ago

This fixes a bug experienced downstream on the Design in the Department of Education website, where a link was being injected with the href of #undefined due to a missing id attribute on the <h2> element.

This could be fixed by auto-generating an ID attribute if one is missing, but perhaps a safer alternative would be to not inject the link at all in these circumstances. This would also let services opt-out of adding the link on specific headings where it doesn’t make sense to do so, such on a homepage where the heading already contains a link to another page.

See https://github.com/DFE-Digital/design-in-dfe/pull/11

What’s changed

Identifying a user need

lfdebrux commented 2 years ago

Hi @frankieroberto, thanks for reporting this bug, it's definitely not great that anchor links can have invalid URLs without authors realising.

Having talked about it with the rest of the tech docs team we're not sure about leaving the heading without an anchor link; that would mean some pages could have some headings with anchor links and some without, which we worry would be an inconsistent experience for users. We'd prefer a solution that autogenerates a sensible default, as you suggested, or failing that reports missing ids during the site generation.

I'm going to close this PR, but will raise an issue with the bug, please feel free to raise another PR with an alternative fix if you are able.