Feat: Add aria-describedby, aria-roledescription #3808

weedySeaDragon · 2022-11-18T00:46:05Z

📑 Summary

Resolves #3626

Added the aria-roledescription to be the diagram key. (In the future we probably want to revise this to use i18n)
Added aria-describedby : the accessible description should describe the diagram, not label it.

Updated the documentation:

reworked the Accessibility section.
- fixed the example for multi-line accessible descriptions. (No ":" goes after the keyword)
updated the "Adding a new diagram" info on accessibility

📏 Design Decisions

All a11y is now handled by mermaidAPI render() instead of being the responsibility of every individual diagram. Individual diagrams still need to parse the accessible title and description and provide the values.
The d3 mock now returns MockedD3 objects. Only the sequence diagram used the specialized NewD3 and MockD3 versions. I moved those specific mocks into the appropriate sequence tests.
Added a mermaidAPI render() spec -- it only tests the a11y stuff, but is now set up for adding other examples & tests in the future.
Added mock diagram renderers. Needed for writing the mermaidAPI render tests.

📋 Tasks

Make sure you

📖 have read the contribution guidelines
💻 have added unit/e2e tests (if appropriate)
🔖 targeted develop branch

…ption')

…ledescription-for-svg

…ers etc)

aloisklink

Looks good to me, although I'm not an expert on the aria accessibility tags, so please take what I'm saying with a pinch of salt! Some minor comments, but they're minor, and you're welcome to disagree with them/or I might be wrong about them.

aloisklink · 2022-11-20T16:54:29Z

docs/config/accessibility.md


-Let us take a look at the following example with a flowchart diagram:
+Support for accessible titles and descriptions is available for all diagrams/chart types. We have tried to keep the same keywords and format for all diagrams so that it is easy to understand and maintain.


🚀

If/when PR #3706 gets merged, we should hopefully be able to add a more consistent method of settings these in the diagram front-matter too!

That will be great.

Now that #3706 is merged, do we need to revisit anything here?

Not yet, as it currently only supports the title: keyword in YAML front-matter.

It should be relatively easy to add a accTitle: and accDescr: in another PR though, although we should probably wait until this PR is merged first to avoid merge conflicts.

docs/config/accessibility.md

packages/mermaid/src/accessibility.spec.ts

packages/mermaid/src/mermaidAPI.ts

Co-authored-by: Alois Klink <alois@aloisklink.com>

…scBy-roledescription-mocks

aloisklink

Looks good from my side, but again, I'm not an expert on aria accessibility tags, so another reviewer might catch things I've missed.

Thanks for keeping this up to date with develop, and for adding more tests!

packages/mermaid/src/utils.spec.js

…scBy-roledescription-mocks

weedySeaDragon · 2022-11-30T05:49:52Z

Who else needs to review this so that it can be merged?

aloisklink · 2022-11-30T20:08:42Z

Who else needs to review this so that it can be merged?

Rules are here: https://github.com/mermaid-js/mermaid/blob/develop/CONTRIBUTING.md#committing-code

But generally, @knsv is the main reviewer (or they assign a reviewer) for anything that changes the output mermaid diagrams. But they tend to be pretty busy, so it might take a couple of days to get a review (especially since this PR looks like a big one with lots of commits, since even though the actual code changes are pretty small, there's a lot of great tests + documentation added!)

My front-end/accessibility knowledge is pretty weak, so there's a good chance I've missed something!

weedySeaDragon · 2022-12-01T17:28:40Z

Thanks for the response, @aloisklink . :-)

