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

Description of role="presentation" in docs is misleading #510

Closed
TrevorBurnham opened this issue Dec 13, 2018 · 12 comments · Fixed by #522
Closed

Description of role="presentation" in docs is misleading #510

TrevorBurnham opened this issue Dec 13, 2018 · 12 comments · Fixed by #522

Comments

@TrevorBurnham
Copy link

TrevorBurnham commented Dec 13, 2018

The no-static-element-interactions docs currently state:

Case: This element is not a button, link, menuitem, etc. It is catching bubbled events from elements that it contains

If your element is catching bubbled click or key events from descendant elements, then the proper role for this element is presentation.

<div
  onClick="onClickHandler"
  role="presentation">
  <button>Save</button>
</div>

Marking an element with the role presentation indicates to assistive technology that this element should be ignored; it exists to support the web application and is not meant for humans to interact with directly.


There are a couple of things wrong with this:

  1. The example implies that a div element should have role="presentation". But setting role="presentation" on a div is redundant. The definition of role="presentation" in WAI-ARIA 1.1 is:

    An element whose implicit native role semantics will not be mapped to the accessibility API.

    Elements like div and span have no implicit native role semantics in the first place. See Example 8 in that section for further clarification.

  2. The paragraph after the example suggests that assistive technology ignores elements with role="presentation" entirely, which has never been the case. In fact, this is a common misconception. Per the note in that same section WAI-ARIA 1.1:

    Many individuals erroneously consider role="presentation" to be synonymous with aria-hidden="true"

You might rewrite the section to explain that the rule is prone to false positives, and that role="presentation" is just an escape hatch to make the linter ignore a particular element. However, that advice could lead to unintended consequences. For example, consider the case where someone is using a ul element to handle events that bubble up from its descendants:

<ul onClick={this.handleItemClick}>
  <li><button>Click me 1</button></li>
  <li><button>Click me 2</button></li>
  <li><button>Click me 3</button></li>
</ul>

Adding role="presentation" to the ul element would remove the list semantics.

So I think you should consider adding a different escape hatch for the event bubble case. There's no appropriate ARIA attribute, because such an element doesn't need any special handling from assistive technology. Maybe you could check for an attribute called something like data-only-handles-propagated-events. 🤷‍♂️

@jessebeach
Copy link
Collaborator

@TrevorBurnham, thank you for the thoughts. I share your concerns about the escape hatch. I've observed some bad patterns emerge at FB as well. This issue is definitely on my mind.

@jessebeach
Copy link
Collaborator

@TrevorBurnham still thinking on this. I acknowledge the potential for confusion. But we have a fundamental challenge with this linter. The only way it works is if we are very strict with semantics. And one semantic distinction in HTML that isn't formally established is application-HTML vs interface-HTML. In your example, you are conflating the two:

<ul onClick={this.handleItemClick}>
  <li><button>Click me 1</button></li>
  <li><button>Click me 2</button></li>
  <li><button>Click me 3</button></li>
</ul>

A ul is a non-interactive element. It should not host event handlers. Because think about the ambiguity it introduces: is the intent here to simply catch clicks (an application-level concern) to to create a interactive list element (a UI-level concern). Maybe the developer who wrote this literally wants the area of the UL element to be clickable. If we were to lay out the li elements with space between them, thus exposing some of the ul element underneath, this would yield an interactive surface. And putting an onClick on a ul is going to the trigger the no-noninteractive-to-interactive-element rule, since we can't know if the developer is coding this correctly and they're trending toward an ambiguous semantic combination.

What a developer should instead do is introduce Application Boundaries (just like Error Boundaries) into their app, yielding something like:

<ClickableArea onClick={this.handleItemClick}>
  <ul>
    <li><button>Click me 1</button></li>
    <li><button>Click me 2</button></li>
    <li><button>Click me 3</button></li>
  </ul>
</ClickableArea>

So that the Application and UI are clearly distinct. ClickableArea is a component (choose your library) that renders down to <div onClick={this.handleItemClick} role="presentation">, because from a user's perspective and assistive technology perspective, this div is not part of the perceivable user interface. If role="none" had wider support, I'd recommend that, yielding: <div onClick={this.handleItemClick} role="none">, because in terms of the semantics of the UI, this element has no semantic role.

I'd prefer to recommend an approach like this in the documentation.

@jessebeach
Copy link
Collaborator

