alphagov / govuk_publishing_components

A gem to document and distribute frontend components for GOV.UK applications
https://components.publishing.service.gov.uk
MIT License
66 stars 20 forks source link

Accordion anchor link navigation breaks with invalid anchor #2086

Closed andysellick closed 2 years ago

andysellick commented 3 years ago

I'm not sure I fully understand the exact problem here but raising for further investigation.

The accordion has an option to allow a section to open based on the URL hash. This can break the entire accordion JS if the hash does not match an accordion section, but does match another element.

I found this in the components audit pages (running locally) where accordions are inside tabs. Reloading the page having clicked a tab left the URL hash as #applications, which resulted in the below.

Screenshot 2021-05-20 at 17 17 27

This is the error in the console:

Screenshot 2021-05-20 at 17 17 34

According to the documentation this option can be disabled if the anchor_navigation flag is false, but this didn't change the problem (this flag defaults to false, and I wasn't setting it).

Thanks to @owenatgov for helping debug this.

andysellick commented 3 years ago

@owenatgov I'm trying to work out what else needs fixing on this. I thought there might be a problem if the anchor link matched both an accordion section and a tab (for example) but that shouldn't happen because IDs are unique. Can you clarify what else needs fixing?

MartinJJones commented 2 years ago

@andysellick following on from the frontend triage meeting yesterday.

An issue exists in the component given the steps below:

Broken example - id in the URL is for a regular heading element that exists in the document, but is separate to the accordion https://www.gov.uk/guidance/how-to-publish-on-gov-uk/creating-and-updating-pages#section-title

A possible fix could be to get the id from the module instead of the document: https://github.com/alphagov/govuk_publishing_components/blob/main/app/assets/javascripts/govuk_publishing_components/components/accordion.js#L64

There may be other fixes and refactoring required when testing further.