@brennanyoung, who entered the issue that this PR addresses (#3626 ) does know a lot about accessibility, so that area doesn't require much review.

And generally, the # of commits is not terribly related to the size of the actual changes. (Some project choose to squash or reduce them to a bare minimum even before submitting PRs.) Looking directly at the file changes is the indicator.

Since this has minimal effect on output, I didn't know if @knsv needed to review this or not.

…scBy-roledescription-mocks # Conflicts: # cypress/integration/rendering/gantt.spec.js # cypress/integration/rendering/requirement.spec.js # docs/config/setup/modules/mermaidAPI.md # packages/mermaid/src/accessibility.js

…scBy-roledescription-mocks

knsv

Another good one @weedySeaDragon. We should make sure it works with lazy loaded diagrams like mindmaps as well.

knsv · 2022-12-07T10:29:01Z

packages/mermaid/src/mermaidAPI.ts

@@ -514,6 +517,13 @@ const render = function (
    throw e;
  }

+  // This is the d3 node for the svg element


Good stuff. Found an issue here though as this is only catering the render flow and not the renderAsync flow. See below. This needs top be done in renderAsync (around row 727 as well)

knsv

Is it possible for you to look at the renderAsync as well?

weedySeaDragon · 2022-12-07T14:50:04Z

I'll definitely look at renderAsync too.

…scBy-roledescription-mocks

weedySeaDragon · 2022-12-07T18:47:03Z

@sidharthv96 FYI - Link checker failed because atom.io no longer has any entries for mermaid items. (Don't know if there's a problem on atom.io or if those packages have been removed.)

sidharthv96 · 2022-12-08T05:28:01Z

It was a false positive, 500 error.
The package seems to be there.

knsv

Thanks @weedySeaDragon will merge now!

weedySeaDragon added 16 commits November 15, 2022 13:47

accessibility.js -> ts; + set aria-roledescription; add spec

8d96518

diagrams: use a11y title,desc specific method (was renamed)

0d9566d

add "roledescription" to cSpell list of words (as in 'aria-roledescri…

d997076

…ption')

Merge remote-tracking branch 'MERMAID/develop' into feat/3626-aria-ro…

c331305

…ledescription-for-svg

(minor) fix typo

03a11e1

add error checking (empty diagramType, title, desc) to a11y methods

4d7496b

MockedD3: node() return Element; add selectAll()

8a3c4f6

move mocks specific to only seq spec files out of global d3 mock

1fc0294

d3 mock: use MockedD3; remove sequence specific mock code

1ad537b

add diagram renderer mocks

f62c5d9

put a11y into mermaidAPI render; add render spec (mock diagram render…

29efc11

…ers etc)

remove a11y from individual diagrams; now happens in mermaidAPI render

0adc6a6

doc: revise A11y: fix multi-line ex, +describedby, alpha sort examples

4fb4aa4

doc: adding diagrams: revise a11y section

9cc862b

(minor) fix typo, whitespace formatting

68b1805

set describeby to accessible description element id

b51759d

github-actions bot added the feature label Nov 18, 2022

weedySeaDragon added 2 commits November 17, 2022 16:51

format .md files

6952640

re-re-fixed the contributing doc

67a015c

aloisklink approved these changes Nov 20, 2022

View reviewed changes

weedySeaDragon and others added 2 commits November 20, 2022 12:17

use optional chaining check for get acc title and get acc description

2a98791

Co-authored-by: Alois Klink <alois@aloisklink.com>

export D3Element from mermaidAPI; use in accessibility

a9c3373

aloisklink mentioned this pull request Nov 22, 2022

Server Side Support #3650

Open

weedySeaDragon added 5 commits November 25, 2022 11:30

Merge remote-tracking branch 'MERMAID/develop' into feat/3626-aria-de…

626a474

…scBy-roledescription-mocks

(minor) fix comment, comment typo

7508cd7

use MockedD3, spies in util insertTitle spec (remove MockD3)

f1bc2de

add test for multi-line accDescr

6e486d3

update /docs

2030885

weedySeaDragon requested a review from aloisklink November 27, 2022 19:42

aloisklink approved these changes Nov 27, 2022

View reviewed changes

packages/mermaid/src/utils.spec.js Outdated Show resolved Hide resolved

packages/mermaid/src/utils.spec.js Outdated Show resolved Hide resolved

weedySeaDragon added 3 commits November 27, 2022 19:10

Merge remote-tracking branch 'MERMAID/develop' into feat/3626-aria-de…

e9d4372

…scBy-roledescription-mocks

make test title clearer

6044e9e

use camelCase

2bf753a

weedySeaDragon added 3 commits December 1, 2022 10:08

remove typeof

bfe3f30

Merge remote-tracking branch 'MERMAID/develop' into feat/3626-aria-de…

96996d0

…scBy-roledescription-mocks

sidharthv96 approved these changes Dec 7, 2022

View reviewed changes

sidharthv96 requested a review from knsv December 7, 2022 06:25

sidharthv96 assigned weedySeaDragon Dec 7, 2022

knsv reviewed Dec 7, 2022

View reviewed changes

knsv requested changes Dec 7, 2022

View reviewed changes

weedySeaDragon added 2 commits December 7, 2022 09:31

Merge remote-tracking branch 'MERMAID/develop' into feat/3626-aria-de…

b1dd000

…scBy-roledescription-mocks

common function for a11y; add to renderAsync; + renderAsync spec

1c9a559

weedySeaDragon requested a review from knsv December 7, 2022 18:47

knsv approved these changes Dec 8, 2022

View reviewed changes

knsv merged commit 90d9724 into mermaid-js:develop Dec 8, 2022

aloisklink mentioned this pull request Dec 8, 2022

Update accessibility.md #3895

Closed

3 tasks

aloisklink mentioned this pull request Nov 16, 2023

feat: Add packet diagram #4839

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Add aria-describedby, aria-roledescription #3808

Feat: Add aria-describedby, aria-roledescription #3808

weedySeaDragon commented Nov 18, 2022

aloisklink left a comment •

edited

Loading

aloisklink Nov 20, 2022

weedySeaDragon Nov 20, 2022

sidharthv96 Dec 7, 2022

aloisklink Dec 7, 2022

aloisklink left a comment

weedySeaDragon commented Nov 30, 2022

aloisklink commented Nov 30, 2022

weedySeaDragon commented Dec 1, 2022

knsv left a comment

knsv Dec 7, 2022 •

edited

Loading

knsv left a comment

weedySeaDragon commented Dec 7, 2022

weedySeaDragon commented Dec 7, 2022

sidharthv96 commented Dec 8, 2022

knsv left a comment


		Let us take a look at the following example with a flowchart diagram:
		Support for accessible titles and descriptions is available for all diagrams/chart types. We have tried to keep the same keywords and format for all diagrams so that it is easy to understand and maintain.

Feat: Add aria-describedby, aria-roledescription #3808

Feat: Add aria-describedby, aria-roledescription #3808

Conversation

weedySeaDragon commented Nov 18, 2022

📑 Summary

📏 Design Decisions

📋 Tasks

aloisklink left a comment • edited Loading

Choose a reason for hiding this comment

aloisklink Nov 20, 2022

Choose a reason for hiding this comment

weedySeaDragon Nov 20, 2022

Choose a reason for hiding this comment

sidharthv96 Dec 7, 2022

Choose a reason for hiding this comment

aloisklink Dec 7, 2022

Choose a reason for hiding this comment

aloisklink left a comment

Choose a reason for hiding this comment

weedySeaDragon commented Nov 30, 2022

aloisklink commented Nov 30, 2022

weedySeaDragon commented Dec 1, 2022

knsv left a comment

Choose a reason for hiding this comment

knsv Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

knsv left a comment

Choose a reason for hiding this comment

weedySeaDragon commented Dec 7, 2022

weedySeaDragon commented Dec 7, 2022

sidharthv96 commented Dec 8, 2022

knsv left a comment

Choose a reason for hiding this comment

aloisklink left a comment •

edited

Loading

knsv Dec 7, 2022 •

edited

Loading