Skip to content

Conversation

kfranqueiro
Copy link
Contributor

@kfranqueiro kfranqueiro commented Aug 8, 2025

Fixes #4425.

This adds IDs to headings that aren't the first under their respective parent section, and relies on @sindresorhus/slugify to produce headings, with counters when duplicate labels exist. (This library is an existing dependency of Eleventy, so it does not increase npm install size.)

This updates all generated IDs to use slugify instead of the previous logic that had been ported from the XSLT process. This means some subsection IDs may change; it also means some which were previously inaccessible due to duplicate IDs will be fixed. None of the standard level-2 headings should be impacted.

See list of diffs for existing IDs; any case where an appended number is the only change is fixing a duplicate (i.e. previously unreachable) ID.

I performed extra testing to check for headings that might've been overlooked by the selector I used here, but all of the first-child cases involve headings under sections which will already have IDs.

Copy link

netlify bot commented Aug 8, 2025

Deploy Preview for wcag2 ready!

Name Link
🔨 Latest commit 3d40f5c
🔍 Latest deploy log https://app.netlify.com/projects/wcag2/deploys/68a34bb93074ad00084bb0e1
😎 Deploy Preview https://deploy-preview-4523--wcag2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@kfranqueiro
Copy link
Contributor Author

kfranqueiro commented Aug 8, 2025

Concerns raised on Backlog TF call with regard to changing existing IDs. Possible resolutions:

  • Add extra elements with old IDs for backwards-compatibility
    • Downside: Clutters the DOM and adds build system complexity
  • Use JavaScript to map old IDs to new IDs
    • Downside: Adds client-side scripting and won't work if it's disabled; also adds build system complexity
  • Don't change any existing IDs
    • Downside: Existing duplicate IDs remain broken
  • Patch the existing approach to deal with duplicates while preserving the rest of its flaws
    • Downside: Will still produce some rather unwieldy IDs for some instances

List of diffs caused by switching algorithm for existing IDs

Bear in mind that any diffs where the new has appended a hyphen and number, e.g. -2, are fixes where there was previously a duplicated ID that wouldn't be reachable via URL hash.

Reminder: the worst-case scenario if we don't preserve existing IDs is that links go to the top of the same page.

@iadawn
Copy link
Contributor

iadawn commented Aug 13, 2025

I am strongly against doing nothing. There is no consistency in how these IDs are generated which is a poor situation.

The point that this does not result in a 404 is an important one.

If the use of these IDs is hypothetical then I think we should make the change and move on.

@kfranqueiro kfranqueiro requested a review from iadawn August 13, 2025 15:26
@bruce-usab
Copy link
Contributor

Revisited on backlog call 8/15. Keeping in Drafted.

@kfranqueiro
Copy link
Contributor Author

kfranqueiro commented Aug 18, 2025

Dan suggested I survey existing links within the repo to check if any would be affected by the output diff that I linked to above.

None of the affected subsections were ever linked to internally. (I suspect that if they had been, the issues with those IDs might've been addressed far earlier.)

However, I did find and fix a couple of section links that had been broken in the Understanding Techniques page, not only prior to this PR (which does not change those sections' IDs), but also prior to the entire Eleventy build system (because this build system was initially coded to be consistent with the previous XSLT build system behavior.)

@kfranqueiro
Copy link
Contributor Author

This was discussed one final time on the August 22 Backlog TF call and no one had any remaining reservations. I hadn't been sure which column to move it to; moving it to Ready for Approval now so we don't unnecessarily re-discuss it.

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

Successfully merging this pull request may close these issues.

Add id to all headings and link them from "Page Contents" in understanding docs
4 participants