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

Best interface for relation list properties like labeledBy, controls, ... #78

Closed
minorninth opened this issue Jun 26, 2017 · 12 comments
Closed

Comments

@minorninth
Copy link
Collaborator

minorninth commented Jun 26, 2017

We need to figure out the best way that AOM properties can express a relationship between the current node and multiple other nodes, for properties like labeledBy, describedBy, controls, owns, ...

For example, in ARIA you can specify aria-labeledby="foo bar baz" where foo, bar, and baz are the ids of 3 other elements.

Option 1 is how we specced it now - the setter takes any list-like object and makes a copy, the getter returns a list-like object that's a copy. However, WebIDL doesn't have any way to express that currently

Spec:

attribute iterable<AccessibleNode> labeledBy;

Usage:

console.log(element.accessibleNode.labeledBy.length);
console.log(element.accessibleNode.labeledBy[0]);
element.accessibleNode.labeledBy = [label1, label2];

Option 2 is some type like AccessibleNodeList, but with the type mostly hidden from the developer.

interface AccessibleNodeList {
  unsigned length;
  ...
}

readonly attribute AccessibleNodeList labeledBy;

Usage:

console.log(element.accessibleNode.labeledBy.length);
console.log(element.accessibleNode.labeledBy[0]);
element.accessibleNode.labeledBy.add(label1);
element.accessibleNode.labeledBy.length = 0;

One weirdness is that just accessing element.accessibleNode.labeledBy creates an empty AccessibleNodeList, which results in shadowing aria-labeledby, even though i didn't set it to anything.

Or to put it another way, for any other property, I can null it and now the AccessibleNode property no longer shadows the ARIA attribute. But I wouldn't be able to do that here.

Option 3 is that AccessibleNodeList has a constructor.

Spec:

attribute AccessibleNodeList labeledBy;

Usage:

console.log(element.accessibleNode.labeledBy); // Might be null
console.log(element.accessibleNode.labeledBy[0]);
console.log(element.accessibleNode.labeledBy.length);
element.accessibleNode.labeledBy = new AccessibleNodeList([label1, label2]);

That has probably the fewest corner cases but seems more verbose and cumbersome than i was hoping.

@robdodson
Copy link
Contributor

I'd definitely prefer option 1. Does Web IDL not supporting this out of the box mean it's a no go?

@robdodson
Copy link
Contributor

just curious, how does classList work under the hood? It seems kinda similar to what we're talking about. For example:

<div class="foo bar">
// div.classList = ['foo', 'bar', value: 'foo bar']

Also, having thought about it, option 3 doesn't bother me. Especially if it means a clear path forward.

@cookiecrook
Copy link
Collaborator

In the face-to-face, we agreed to avoid using any of the regrettable attribute names like labelledBy. I see it changed here to a single 'l' in labeledBy but I think that would be more confusing to those who already know ARIA. IMO, these should use labelElements which is more explicit and avoids the UK vs US spelling discrepancy.

@cookiecrook
Copy link
Collaborator

This could also mirror the classList naming convention (labelList or labelElementList) as a hint to authors that it allows:

element.accessibleNode.labelElementList.add(labelElement1);
element.accessibleNode.labelElementList.remove(labelElement2);

@cookiecrook
Copy link
Collaborator

cookiecrook commented Aug 7, 2017

Similar patterns to use as a starting point for the other IDREF/IDREF attrs that are well supported and consistent across implementations. fooElement for IDREF types and fooElementList for IDREFS types.

IDREFS

  • labelElementListmirrors aria-labelledby
  • desciptionElementListmirrors aria-describedby
  • controlElementList mirrors aria-controls. Is 'control' clear here? I am not sure 'controls' was clear in ARIA either. It was intended as a verb ('this controls that') but the term 'controls' is often used as a plural noun in UI discussion and API.

IDREF

  • errorMessageElementmirrors aria-errormessage. Single IDREF, though I'm not sure why.
  • activeDescendantElement. Explicit but verbose. Too much so?

I think we should avoid implementing flowto in AOM, since that's one of the features of ARIA that was not fully baked. It can cause looping and skipped elements, and is ambiguous with regards to reversing the order.

@minorninth
Copy link
Collaborator Author

labelElementList mirrors aria-labelledby
desciptionElementList mirrors aria-describedby

These two sound okay, but note that with Phase 3, the targets of these will be AccessibleNodes, but not necessarily DOM Elements. So how about labelNodeList, descriptionNodeList?

controlElementList mirrors aria-controls

Yeah, this is really ambiguous. I think controlledElementList is much more clear, but now we run into the same naming problem as with labeledBy vs labelledBy.

At least part of the reason that aria-labelledby was a big problem was because text editors and browsers didn't flag it as wrong if you spelled it the wrong way - unless you tested it with AT you wouldn't have any clue you had a misspelling.

I think it will be less of an issue with a JavaScript API. You can tab-complete these properties in the developer tools, and you can tell whether they're supported or not by checking whether they're null or undefined on an empty AccessibleNode.

@minorninth
Copy link
Collaborator Author

As for flowTo, I'd be happy to get rid of it but I deliberately didn't want to use the AOM as a place to make judgement calls on ARIA. If we toss that, we'd have to toss other ARIA attributes of dubious value, and it's a slippery slope.

If flowTo gets marked as deprecated in an editor's draft I'm happy to remove it from this spec, otherwise I think we should support all of ARIA.

I'm not sure there are looping issues, though - Windows screen readers make following flowto optional via an extra keystroke. I suspect on other platforms user agents could maybe expose flowto as a custom action.

@cookiecrook
Copy link
Collaborator

@minorninth wrote:

how about labelNodeList, descriptionNodeList?

Sure. Sounds good.

As for flowTo, I'd be happy to get rid of it but I deliberately didn't want to use the AOM as a place to make judgement calls on ARIA. If we toss that, we'd have to toss other ARIA attributes of dubious value, and it's a slippery slope.

We should not add any API that is either ambiguous or known to conflict with a planned API in a later phase of AOM. Flowto is both, since it could cause conflicts (or require disambiguation) with the Phase 3 ordering of virtual nodes. Also, since we planned to have an announcement notification in Phase 2, all the live region attrs (live, relevant, atomic, etc.) should not be mapped.

Windows screen readers make following flowto optional via an extra keystroke.

And iOS adds an "all items" rotor that ignores flowto. Regardless, authors wanting to use live regions or flowto can still implement them via DOM methods like setAttribute, so we're not losing anything by not including the aliases on accessibleNode.

I will write these two up as separate issues.

@cookiecrook
Copy link
Collaborator

@minorninth wrote:

Yeah, [controlElementList] is really ambiguous. I think controlledElementList is much more clear, but now we run into the same naming problem as with labeledBy vs labelledBy.

AFAIK, "controlled" and "controller" are spelled with a double-l in both UK and US English, so this one doesn't bother me much. I'm open to another alias if one presents itself, but let's consider controlledNodeList to match the others you suggested above.

@cookiecrook
Copy link
Collaborator

Proposed deprecation of aria-flowto in ARIA Issue #630

@cookiecrook
Copy link
Collaborator

@rniwa mentioned that the AccessibleNodeList could be replaced by simple array assignment.

el.acccessibleNode.labelList = [label1, label2];

@alice
Copy link
Member

alice commented Feb 16, 2018

Tracked in #102.

@alice alice closed this as completed Feb 16, 2018
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

No branches or pull requests

4 participants