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

Implement a11y api changes #748

Closed
9 tasks done
zepumph opened this issue Mar 15, 2018 · 9 comments
Closed
9 tasks done

Implement a11y api changes #748

zepumph opened this issue Mar 15, 2018 · 9 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Mar 15, 2018

These changes where discussed in #686, and decided on in a11y dev meeting today.

  • Remove *AsHTML methods and have a single method determine if we should use innerHTML settings or textContent on DOM elements. Performance considerations in the comment [Discussion] Rethink cases for setAccessibleContent #686 (comment)

  • Reimplement accessibleLabel: {{string}} so that it only ever set's a sibling DOM element, tag declared in labelTagName. labelTagName should default to label if not specified when accessibleLabel is given.

  • innerContent: sets the innerHTML on tagName DOM element of the Node itself.

  • Reimplement useAriaLabel: {{boolean}} to setAriaLabel: {{string}} which will set the aria label attribute on the Node's DOM Element

  • prependLabels: {{default false}} will be split into two options:

    • appendLabel: {{default false}}
    • appendDescription: {{ default false}}

    Notice that the default value has switched, and the default now places the label/description before the Node's DOM Element.

  • parentContainerTagName exists if ever there is a label or description DOM Element, defaults to div, (implementationally defaults to null until added to the dom)

  • If parentContainerTagName is given without label/description content, then just surround the single Node's tagName

  • Label and description DOM Elements cannot be children to the Node's DOM element, only siblings.

  • Rename parentContainerTagName -> containerTagName

@zepumph
Copy link
Member Author

zepumph commented Mar 15, 2018

@jessegreenberg @mbarlow12 please add to suggest edits.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Mar 15, 2018

parentContainerTagName exists if ever there is a label or description DOM Element, defaults to div
If parentContainerTagName is given without label/description content, then just surround the single Node's tagName

These two in combination seem tricky. This means the result of specifying a parent container tag name of "div" on the node with no label or description elements will be a "no-op", which is a confusing case.

Maybe default should be null, but a div could be use if label or description elements are added and a parent container ins't specidied?

@zepumph
Copy link
Member Author

zepumph commented Mar 15, 2018

agreed. I'll update the checklist above.

@zepumph
Copy link
Member Author

zepumph commented Mar 26, 2018

We want to be able to disseminate the a11y api to other devs in the coming weeks, so marking as High priority.

@samreid samreid changed the title Implemenet a11y api changes Implement a11y api changes Mar 26, 2018
zepumph added a commit to phetsims/forces-and-motion-basics that referenced this issue Mar 26, 2018
zepumph added a commit to phetsims/molecules-and-light that referenced this issue Mar 26, 2018
zepumph added a commit to phetsims/scenery-phet that referenced this issue Mar 26, 2018
zepumph added a commit that referenced this issue Mar 26, 2018
zepumph added a commit to phetsims/joist that referenced this issue Mar 26, 2018
@zepumph
Copy link
Member Author

zepumph commented Mar 26, 2018

This issue is massive, so I think it may be best to work on it in pieces (perhaps breaking some pieces into separate issues). With that, to try to be as thorough as possible, I think that each piece would be good to review separately, that way we don't lose track of some nuance or forget about some pieces.

@zepumph
Copy link
Member Author

zepumph commented Mar 27, 2018

#753 holds dev versions as a benchmark before diving into this issue.

zepumph added a commit to phetsims/a11y-research that referenced this issue Mar 27, 2018
zepumph added a commit to phetsims/john-travoltage that referenced this issue Mar 27, 2018
zepumph added a commit to phetsims/scenery-phet that referenced this issue Mar 27, 2018
zepumph added a commit to phetsims/friction that referenced this issue Mar 27, 2018
zepumph added a commit to phetsims/ohms-law that referenced this issue Mar 27, 2018
zepumph added a commit to phetsims/joist that referenced this issue Mar 27, 2018
@zepumph
Copy link
Member Author

zepumph commented Mar 31, 2018

All that is left as part of this issue is separating out prependLabels to individual appendLabel/appendDescription. I won't have time to get to this until next week sometime.

While working in the A11y trait, I found lots of smallish things that should be fixed. For the most part, I just jotted them down, but I added a TODO also. I will create separate issues for the things I think need them.

@zepumph
Copy link
Member Author

zepumph commented Apr 5, 2018

All features have been implemented with the exception of some usages of prependLabels. That will mostly happen as we go through sims in #753. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants