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

feat(plugin): add aria-* visualizer plugin #142

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ericrallen
Copy link

This is a first pass at working on the aria- Visualizer* plugin.

It currently only supports aria-hidden, but will be made to work with more aria* attributes and be made more robust in general in the near future.

It leverages some of the info panel updates from #139 and adds a new option to disable the annotation checkbox for plugins that don't have annotation.

I feel like I probably need to add annotation for both the plugin and the Focus Tracker added in #139, but I don't know enough about how it's implemented in this codebase yet. Regardless, I think it's a nice option to have for situations where we're using the info panel as more of a status indicator.

Closes #128

@khanbot
Copy link

khanbot commented Mar 21, 2019

CLA signature looks good 👍

@somewhatabstract
Copy link
Contributor

I haven't forgotten this. I will try to make time this week to give feedback. Thank you for your patience.

this.ariaAttributes = ATTRIBUTES.reduce((functionMap, attribute) => {
const methodSuffix = formatAttributeForMethodName(attribute);

functionMap[attribute] = {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's probably a much better way to handle this, but I wasn't sure how we'd deal with all the various different approaches you might need to take to visualizing the semantics of some of the aria-* properties.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not certain yet how we'd do that either. Might be something that evolves a bit as we add support for different aria-* features.

Copy link
Contributor

@somewhatabstract somewhatabstract left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a reasonable approach so far.

I think having it show the roles like this and then also hide things that are aria-hidden (harder to do, but it would be great if we could somehow show things that a screen reader would see but we don't visually - perhaps you already planned for that).


let Plugin = require("../base");

let annotate = require("../shared/annotate")("roles");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use const instead of let?

@@ -0,0 +1,104 @@
/**
* Allows users to see what screen readers would see.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment!

this.ariaAttributes = ATTRIBUTES.reduce((functionMap, attribute) => {
const methodSuffix = formatAttributeForMethodName(attribute);

functionMap[attribute] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not certain yet how we'd do that either. Might be something that evolves a bit as we add support for different aria-* features.

}

getTitle() {
return "aria-* Visualizer";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could call this something a little more descriptive?

Spitballing:

  • "Render as Screen Reader"
  • "Screen Reader View"
  • "ARIA View"?

Not super important; just thinking aloud.

}

getDescription() {
return "See the effects of your aria-* and other a11y attributes";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about making the description speak more to why one would want to use this feature rather than what id does? Perhaps something like, "View the page as though you were the screen reader. What things are visible, what aren't; what text will be read out, etc."

<p>This paragraph is visible.</p>
<p id="p-with-hidden">This paragraph has a <span id="span" aria-hidden="true">hidden</span> span.</p>
<p id="hidden-p" aria-hidden="true">This paragraph is hidden.</p>
</article>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't currently working

image

I turn on the visualizer but they aren't getting hidden (also, can this text say aria-hidden instead of hidden so it's clear what we mean?

Perhaps it's just that this draft doesn't do that bit yet. Sorry if I'm jumping ahead here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was an error on my part. Should be working as expected after the next push.

@ericrallen
Copy link
Author

@somewhatabstract I haven't quite figured out how to approach other attributes for the visualization side of things.

It might be good to take some time to compile a list of different attributes we want to tackle, prioritize them, and then muse on what a good visualization for that might be.

I think #128 would be a good place for that discussion. I'm going to leave this one open as a draft so I can continue to rebase changes and implement new visualizations as we come up with them.

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.

Feature: Switch things into or out of the UX based on aria attributes
3 participants