-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More Accessible Mermaid Charts #2732
Comments
The syntax for this should be the same in all diagram types |
I want to add some more context to this issue. Using the following flow chart: flowchart TD;
A[Using a flow<br>chart in GitHub] --> B{Must it be<br>accessible?};
B -- No --> C[Wrong,<br>try again];
B -- Yes --> D{Into what format<br>does it render?};
D -- JPG/PNG/GIF --> E{Does it have<br>an `alt`?};
E -- No --> V[Wrong,<br>try again];
E -- Yes, and it<br>is lengthy --> V[Wrong,<br>try again];
E -- Yes, a<br>brief one --> F[Manually add<br>a plain text<br>description<br>of the chart.];
D -- SVG --> G{What `role`<br>does it have?};
G -- `none` --> N[Wrong,<br>try again];
G -- `table` --> N[Wrong,<br>try again];
G -- `grid` --> N[Wrong,<br>try again];
G -- `presentation` --> Y[Manually add<br>a plain text<br>description<br>of the chart.];
G -- `image` --> H{Does it have<br>an `aria-label`?};
H -- No --> P[Wrong,<br>try again];
H -- Yes, and<br>it is lengthy --> P[Wrong,<br>try again];
H -- Yes, a<br>brief one --> Z[Manually add<br>a plain text<br>description<br>of the chart.];
G -- `tree` --> I{Does it contain<br>any `treeitem`<br>roles for the<br>text nodes?};
I -- No --> R[Wrong,<br>try again];
I -- Yes --> J{Are they contained<br>in / owned by `group`<br>roles beyond the<br>first level?};
J -- No --> S[Wrong,<br>try again];
J -- Yes --> K{Have you<br>tested in<br>any screen<br>reader?};
K -- No --> T[Wrong,<br>try again];
K -- Yes --> L{Did it accurately<br>convey relationships<br>and structure from<br>the chart?};
L -- No --> U[Wrong,<br>try again];
L -- Yes --> M[You should write a<br>CSUN talk about it];
Screen readers announce it as nothing but unordered run-on text. There are no relationships, no structure, nothing to give context to the chart. It lacks necessary roles and properties as well. Here is the output directly from the speech viewer of JAWS 2021 with Chrome (the line breaks are from the
Here is output from the speech viewer of NVDA 2021.3 with Firefox (same about the line breaks):
The announcement is a bit different than what is shown here (every ` announces as "grav", for example). As you read the preceding text, I am sure you can understand how that conveys nothing of the meaning of the chart. Separately, the answer options in dark mode ("Yes", "No", "JPG/PNG/GIF", etc) all fail WCAG contrast checks. I did not check outside of dark mode. The Firefox accessibility inspector and dev tools identify these failures readily. Finally, this sentence from the GitHub announcement reveals a complete misunderstanding of how screen readers parse content (they work fine with JS, but the raw MarkDown is just as meaningless as the text I pasted):
Some reference material for working on making the SVGs more accessible:
|
Ok I started playing with a pr to understand some of the decisions that will need discussion. So far I think that the path of using A few charts already have a title syntax, I think we add the title to all charts most charts would likely benefit from its visual inclusion as well. This can be used as the label. Then we need a description. Adding a description syntax to the chart is a bit weird from the perspective of a non-screen reader user, in this I mean that I think most people would expect it to be displayed. Given this syntax
I would personally expect description to be rendered in the graph like every other language feature. The puzzle here is what to call this field. Its seems natural to me to have it be |
I assume these properties you are naming are something to be added in the MarkDown configuration syntax. If so, perhaps…
The The The terms So my example might start as:
It might render an SVG that looks like: <svg xmlns="http://www.w3.org/2000/svg" aria-labelledby="T1" aria-describedby="D1" …>
<title id="T1">GitHub format decision tree.</title>
<desc id="D1">A series of questions walking through establishing first the file
format, then the accessible name/description, and finally the appropriate
properties to use for creating an accessible flow chart.</desc>
…
</svg> Despite all this, since many of the charts that can be generated do not have the relatively simple narrative flow of the one I added above, I still recommend an auto-generated plain text equivalent alongside the chart. Especially when the chart is small, the contrast is poor, or rendering breaks. For example, the alternative text I wrote for the chart above:
As you can see, that is a bit verbose for a plain text description and lacks the nuance and structure you could represent with things like lists. |
Thanks @aardrian
I am tempted to just re-use the title, it is already on some charts and I think that if the element is actually used for something that is used for multiple purposes like the visual title of the chart it is likely to be added more frequently then something that is just for accessibility, I frequently see images without alt text.
I went with accDescription on my pr, thanks for the suggestion.
I generally agree, it would be good to move this issue first as an enhancement and not get stuck on the creation of such an automated feature, that would be a great follow up enhancement. Maybe open another issue? |
That's fine for moving this along. Bear in mind that what authors think of as a good title is not always appropriate as an image alternative or accessible name. I say this because down the road you may want to allow authors to add / override / supplement the title to provide something that is a better fit.
thumbs up emoji
Agreed it is out of scope for a quick hit for which this issue is asking. |
@gwincr11 I took a quick look, but it has been merged and I really need to see the SVG output to know if all the right bits end up in all the right spots. That being said, nothing jumped out at me. |
Heads up, this ticket is also an A11Y issue: Just to tie it all together. |
It is frustrating that the bug report was opened after another bug report covering the same issue was closed due to inactivity from the project maintainers. The person who closed it also locked it from follow-up comments or re-opening. Bugs don't stop being bugs because you ignore them. Though it was oddly marked as a feature enhancement instead of a bug. |
Thanks @lindseywild @el-mapache @khiga8 and @therzka! All the charts now have the accesiblity features added as outlined in this issue! |
I missed that this was closed. The Live Editor does not appear to support the new Are any of those on the radar? |
The work to add accDescription and title to all charts is now on the development branch and should go out with the next release. I am not sure when or how the live editor gets updated but once they update to the new version it should be there.
This was not in scope for this issue, as it was meant to address title and description elements. I would suggest moving that particular issue out into its own issue, thank you ❤️ for keeping an eye on it. A number of new issues have been opened as well: |
Hi guys! This one is trickier then you would think... During regression tests we found that this causes issues and will affect existing diagrams out there. Take for instance this flowchart: flowchart TB
a[I am the title]
This will break due the title keyword appearing in a node text. I suggest we return to the initial suggestion of using accName and accDesc as keywords:
We can still keep the same rendering though. I will take a look and adjust the existing work. |
An update... The description will have these two forms:
or
|
👋 it would be nice to work towards more accessible features now that this library is in much wider use. I think a couple of low hanging fruit items would be to include
<title>
elements in the output svg and<description>
so that people creating charts can offer text explanations of the charts.There is lots of other work too be done in this area, but this seems like a simple place to start.
Charts to be completed
:UPDATE 2-22-2022 (Twosday):
Ok I have a pr open with a new accessibility function that can add a title and a description to a chart. I set it up for Pie charts as they already had a title.
I am probably a blocker on this issue since I have been dealing with a bunch of family things and do not want to prevent someone else from picking this up. Feel free to either merge my pr and then add other charts or start work in some other way. If you would like to merge my pr and something needs to be cleaned up I will happily do it. cc @knsv
The text was updated successfully, but these errors were encountered: