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

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

Closed
wants to merge 1 commit into from

Conversation

frankieroberto
Copy link

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

This fixes a bug experienced downstream on the [Design in the Department of Education](https://design.education.gov.uk) 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
@lfdebrux
Copy link
Member

lfdebrux commented Oct 7, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants