Angular.js: ngAria : role="button" added with ng-click directive cannot be disabled and is dangerous

[pgayvallet]

Automatically adding unwanted role="button" on elements with ng-click directives can be very dangerous with some screen reader's behaviour. This ngAria feature cannot be disabled in ngAria config, which force developper to add a role attribute to element with ng-click when they dont want ngAria to add the button role to their clickable elements.

[Narretz]

Can you explain why this is dangerous?

[Narretz]

/cc @marcysutton

[marcysutton]

Some specific examples would help. There is a node blacklist preventing it from being added to certain elements, which can be expanded. I'd argue it's more dangerous to put ng-click on elements that aren't interactive, and it isn't difficult to override the role.

[pgayvallet]

Sure, I can explain some specific example

We had the issue with a a list of radio button :
Each radio is composed of a div container, with an ng-click handler containing the radio button itself. (The ng-click handler was here for mobile device purpose, to increase the click zone to select the radio buttons - ng-click='set-model-to-value-of-radio, basically).

With this configuration ( div role=button label input type=radio /div) VoiceOver (and maybe NVDA too, but not 100% sure) simply dont understand there is nested elements, stoping at the button role, with make the radio list description broken on the screen reader.

Sure, adding role="presentation" to the div do fix the issue. But honestly, I could find 100 more case where I can use a click handler on an element with simply doesnt match the "button" role, and forcing the developer to verbosely add a dummy role simply to say "hey, plz, do not mess around with inappropriate button role" seems unecessary.

Allowing the feature to be disabled ( it's the ONLY one ngAria doesnt allow to disable ) seems... legitimate imho.

[marcysutton]

Cool, thanks for the information. We can totally make that configurable.

[OlsonDev]

I just ran into this myself. Until those commits are available on bower, I threw a role="presentation" tabindex="-1" on my container. Is there a declarative way to change the focus to my <md-switch> inside the container like a native <label for="some-id"> would do? Or do I have to write some JavaScript to do that in my ng-click handler?

In case my question isn't clear, this is a snippet I'm messing around with:

<div layout ng-click="fullscreen = !fullscreen;" role="presentation" tabindex="-1">
    <div flex layout layout-align="start center"><md-icon class="md-primary">fullscreen</md-icon> Fullscreen</div>
    <md-switch flex="5" class="md-primary" ng-model="fullscreen" aria-label="Fullscreen"></md-switch>
</div>

[marcysutton]

@OlsonDev that's an acceptable approach for the wrapping element with role=presentation and tabindex. :+1:

Your question is unrelated to the reported issue, so let's try to keep it short. Are you saying a click on the wrapping div should send focus to the switch? Given md-switch is a custom component, it lacks some of the native behavior of a label and input. You could watch fullscreen and if it changes value, send focus to the switch, or send focus directly to the switch when the div is clicked. The former option seems cleaner (especially if you need the fullscreen property for other things), but it relies on a watch.

[OlsonDev]

@marcysutton My question was unrelated to the reported issue, sure. But the OP's real-world example was near-identical to mine so I figured it was related enough to ask. Regardless, apologies if I hijacked the issue; I'll try to keep it short.

Correct, the wrapping element should send focus to the switch on click/tap. Fair that it's custom and doesn't support native events. I don't think watching fullscreen and focusing on change makes sense; if an external event causes it to change, it effectively focus-steals. However, delegating the interaction (down the DOM) in an event handler does make sense to me. To make this functionality reusable, I made a directive; I think it's pretty clean (albeit perhaps not feature complete). Check it out. @wayofspark, you may find it useful, too.

[marcysutton]

That totally works. It's much easier to reason about in live demo form. :)
In your scenario, event delegation from the wrapping element enhances the experience for mouse users, while the switch itself is keyboard accessible and labeled. Nice one!