I can't get on board with a flag like data-only-handles-propagated-events because it is more arbitrary than role="presentation". At least the role indicates intent within the system of ARIA.

The paragraph after the example suggests that assistive technology ignores elements with role="presentation" entirely, which has never been the case. In fact, this is a common misconception. Per the note in that same section WAI-ARIA 1.1:

Good to bring this up. I don't quite understand what you intend with this comment, but there's an important point in the comment that should make it into the documentation as well. If you're thinking it, so are other developers. Might I trouble you for an elaboration?

@TrevorBurnham
Copy link
Author

TrevorBurnham commented Jan 10, 2019

Let me step back for a moment, because you're right that the <ul> example is a stretch. 😅 My objection to the current no-static-element-interactions docs is that they don't distinguish between what you should do for the benefit of accessibility vs. what you should do for the benefit of the linter (or, if you prefer, for semantic clarity). The docs say:

Marking an element with the role presentation indicates to assistive technology that this element should be ignored; it exists to support the web application and is not meant for humans to interact with directly.

By contrast, the code for the rule says:

Presentation is an intentional signal from the author that this element is not meant to be perceivable.

I think the code is right: Setting role="presentation" on a static element is only a signal of intent. AFAIK it has no effect on any assistive technology.

I'd phrase the section of the documentation something like this:

### Case: The event handler is only being used to capture bubbled events

If a static element has an event handler for the sole purpose of capturing events from
its descendants, you can tell the linter to ignore it by setting `role="presentation"`.

```jsx
<div
  onClick={this.handleButtonClick}
  role="presentation">
  <button>Save</button>
  <button>Cancel</button>
</div>
```

This `role` has no effect on static elements, but it serves to clarifies your intent.

@jessebeach
Copy link
Collaborator

My objection to the current no-static-element-interactions docs is that they don't distinguish between what you should do for the benefit of accessibility vs. what you should do for the benefit of the linter

@TrevorBurnham that's a totally fair and good way of putting it.

I agree with your updated documentation. If you put that in the PR, we can merge it in.

@eps1lon
Copy link
Contributor

eps1lon commented Dec 5, 2019

Would it make sense to reject role="presentation" or role="none presentation" with interactive handlers? We used the old recommendation which seems to have quite the negative impact. That way users are automatically notified that the recommended workaround changed.

"rewrite bubble events section" does not indicate that the old behavior was wrong.

Happy to work on on this but I'd like to know what @TrevorBurnham thinks.

@TrevorBurnham
Copy link
Author

Would it make sense to reject role="presentation" or role="none presentation" with interactive handlers?

🤔 I'm just now seeing the subsequent change to the recommendation in the docs (#522). When you say "We used the old recommendation which seems to have quite the negative impact," do you mean that setting role="presentation" on a static element affected accessibility in your app? Can you show me an example of how?

@eps1lon
Copy link
Contributor

eps1lon commented Dec 9, 2019

Would it make sense to reject role="presentation" or role="none presentation" with interactive handlers?

I'm just now seeing the subsequent change to the recommendation in the docs (#522). When you say "We used the old recommendation which seems to have quite the negative impact," do you mean that setting role="presentation" on a static element affected accessibility in your app? Can you show me an example of how?

We're maintaining a component library and have limited capabilities for testing assistive technology. I could not find any issues with NVDA + firefox but from the documentation of the rule it seems that others might.

We basically used the click handler on a backdrop which could own a dialog or popover menu. I'm a bit surprised that the only issue report we got was due to a third party testing tool and not actual assistive technology (mui/material-ui#18106).

I read https://www.w3.org/TR/wai-aria-practices-1.1/#presentation_role_effects again and it seems like the role usage is safe on generic containers such as <div> or <span> though unnecessary.

@TrevorBurnham
Copy link
Author

Looking at that issue, I'm pretty sure that report is a false positive from HTML CodeSniffer. The spec you linked to specifically says that when you set role="presentation" on an element:

The roles, states, and properties of each descendant element remain visible to assistive technologies

@TrevorBurnham
Copy link
Author

I've filed an issue on the CodeSniffer side: squizlabs/HTML_CodeSniffer#274

@TrevorBurnham
Copy link
Author

Per discussion on that issue, the CodeSniffer folks agreed that it's a false positive and have fixed their detection logic.

@jessebeach
Copy link
Collaborator

@TrevorBurnham great research and follow-through on this one. Thank you so much!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants