fix(theme): add accessible name for the heading hash-link #8562

zmrhaljiri · 2023-01-19T13:20:48Z

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
If this is a new API or substantial change: the PR has an accompanying issue (closes Inaccessible markdown heading links #8336) and the maintainers have approved on my working plan.

Motivation

An accessibility improvement - anchor elements must have an accessible name - a programmatically determinable name that comes either from the text content inside, from aria-label, or aria-labelledby.

There is no reliable way to hide the textual contents of pseudoelements from screen readers, would be better to implement the link text as <span aria-hidden="true">#</span> but not sure if that would be wanted.

Test Plan

Before - the accessible name of the link is not meaningful:

After - the accessible name of the link is meaningful and unique:

Test links

Deploy preview: https://deploy-preview-8562--docusaurus-2.netlify.app/docs/installation/#requirements

Related issues/PRs

#8336

Anchor elements must have an accessible name - a programmatically determinable name that comes either from the text content inside, from `aria-label`, or `aria-labelledby`.

netlify · 2023-01-19T13:25:04Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`55d0f50`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63cb068ebaa59c0008c1232f
😎 Deploy Preview	https://deploy-preview-8562--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2023-01-19T13:25:36Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 84	🟢 97	🟢 92	🟢 100	🟢 90	Report
/docs/installation	🟠 79	🟢 100	🟢 100	🟢 100	🟢 90	Report

slorber · 2023-01-19T13:29:08Z

packages/docusaurus-theme-classic/src/theme/Heading/index.tsx

      <Link
        className="hash-link"
        to={`#${id}`}
+        aria-labelledby={id}


wouldn't it be better to use children instead of id there?

(ie "Project Structure" instead of "project-structure")

Ids are not necessarily as human readable as the original text of the heading

aria-labelledby accepts the id of an element containing some text. And because the heading contains e.g. "Project structure" text, it becomes the accessible name of the link as well.

But if you are mentioning children, it would be even better to do this: aria-label={`Direct link to ${children}`}. I just was not sure about TypeScript here (not an expert). Will children always be only text content or it's possible there will be some tags too?

hmmm right it might not always be a string

slorber · 2023-01-19T13:32:33Z

packages/docusaurus-theme-classic/src/theme/Heading/index.tsx

+        aria-labelledby={id}
        title={translate({
          id: 'theme.common.headingLinkTitle',
          message: 'Direct link to heading',


Don't we want to update this to Direct link to {headingTitle}?

I think this was what we wanted in the original issue? (this would require modifying all our translation files to add the extra param/placeholder)

We can, and I think it would be great if we could do this:

<Link aria-label={`Direct link to ${headingTitle}`} // to set the accessible name title={`Direct link to ${headingTitle}`} // this will be shown to users who hover over the link />

(just with that translation syntax)

👍 let me edit the PR I'll do this

slorber

Did my best to update all the locales

It's likely that there are some mistakes. I'll merge the pr asap so if there are mistakes open a new PR to fix issues.

slorber · 2023-01-20T12:24:23Z

packages/docusaurus-theme-translations/locales/zh-Hans/theme-common.json

  "theme.colorToggle.ariaLabel.mode.light": "浅色模式",
  "theme.common.editThisPage": "编辑此页",
-  "theme.common.headingLinkTitle": "标题的直接链接",
+  "theme.common.headingLinkTitle": "标题的直接 {heading}",


@Josh-Cena ?

Have no idea if I should add a space or not 🤷‍♂️

Suggested change

"theme.common.headingLinkTitle": "标题的直接 {heading}",

"theme.common.headingLinkTitle": "{heading}的直接链接",

Whether to add a space or not is a hard question. Since we should be able to insert Chinese text here, I suggest no—but if heading is always Latin, then my own preference is yes.

packages/docusaurus-theme-translations/locales/zh-Hant/theme-common.json

zmrhaljiri · 2023-01-20T21:20:11Z

Did my best to update all the locales

I can confirm the Czech locale is fine :)

…ommon.json

Josh-Cena · 2023-01-26T15:34:50Z

@slorber Looks like #8562 (comment) is missed. I won't be able to get to my computer until a couple of hours later, so it would be great if you can fix that earlier in case I forget.

slorber · 2023-02-01T16:36:33Z

@slorber Looks like #8562 (comment) is missed. I won't be able to get to my computer until a couple of hours later, so it would be great if you can fix that earlier in case I forget.

What do you mean by it's missed?

I'd prefer if you fix it because it's hard for me to figure out what is the right final solution 😅

Josh-Cena · 2023-02-01T16:43:51Z

Oops, I didn't see the suggestion was already applied.

fix: Add accessible name for the hash-link

4ec866c

Anchor elements must have an accessible name - a programmatically determinable name that comes either from the text content inside, from `aria-label`, or `aria-labelledby`.

zmrhaljiri requested review from Josh-Cena, lex111 and slorber as code owners January 19, 2023 13:20

facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 19, 2023

slorber reviewed Jan 19, 2023

View reviewed changes

slorber added 3 commits January 20, 2023 12:53

Merge branch 'main' into patch-1

c631341

add heading param to theme.common.headingLinkTitle translation

d0662cd

add heading param to theme.common.headingLinkTitle translation

f2b230d

slorber approved these changes Jan 20, 2023

View reviewed changes

slorber added pr: bug fix This PR fixes a bug in a past release. to backport This PR is planned to be backported to a stable version of Docusaurus labels Jan 20, 2023

slorber changed the title ~~fix: Add accessible name for the hash-link~~ fix(theme): Add accessible name for the heading hash-link Jan 20, 2023

slorber added the domain: a11y Related to accessibility concerns of the default theme label Jan 20, 2023

Josh-Cena changed the title ~~fix(theme): Add accessible name for the heading hash-link~~ fix(theme): add accessible name for the heading hash-link Jan 20, 2023

Update packages/docusaurus-theme-translations/locales/zh-Hans/theme-c…

55d0f50

…ommon.json

slorber merged commit d8459e5 into facebook:main Jan 26, 2023

slorber pushed a commit that referenced this pull request Jan 26, 2023

fix(theme): add accessible name for the heading hash-link #8562

63ec772

zmrhaljiri deleted the patch-1 branch January 26, 2023 22:09

slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Jan 27, 2023

slorber mentioned this pull request Oct 19, 2023

chore: v3.0.0-rc.0 release #9418

Merged

	"theme.common.headingLinkTitle": "标题的直接 {heading}",
	"theme.common.headingLinkTitle": "{heading}的直接链接",

Uh oh!

fix(theme): add accessible name for the heading hash-link #8562

fix(theme): add accessible name for the heading hash-link #8562

Uh oh!

Conversation

zmrhaljiri commented Jan 19, 2023 • edited by Josh-Cena Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-flight checklist

Motivation

Test Plan

Test links

Related issues/PRs

Uh oh!

netlify bot commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ [V2]

Uh oh!

github-actions bot commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚡️ Lighthouse report for the deploy preview of this PR

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

zmrhaljiri Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

zmrhaljiri Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

slorber Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Josh-Cena Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zmrhaljiri commented Jan 20, 2023

Uh oh!

Josh-Cena commented Jan 26, 2023

Uh oh!

slorber commented Feb 1, 2023

Uh oh!

Josh-Cena commented Feb 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zmrhaljiri commented Jan 19, 2023 •

edited by Josh-Cena

Loading

netlify bot commented Jan 19, 2023 •

edited

Loading

github-actions bot commented Jan 19, 2023 •

edited

Loading

zmrhaljiri Jan 19, 2023 •

edited

Loading