Open romaricpascal opened 1 year ago
I think the issue is not so much with how the headings are computed by the GovukTechDocs::UniqueIdentifierGenerator
as with where the cache of existing headings is reset.
At the moment it is done by GovukTechDocs::UniqueIdentifierExtension
. Haven't quite figured out when exactly it triggers. The docs mention "before Rack requests" plural. I may be misunderstanding, but I think it's not "once before each request to generate the content of a page". It does trigger multiple times when loading a page though[^1].
That said, I think it the reset of the generator should be tied more closely to the markdown generation, using Redcarpet's preprocess hook instead of being tied to the overall Middleman lifecycle. This would limit the risks of leakage.
Bluntly, this could be done via:
# lib/govuk_tech_docs/tech_docs_html_renderer.rb
def preprocess(document)
UniqueIdentifierGenerator.instance.reset
return document
end
However, given that would put the ID generation all in one place, this means we could stop GovukTechDocs::UniqueIdentifierGenerator
from being a singleton and just instanciate a new one each time we pre-process. Or at least store the instance in the renderer rather than having it as a global singleton.
[^1]: Investigated through editing the files in ~/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/govuk_tech_docs-3.1.0
and logging when things got reset with pp
calls
What should change
The computation of unique IDs for the links in the navigation gives the wrong results when:
The link points to an ID composed of the heading and the previous heading (for ex.
/javascript-api-reference/#javascript-api-reference-button
), as if the target was on the current page. It should instead be only the heading (in the example/javascript-api-reference/#button
) as on the other page, the ID is unique.To reproduce:
npm install
,bundle install
bundle exec middleman server
#javascript-api-reference-button
, while when browsing the other pages, it points to#button
User need
When publishing the latest release of the design system, we had our JavaScript API Reference and Sass API Reference both list the Button component, leading to the issue described. A situation that may happen further as both docs get expanded and list the options for our components. Other documentations with repetitive structure/content across pages may run into this issue as well.