Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngAria should check for interactive child elements before binding role=button and tabindex=0 #14672

Closed
marcysutton opened this issue May 25, 2016 · 3 comments

Comments

@marcysutton
Copy link
Contributor

marcysutton commented May 25, 2016

I've seen this bug in the wild on a large web application using ngAria: ng-click tries to help accessibility by binding role="button" and tabindex="0", unless opted out. The problem is if that DIV with an ng-click wraps another interactive element such as <button> or <a>, the extra tabindex and role cause problems.

Current behavior

tabindex="0" on everything causes keyboard hell with often-redundant tab stops; and the role=button wrapping other content confuses screen reader users.

Here is a Plunker showing the wrapping interactive elements, prompted from #14665: https://plnkr.co/edit/SPxQ0X2RQvvfvAmDVTNA?p=preview

What is the expected behavior?

If we can do it fast enough, ngAria should test an ng-click target's child elements against the nodeBlacklist and return false on the first instance, leaving off tabindex="0" and role="button". The DIV with ng-click will then stay a mouse-only element to prevent degrading accessibility in the rest of that DOM subtree.

What is the motivation / use case for changing the behavior?

ngAria currently causes more problems than it solves with "fixes" like the one ng-click tried to solve. Addressing this problem would improve the utility of the library.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Visible in the latest version.

@Narretz
Copy link
Contributor

Narretz commented May 25, 2016

One problem I see is that this won't work for dynamically loaded content (templateUrl, ngInclude), but also for content that changes dynamically, like ngIf.
The other option would be to to look up in the DOM from interactive elements and then remove tabindex from them when they have been found. But this could incur a big performance penalty.

@gkalpak
Copy link
Member

gkalpak commented May 25, 2016

The other option would be to to look up in the DOM from interactive elements and then remove tabindex from them when they have been found.

That would also break usecase, where the tabindex/role are intended in the parent elements (for whatever reason).

Indeed, taking dynamic elements into account, I feel that this would be too much magic to implement - and it wouldn't be possible to cover all usecases either.

Better alternatives might be:

  1. Document this as a known issue and let the developers explicitly prevent the addition of tabindex/role (by supplying their own).
  2. Provide a way (e.g. via a new attribute) to opt out of ngAria for specific elements. (This has been brought up in Allow for ngAria to ignore specific elements #14602.)

@marcysutton
Copy link
Contributor Author

Those are really great points, thanks for the feedback. I agree that going up the DOM tree from interactive elements would be too costly. It would also be tough to know which attributes were added intentionally without keeping track of everything...again, a huge cost for a click handler.

I like the idea of an attribute for opting out of ngAria on specific elements. I still think there will be a lot of ng-click abuse, but at least if we had a better hook it would be easier to do the right thing. Paired with additional documentation, that would set developers up a lot better than the current situation.

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

Successfully merging a pull request may close this issue.

3 participants