WICG / webcomponents

Web Components specifications
Other
4.39k stars 376 forks source link

[a11y]: permit aria-hidden=true on focusable shadow elements in FACE #1014

Open bennypowers opened 1 year ago

bennypowers commented 1 year ago

consider:

class FACEInput extends HTMLElement {
  static formAssociated = true;

  static template = document.createElement('template');
  static {
    this.template.innerHTML = /*html*/`
      <input aria-hidden="true">
    `;
  }

  #internals = this.attachInternals();

  constructor() {
    super();
    this.#internals.role = 'textfield';
    this.attachShadow({ mode: 'open', delegatesFocus: true })
      .append(this.constructor.template.content.cloneNode(true));
    // handwave: imagine we add a "Reactive controller" which handles syncing aria props
    //           between the shadow root input and the host element.
    hookUpListenersAndAriaMixinProps({
      host: this,
      target: this.shadowRoot.querySelector('input'),
      internals: this.#internals
    });
  }
}

customElements.define('fancy-input', FACEInput);

Thus, we have

In this case, the author's intent is for the host element (fancy-input) to act transparently as an input. BUT, the accessibility tree will report two nested textfields - one for the host (internals.role) and a nested one for the shadow input, even though it is marked by the author as aria-hidden.

As well, the 'hidden' input will trigger a failure for automated accessibility audits.

We should consider relaxing the rule which disallows aria-hidden=true and role=presentation for elements which are within a FACE' shadow root. This would allow FACE authors to remove focusable elements from the ax tree, imperatively delegating ARIA stuff to the host.

cc @annevk and @rniwa who had some pointed critique with regard to focus and keyboard events. If there was a way to overcome those critiques, could this be a viable (though less capable) alternative to @alice' semantic delegate proposal?

bennypowers commented 1 year ago

As discussed in #1005

bennypowers commented 1 year ago

I'm grateful to @alice for her time over element chat while she participates in the Igalia WebEnginesHackfest. She graciously donated her time and (considerable) expertise to try and understand my intent with this issue and work through possible solutions.

In summary:

The proposal would therefore be amended:

IF
  a shadow host delegatesfocus
  AND (
    is a FACE
    OR has internals attached
  )
THEN
  allow removing focusable shadow children from the AT
  have AT focus land on the host when keyboard focus is on the input

Alice recommended a workaround where


@alice reports that rough experimentation with implementing the original proposal here (permitting aria-hidden) has better-than-expected results. the screen reader does report that a text field is focused, perhaps because the <input aria-hidden="true"> is within the host with role=textbox, so the host is the nearest ancestor with a role

for completeness, this proposal (originally, and with this comment's amendation) assumes the component developer has to hook up all the plumbing via element internals, but the developer advantage of not having to reimplement <input> with contenteditable remains.

EisenbergEffect commented 1 year ago

@bennypowers Just to be clear, if I were to implement a custom input, the guidance would be to:

Is that correct or did I mix together two different approaches? I'm building some learning contents around this so I want to make sure I've got the best approach in my demos.

bennypowers commented 1 year ago

Thanks for asking @EisenbergEffect. I'm hesitant to offer anything in this thread as "good advice" for a few reasons:

  1. this whole idea is intended to be a temporary stopgap until stronger solutions are adopted, be they semantic delegate or a more general solution
  2. My own understanding, and I would hazard to suggest that of the implementers as well, is rapidly advancing

That being said, as of this writing, your bullet points combine two separate and possibly incompatible solutions. This is how I understand things:

Solution for right now

Possible future solution according to this proposal


What I'm really hoping for is to gather multiple developers together with some implementers to develop advice for these components. It would be tremendous to get yourself, @nolanlawson, and @asyncLiz together with maybe @alice, @annevk and @rniwa, but perhaps that would be too onerous to arrange.

EisenbergEffect commented 1 year ago

@bennypowers Thanks for the clarification! I'm happy to get together. I don't think I can add much value to the conversation itself. I do want to make sure things are properly documented and propagate the recommended approaches to the community so we can have higher quality components in the wild though.

Let's keep updating this thread as things progress. I'll use this as a resource for whatever I'm teaching so I can both show people the best current approach as well as inform them of future directions.

alice commented 1 year ago

Thanks for the summary, @bennypowers.

Just to add some nuance, the experiment I ran was:

I'm less than optimistic about this as a viable pathway even with this mild success; I think there will be a lot of details to figure out, and I would rather we spend that energy on one of the less hacky, longer-term, more general options.

clshortfuse commented 1 year ago

I mulled this over for a couple of hours.


My current, evolved interpretation of FACE is they:

  1. Are containers that can read :disabled state
  2. Should have an ARIA role that match their container type.
  3. Can unifyingly set a single form value representing the value within the container
  4. *Can have labels passed to it.
  5. *Can have an AXLabel
  6. *Can have an AXDescription

The starred points are the points of issue. Yes, a label and description can be passed, but we have no ability to pass that value down to whatever element of our choosing (if any) in the Shadow Root. I need to stress that there is a difference between the textContent of a label from .labels and the AXLabel. We cannot just take the textContent because a label can be like this:

<label for=foo>
  <i class=font-icon aria-hidden=true>person</i>Name
</label>
<fancy-input name=foo></fancy-input>

That means we need more than just text, we need the actually AXLabel to be passed down. Using .labels is not enough because textContent isn't safe. I've experimented with observing aria-labelledby and aria-describedby, finding the nodes, and putting a mutation observer, cloning in the shadowRoot and then referencing them, but it's a hacky solution and considering those nodes may not exist at the time the attribute is created, it's unreliable.


I took a look at https://github.com/alice/aom/blob/gh-pages/semantic-delegate.md and it seems it could satisfy my current issues with using external labels for some FACEs. My other concern is a singular element to receive all the ARIA attributes. I haven't had the use case for it yet off the top of head, but I wouldn't like to be restricted between using aria-label and aria-describedby to the same shadowed element. If I have a container-like FACE, aria-label can reference one element, but aria-controls may refer to another. As the author, I would like to choose where I use them. Of course, that doesn't mean extra attributes or ElementInternals properties can't be added later.