phetsims/scenery

Add support for aria-activedescendant

Closed this issue · 5 comments

Use of this attribute was requested in build-an-atom, but hasn't been used since. The attribute is a way to tell a screen reader what the active element should be in a composite widget without having to change document focus. More documentation: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-activedescendant_attribute

It is kind of like aria-labelledby/aria-describedby in that the value is the id of another element in the document, but more explicitly a child of the element with aria-activedescendant. Also, according to https://www.w3.org/TR/wai-aria-1.1/#aria-activedescendant, that element must be a textbox, which is surprising.

@zepumph adding to the dev meeting agenda, we may get to this at some point. But also lower priority until the need for this comes up again.

Added above. @jessegreenberg please give a spot check review to the above commit, knowing further review will be done as part of #819.

I'm interested in potentially coming up with a way that we don't have to copy paste so much code. Though I'm unsure how many more times this will come up (famous last words).

Clearly working well, thanks for adding unit tests. I realized that one way in which this differs from the other association setters is that there can only be one aria-activedescendant, rather than a list. So maybe we could either refactor the association setters to handle aria-labelledby, aria-describedby, and aria-activedescendant, then assert that there is only one association in the list of ariaDescribedbyAssociations. Or we could have a different implementation for aria-activedescendant. What do you think is best @zepumph?

@zepumph assigning back to you to consider the above. I am leaning toward not having an associations array for aria-activedescendant because it would be strange to use an array-like api for this thing that can only have one value.

One idea:

Maybe it could have an array like implementation internally, and then just have public methods that differ. So the active descendant api would just be:

let x = new Node({
activeDescendant: { /* associationObject */ }
});

x.activeDescendant = { /* associationObject */ };
x.setActiveDescendant( { /* associationObject */ } );

A pro of this would be that it keeps all the association object methods in sync, and makes them easier to be maintained.


Another idea:

Internally I may already want to refactor all of the logic of these into a better system. If you look at the chunks of code that are these three attributes, their public methods take up 445/2700 lines. 15% for three options!?!?

I especially realized this when adding this attribute was basically just an exercise in copy pasting and then renaming vars to active descendant.

Currently there are no usages of activedescendant even though it is implemented and working in tests. I think we should close this issue, and move to #952 to support (a) a better implementation, and (b) support for a single value or an array as a value