-
Notifications
You must be signed in to change notification settings - Fork 168
Steps to adding accessibility to Phosphor #392
base: master
Are you sure you want to change the base?
Conversation
This commit also does some stylistic changes as well, like capitalizing ARIA and treating it like the other generic attributes.
Add all ARIA attributes from the standard.
According to experts (including both an invited expert advisor and a co-chair of ARIA committee), aria-controls is not needed, and actually annoying in JAWS.
Also use the tab bar orientation setter to set the default orientation.
This allows interfaces (including accessible ones) to indicate to the user that the command may toggle state.
Before, we needed to clamp down on these types for aria attributes. This is (a) backwards incompatible and (b) not needed anymore.
@@ -1167,6 +1197,7 @@ namespace Private { | |||
dataset: asFunc(options.dataset, emptyDatasetFunc), | |||
isEnabled: options.isEnabled || trueFunc, | |||
isToggled: options.isToggled || falseFunc, | |||
isToggleable: options.isToggleable || !!options.isToggled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? It seems like it should only be !!options.isToggled
and never use the options.isToggleable
.
@@ -893,6 +914,9 @@ class TabBar<T> extends Widget { | |||
let ci = this._currentIndex; | |||
let bh = this.insertBehavior; | |||
|
|||
|
|||
// TODO: do we need to do an update to update the aria-selected attribute? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This question seems important for accessibility to actually work as tabs are used. I think answering it is probably within the scope of this pull request in lieu of punting.
This pull request looks nearly ready to me; I only have the two comments above. |
@sccolbert and I had some conversations about this a while ago. Unfortunately, I didn't document the minutes from the conversation the PR. One thought was that we should break it up into a few PRs: adding ARIA support, adding the toggleable state, the menu work, and the tab work. If you'd like I can do that. Actually, I also have a few more changes I found on my disk I'd like to remember, review, and push here. |
Addresses some of the issues presented in #391 which was done during the Web4All Hackathon
Code written in part by @jasongrout, Volker (@zorkow) and others at the Hackathon