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

RFC: Reference Target for Cross-root ARIA #207

Merged
merged 23 commits into from Apr 23, 2024

Conversation

behowell
Copy link
Collaborator

@behowell behowell commented Dec 13, 2023

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.

@behowell behowell requested a review from alice December 13, 2023 00:24
```html
<template id="t-fancy-combobox">
<combobox-input id="combo-input"
listbox="combo-listbox"> <!-- (1) -->
Copy link
Collaborator

@Westbrook Westbrook Dec 13, 2023

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.

Comment on lines 83 to 97
```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>
```
Copy link
Collaborator Author

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)


#### 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.
Copy link

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?

Copy link
Collaborator Author

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.

Copy link
Member

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.

Copy link

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.

Copy link
Collaborator Author

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 Show resolved Hide resolved
reference-delegate-explainer.md Outdated Show resolved Hide resolved
if (attr === "activeitem") {
this.shadowRoot_.referenceDelegateMap['aria-activedescendant'] = value; // (2)
}
}
Copy link
Collaborator

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.

Copy link
Collaborator Author

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 Show resolved Hide resolved
* 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
Copy link
Member

@alice alice Dec 20, 2023

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 options 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 :)

Copy link
Collaborator

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. 🙂

@behowell behowell changed the title RFC: Reference Delegate for Cross-root ARIA RFC: Reference Target for Cross-root ARIA Feb 9, 2024
@behowell
Copy link
Collaborator Author

behowell commented Feb 9, 2024

Apologies this has been sitting around for a while. I've made some updates to this proposal. Here's a summary:

  1. Renamed the feature to "Reference Target" instead of "Reference Delegate". See RFC: Reference Target for Cross-root ARIA #207 (comment)
  2. Broke proposal into two phases. Phase 1 includes only the referenceDelegate attribute to try to get that feature in faster. Phase 2 can come later to give more bake time for the referenceDelegateMap attribute, or some alternative.
  3. Made updates based on PR feedback, and for clarity.

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

Copy link
Collaborator

@nolanlawson nolanlawson left a 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.

reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Show resolved Hide resolved
reference-target-explainer.md Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
Co-authored-by: Nolan Lawson <nolan@nolanlawson.com>
Copy link
Collaborator

@Westbrook Westbrook left a 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!!

Copy link

@asyncLiz asyncLiz left a 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 🥳

exportid-explainer.md Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Outdated Show resolved Hide resolved
reference-target-explainer.md Show resolved Hide resolved
behowell and others added 2 commits April 23, 2024 14:30
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.
@behowell
Copy link
Collaborator Author

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!

@behowell behowell merged commit 3327ce7 into WICG:gh-pages Apr 23, 2024
1 check passed
@alice
Copy link
Member

alice commented Apr 24, 2024

Nice one! Thanks for all your work on this!

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

Successfully merging this pull request may close these issues.

None yet

6 participants