docs: fix heading level a11y issue

[geoffrich]

This resolves one of the Axe Accessibility violations listed in #5678

• Heading levels should only increase by one (1 instance). The "Component Format" section of the docs skipped from an h3 to an h5. I changed the h5s to an h4.
• Ensure that scrollable region has keyboard access (113 instances). The code blocks were scrollable but were not keyboard-focusable. This meant that a user unable to use a mouse would be unable to scroll the code blocks and content could be obscured. Since this PR is becoming more complex, I'll be submitting these changes in a separate PR.

A side effect of fixing the heading levels is that additional items appear in the Table of Contents, presumably since h4s are shown in the TOC by default but h5s are not.

Before:

After:

Also, export and $ are no longer highlighted in the affected headings due to differences in h4/h5 styling.

Before:

After:

Please let me know if either of these prevent merging this PR.

[geoffrich]

Looks like there were some (unrelated) test timeouts -- any way to re-run those?

[tanhauhau]

Actually, the heading levels was intentional see: #2390.

[geoffrich]

I've removed the scrollable region a11y issue from this PR because this change is becoming more complex. I'll submit a fix for that in a separate PR.

[geoffrich]

I managed to get the offending headings removed from the TOC, but I had to add some custom markdown parsing logic to do it.

Since there was no existing way to distinguish between level-4 headings we wanted to show up in the TOC and ones that we didn't, I added custom attribute parsing similar to Pandoc.

Given this line of markdown:

#### This is a heading { .red #important data-something=true }

The markdown renderer converts it to the following HTML.

<h4 class="red" id="important" data-something="true">This is a heading</h4>

During markdown processing, we are then able to determine whether or not to add a subsection based on the presence of the new data-toc-ignore attribute.

I only attempt attribute extraction on headings for now, but it could be theoretically applied to other elements if desired. One caveat is that the Svelte block syntax (e.g. {#if}) could be interpreted as attributes if they're used in headings. However, the existing block ({#if}, {#await}, etc) headings currently fail the regex I'm using to detect attributes because there is no content before the opening bracket. This mitigates the issue for now, but we could implement a blocklist of reserved words if necessary.

I'm willing to write tests for the new attribute extraction function if desired, but it doesn't seem like the site has very many tests at the moment. I was unable to run the site-specific tests and it doesn't look like they're being automatically run in a pipeline anywhere.

[pngwn]

This change to the markdown parsing won't be compatible when we we migrate to kit and the new docs handling, we could add this {} attribute stuff to the parser but it is complex enough as it is and I'd rather we solved it in a different way.

Could we not just make those first set of links with heading level 4 a special case and not render them? I don't think we need this complexity in the parser for what is a very niche issue that occurs exactly once (and that should be solved by having better headings imo). Our custom markdown syntax is challenging enough to figure out for contributors as it is, I'm opposed to making it even worse.

[benmccann]

marking this as changes requested instead of approved

[geoffrich]

@pngwn that makes sense, thanks for the review. I agree that the new {} syntax makes things too complicated.

Instead of looking for data-toc-ignore would it make sense to check the slug here instead and not add a subsection if it's one of those first h4s? Or did you want the logic in a different file entirely?

[pngwn]

This is a bit awkward which isn't completely unexpected. I think I imagined this would go in the actual component {#if index !== 0} and then render the nested each if that condition is met. But this is problematic because this logic is here and this Docs component, and the GuideContents component within, is used by other documentation websites. This would break anything else that used it (such as the kit docs).

It wouldn't actually break them now because the site-kit has been brought into the sites monorepo (here) and these two version of site-kit are independent. But they'll eventually use the same components, so this wouldn't only kick the can down the road.

Modifying the parser as you suggested is one solution, but that will have the same problem when we migrate, because the new parser is shared by all documentation sites.

There are two options: either we add some custom logic for the svelte docs specifically (either in the parser to avoid generating subsections or in the component to prevent rendering for that section) or we figure out a way to add these subsections in the side navigation without breaking the layout (making the nav structure consistent for all sections).

[benmccann]

Rather than hide the headings, it might be nice to show a short version of them in side sidebar (like in #6480)

[geoffrich]

If we want to show a short version in the sidebar, we could add logic inside the /docs route on svelte.dev to change the title of those sections before rendering:

svelte/site/src/routes/docs/index.svelte

Lines 2 to 5 in a6fbf7f

	export async function preload() {
	const sections = await this.fetch(`docs.json`).then(r => r.json());
	return { sections };
	}

Since slugs will be changing eventually, we could map original section titles to a short version, e.g. 2. Assignments are 'reactive' => reactive assignments. Then the shorter title will be the one used in the TOC. That's still a little brittle but I can't think of a better way that doesn't involve complicating the markdown.

Not having to hide anything from the table of contents means we shouldn't have to do any additional markdown processing.

[benmccann]

this seems a bit hard to discover to me compared to if it were in the markdown. though I suppose I'm slightly less worried about that because you could see something happening in the sidebar. but it might be tough to trace it back to this

[geoffrich]

Yeah, it's not the most obvious place. However, I don't think there's a way to have it in the markdown without implementing custom parsing logic.

At least the effect of this breaking is small -- the sidebar titles would use the section titles instead of the abbreviated version.

[pngwn]

I think this is a pretty reasonable place to have it: it is specific to this view rather than an important part of the docs.

The only other place it could go (without implementing custom parsing logic) is in the Docs component in site-kit, which would be very difficult to discover. One alternative could be add it to the YAML frontmatter as a map of full -> short heading titles, would require a little work but doable.

I think this is good for now.

[geoffrich]

I've removed the special markdown parsing logic per the feedback above. Instead, the preload function in the /docs endpoint replaces some subsection titles with an abbreviated version for the Table of Contents. These modified sections are then passed to the <Docs> component. It uses a list of slugs to know what to replace.

One downside with this is that the title will not be replaced if the slug changes. If that happens unexpectedly, the longer titles will start appearing in the TOC again. I couldn't think of a better key to match on, but let me know if you have any alternatives.