-
Notifications
You must be signed in to change notification settings - Fork 59
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
RFC: Reference Target for Cross-root ARIA #207
Conversation
reference-delegate-explainer.md
Outdated
```html | ||
<template id="t-fancy-combobox"> | ||
<combobox-input id="combo-input" | ||
listbox="combo-listbox"> <!-- (1) --> |
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.
I wish there was a quality path towards normative API here, so that <fancy-input>
could be consumed as a drop-in replacement of <input>
in something like the ARIA APG demos for combobox.
reference-delegate-explainer.md
Outdated
```html | ||
<input role="combobox" | ||
aria-controls="fancy-listbox" | ||
aria-activedescendant="fancy-listbox" /> | ||
<fancy-listbox id="fancy-listbox"> | ||
<template shadowrootmode="closed" | ||
shadowrootreferencedelegatemap="aria-controls: real-listbox, | ||
aria-activedescendant: option-2"> | ||
<div id="real-listbox" role="listbox"> | ||
<div id="option-1" role="option">Option 1</div> | ||
<div id="option-2" role="option">Option 2</div> | ||
</div> | ||
</template> | ||
</fancy-listbox> | ||
``` |
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.
It looks like the comment you replied to has been deleted. But I answered a similar comment here: #207 (comment)
reference-delegate-explainer.md
Outdated
|
||
#### Cons | ||
|
||
* The Reference Delegate feature is intended to support all attributes that use ID references. Thus, the only time a new attribute will be supported by Reference Delegate is when it is a completely new attribute in the HTML spec. There is no backwards compatibility concern, since no websites will be using the new attribute before is is supported. |
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.
I think this argument works when scoped to targeting, although presumably it should not be limited to ID references. What about <label>
-sans-for=""
?
However, before there was a suggestion that all kinds of other behavioral things would be delegated as well. E.g., focus would magically transfer. I guess that's off the table now?
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.
There's a Nesting inside <label>
section that currently says it'll work when nested in a label.
However, as I mentioned in a note in that section, that is the only part of this feature that doesn't directly work with IDREF attributes. Perhaps it's better to keep referenceDelegate
scoped to deal only with ID references?
Delegating other things isn't off the table; but it would be out of scope of this particular feature. Previous proposals have stalled in part due to scope creep, and it might be best to keep this feature focused on the one piece currently blocking ARIA from working across shadow roots.
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.
I'm reluctantly in favour of leaving <label>
wrapped out of scope - as much as I would love it to work, I agree that it feels like scope creep.
It does seem like if it's included, it should be an explicit inclusion, to avoid opening a can of worms in figuring out how all the other types of nesting element behaviours would work and/or what would be in/out of scope.
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.
I think <label>
without for
should work. I don't know if any other elements that work like it, but I guess we should double check that.
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.
I've updated the proposal to say that label wrapping doesn't work without the for
attribute. This keeps the feature focused only on ID references. I'm open to changing it back if there is strong feedback in favor of it.
reference-delegate-explainer.md
Outdated
if (attr === "activeitem") { | ||
this.shadowRoot_.referenceDelegateMap['aria-activedescendant'] = value; // (2) | ||
} | ||
} |
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.
Minor quibble: I think a listbox component would provide better encapsulation by exposing methods to move the focus up/down rather than an attributeChangedCallback
explicitly setting the index of the active item. The issue here is that the consumer of the listbox has to "know" how many items are in the listbox and which one is the active one (thus exposing internal details).
Whereas using methods (or events or what have you), the fancy input could just communicate to the listbox "Hey, the ↑ key was pressed," so the listbox could change its referenceDelegateMap
internally without the consumer knowing how many items there are or which one is the active one.
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.
That does make sense. However, my goal was more to show how the referenceTargetMap works, rather than design a real listbox/combox. I'm inclined to leave it as-is to keep things simpler for the example.
reference-delegate-explainer.md
Outdated
* Requires new attributes in two places in order to work: E.g. `shadowrootreflectscontrols` on the shadow root _and_ `reflectariacontrols` on the target element. | ||
* When multiple elements are used for the same attribute, the author cannot control the order (the order is always the DOM order). | ||
|
||
### Use exported IDs instead of per-attribute targeting |
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.
I'd like to make one last pitch for this, and then I promise I'll let it go :)
The default referenceDelegate
attribute (without the per-attribute targeting) supports a fairly well-understood and easy to articulate use case: the "semantic delegate" case. This is a pattern already widely used in custom element libraries.
referenceDelegate
supports the "semantic delegate" use case by solving the problem that, conceptually, the custom element should be the target for certain IDREF attributes, but those attributes need to target the "delegate" element in practice. This preserves the encapsulation of the custom element by not requiring authors using the element to know anything about those implementation details. They can simply use the custom element as though it is the delegate element[1].
I think referenceDelegateMap
is trying to solve other problems, and I don't think it's solving them anywhere near as well.
The only obvious[2] case I can come up with for referenceDelegateMap
is the aria-activedescendant
case, like where <input>
uses aria-activedescendant
to refer to one of the option
s inside the shadow root of a custom element representing a listbox
in the Combining referenceDelegate
and referenceDelegateMap
example.
In this case, you must know that there are option
elements within the <fancy-listbox>
which can be a valid target for aria-activedescendant
, and you also must know that the attribute will be forwarded correctly. This is a completely different case from the semantic delegate case, in which you don't need to know anything about the internals, only that you can treat the host as though it is what it "appears" to be.
To me, exportid
is a better fit for this type of problem: where there is some element inside the shadow root that needs to be targeted explicitly, but for whatever reason needs to remain inside the shadow root. exportid
allows you to specify an "API" for your element to allow references to elements inside its shadow root that you guarantee will exist, but need not reveal any more information about.
exportid
also gives us ample flexibility for unforeseen cases. What I would especially like to avoid is a situation where we figure out all of the inherent complexity of supporting referenceDelegateMap
(e.g. how to design its JavaScript API) and then realise that there actually are cases where the "bottleneck problem" is an issue, so we need to do something like exportid
anyway. Not only would we then have spent extra effort in designing referenceDelegateMap
, but now authors have to choose which one to use for their largely overlapping use cases.
[1] Leaving aside that you can't use IDREF attributes on the host and also have them forwarded, which was deliberately left out of scope here to keep the scope as minimal as possible.
[2] I would really like to understand what other cases there are! @nolanlawson you mentioned doing an audit of your component library recently; perhaps you have some examples? The example under Delegating to multiple elements is interesting, but seems like it would equally be a problem in a non-shadow root world. That is, assuming the content inside the shadow root in this example is intended to be "unknown" (i.e. user-generated or similar) to the page author, the author would still have no way in practice to correctly set aria-describedby
on the <input>
even if that content were in light DOM. And if it was user-generated, how would you correctly set the referenceDelegateMap
? But, maybe this is all wishful thinking on my part :)
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.
@alice The ariaActiveDescendant
case is definitely the strongest one I've seen. The fact that you want an ariaControls
as well as an ariaActiveDescendant
pointing into the same shadow root came up twice in my component audit (both were comboboxes).
Aside from that, every use case I've seen for any kind of cross-root references could be solved by either a "semantic delegate" type solution, or element reflection rather than reference delegates – including the tricky case of multiple elements inside one shadow root sharing the same ARIA attribute (see our lightning-input and its date-aria-*
and time-aria-*
attrs, which allow for a separate date element and time element to each point to separate elements in their shadow-including-ancestor using controls
/describedBy
/labelledBy
). This has led me to believe that the "bottleneck effect" is not such a dealbreaker for the current proposal.
I definitely agree though that it feels "weird" to have (say) the listbox be both the target for controls
as well as activeDescendant
. But I would still be happy with this proposal since it gives us a path forward – perhaps userland solutions can make it more ergonomically appealing. 🙂
Apologies this has been sitting around for a while. I've made some updates to this proposal. Here's a summary:
It would be great to get this explainer merged into the AOM repo once people have had a chance to look at the edits. Thank you for the reviews! cc @alice, @nolanlawson, @Westbrook, @annevk, @asyncLiz |
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.
Looks great to me. I especially like how it's broken into two phases, with the first phase covering the most common use case.
Co-authored-by: Nolan Lawson <nolan@nolanlawson.com>
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.
Very excited to see this continue to move forward!!
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 looks great!! Thank you so much for all the work in pushing this forward 🥳
Co-authored-by: Alice <95208+alice@users.noreply.github.com>
Add `anchor` to the list of supported attributes, and add a note about `::part()` to the "Cons" list of ExportID.
Thank you for the feedback everyone! I'm going to merge this PR to get the explainer checked in. Further feedback can be in the form of an issue in the aom repo. Thanks! |
Nice one! Thanks for all your work on this! |
Just saw that this was merged. Awesome work! |
Note
This PR has been merged. The latest version of the explainer is available here:
📜 Reference Target for Cross-root ARIA
For further feedback, please log an issue or PR.
Reference Target is a new feature that enables creating ARIA links to elements inside a component's shadow DOM, while maintaining encapsulation of the internal details of the shadow DOM.
I'd really appreciate any feedback and comments! You can leave comments directly on the .md file in this PR.
Thank you to @alice, @annevk, and @chrisdholt for taking an earlier look at this proposal and giving feedback.