Skip to content
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

ariaLabel, ariaDescription, ariaHidden #710

Merged
merged 3 commits into from
Jan 28, 2022
Merged

ariaLabel, ariaDescription, ariaHidden #710

merged 3 commits into from
Jan 28, 2022

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Jan 26, 2022

This adds:

  • an optional top-level aria-label attribute for the plot
  • an optional top-level aria-description attribute for the plot
  • automatic aria-label attributes for all marks and axes (e.g., aria-label="bar")
  • an optional per-element aria-label attribute channel for marks
  • an optional singular aria-description attribute for marks (e.g., “a set of bars showing a normal distribution”)
  • an optional singular aria-hidden attribute for marks

I am not sure whether we should be using the aria-description attribute or SVG’s desc element. I chose the latter the aria-description attribute somewhat arbitrarily. I primarily consulted Doug Schepers’ recommendations:

https://blog.tenon.io/accessible-charts-with-aria/

I did not add role attributes since I believe that the implicit role for SVG is already graphics-document, and I don’t know what else to do.

I went back and forth several times on whether these options should be called “ariaLabel” and “ariaDescription” or simply “label” and “description”. The former is more technically precise (although we are using the SVG desc element instead of the aria-description attribute), while the latter is perhaps “friendlier” and less to type. I felt the technical/precise names would be less likely to conflict in the future if a mark wants a visible label. Also it feels more “webbish”, I guess.

Fixes #361, though there is much more that we need to do improve accessibility.

@mbostock mbostock requested a review from Fil January 26, 2022 21:50
@Fil
Copy link
Contributor

Fil commented Jan 27, 2022

Important, and important to get it right. For now I find it quite difficult to evaluate this PR, as I lack a tool that would help me "visualize" the tree of text/navigation elements that screen readers see in the code, and ideally compare it against a desired state. Maybe we should write one…

The only tool I know that works for this purpose is ARIA Devtools —which is not working in Observable's iframes, but works on a javascript-type embed (and on the local dev server). However, the code doesn't seem enough to indicate to this extension that a group of labelled dots or bars is available for browsing. Maybe because this extension's support for SVG is incomplete (it considers role=graphics-document as an Image).

FWIW here's what's shown with ARIA Devtools:
aaplBollinger
letterFrequencyBar

One thing that confuses me in the PR is the option named ariaDescription; if it is rendered as a <desc> element, I think it should be called desc. The mark's common <desc> is read out loud by ChromeVox, but apparently not by Apple's VoiceOver (or quite possibly, I'm not using VoiceOver correctly).

Using the ChromeVox screen reader extension, I realized that adding a title or an ariaLabel on a scatterplot's dots yields the same result (under that particular screen reader). When both are set, aria-label has priority, ie the title is then ignored.

I'd like to see a demonstrably working example; does this imply having to add a keyboard navigation script (as in the visa chart components), or something else?

@mbostock
Copy link
Member Author

One thing that confuses me in the PR is the option named ariaDescription; if it is rendered as a <desc> element, I think it should be called desc.

I alluded to this in the description, but to elaborate: I consider the use of the SVG desc element over the aria-description attribute more of an implementation detail. My understanding is that they provide the same semantic meaning, and I’m happy to switch to the aria-description attribute if it’s less surprising/more consistent with the option name.

I also think it’s important that we make clear that this option is distinct from a visible description. As MDN says:

The aria-description property should only be used when providing a visible description is not the desired user experience.

We already provide a caption option for a user-visible caption and label options for user-visible scale labels. In the future, we might provide title and subtitle options. #423 Therefore I think it’s useful to delineate these non-visible options from the visible ones.

For now I find it quite difficult to evaluate this PR, as I lack a tool that would help me "visualize" the tree of text/navigation elements that screen readers see in the code, and ideally compare it against a desired state.

I agree. I similarly found this challenging because it is difficult to interpret the specification and tutorials, and to understand the actual effect of the implemented changes. But the approach here is minimally opinionated and thus unlikely to be “wrong”—though by the same degree, it doesn’t do very much. Most of what we’re doing here is simply exposing the aria-label and aria-description attributes (or desc element), allowing authors to then use these attributes to improve the accessibility of their charts. I don’t think there’s any potential harm in simply allowing the use of ARIA attributes; and I suspect that there’s more we might want to add here in the future (e.g., the role attribute). It’s possible that the default aria-label added in this PR is not helpful, and I could remove that, though, I am somewhat hopeful that as a high-level grammar for visualization there is a way to make Plot visualizations at least slightly more accessible by default.

@mbostock
Copy link
Member Author

I’ve switched to the aria-description attribute instead of the desc element, since my guess is that you prefer that.

Otherwise, here are some questions for you:

  • Should I remove the automatic aria-label for marks and axes (e.g., “x-axis”)?
  • Or do we need a way to customize/override the constant aria-label for marks and axes?
  • Are you okay with the ariaLabel mark option being a channel, but the ariaDescription mark option being a constant?
  • Are there other ARIA attributes you think must be included in this PR (e.g., “role”)?

@Fil
Copy link
Contributor

Fil commented Jan 27, 2022

I’ve switched to the aria-description attribute instead of the desc element

yes, thank you!

Should I remove the automatic aria-label for marks and axes (e.g., “x-axis”)?

Keep. They are probably minimally useful (better to know that it's a chart made of 2 axes, lines and rules than nothing?), and if anyone inspects the chart they'll discover that there is something here (and maybe want to change the defaults ↓).

Or do we need a way to customize/override the constant aria-label for marks and axes?

Probably later, when we understand better what's needed?

Are you okay with the ariaLabel mark option being a channel, but the ariaDescription mark option being a constant?

Yes this felt natural in my tests:

        ariaDescription: "A dot representing each astronaut",
        ariaLabel: "name",

Are there other ARIA attributes you think must be included in this PR (e.g., “role”)?

I'm pretty sure we'll need role and tabindex in the future, but I have yet to understand where and how exactly.

@mbostock
Copy link
Member Author

Great, thanks! Is there anything else you’d like me to do here, or do you just want more time to think and review?

@mbostock mbostock changed the title ariaLabel, ariaDescription ariaLabel, ariaDescription, ariaHidden Jan 28, 2022
@mbostock
Copy link
Member Author

Seems to work pretty well for me? I wasn’t able to get QuickTime Player to record the generated voice, but you can see what it was saying in the overlay. I’m not sure what the “image” part is, probably a role thing?

bar-chart-voiceover.mov

@Fil
Copy link
Contributor

Fil commented Jan 28, 2022

Thanks for the recording! I persisted and finally managed to get Voice Over to speak my chart (as well as the letterFrequency chart).

One thing that helped a bit was to remove the "image" on each and every tick of each axis:

.selectAll(".tick line").attr("aria-hidden", "true");

I haven't been able to remove the "image" spelled out at the end of each bar, but it looks like that's the expected outcome (the last word describes the object).

@mbostock mbostock merged commit 9f64f5a into main Jan 28, 2022
@mbostock mbostock deleted the mbostock/aria branch January 28, 2022 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ARIA accessibility support
2 participants