feat(ngAria[ngClick]): anchor should treat keypress as click #13996
Conversation
…hor, button elements Enhances ngAria to trigger ngClick for 'space' keypresses on anchor and button elements, and for 'enter' keypresses on anchor tags when there is no value for the href attribute. Closes #11053
src/ngAria/aria.js
Outdated
| @@ -377,6 +377,27 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |||
| }); | |||
| } | |||
| } | |||
|
|
|||
| if ($aria.config('bindKeypress') && isNodeOneOf(elem, ['A', 'BUTTON'])) { | |||
There was a problem hiding this comment.
Actually, we don't want to do anything with buttons, just as. The browser handles pressing both ENTER and SPACE on buttons.
There was a problem hiding this comment.
You should also check that attr.ngKeypress is falsy.
|
I left some comments (that should be easy to address). Additonally, according to #11053 (comment), we should also bind the @LeeAdcock, feel free to ping when/if you've made any changes, so I can take another look. Thx ! |
|
Thank you for your feedback. I'm made the suggested changes, but I believe there's still a little bit of work to go.
Please let me know which direction you'd like me to go. @gkalpak |
|
I continued to iterate on these changes, and believe the PR now aligns with the intent of the issue and provides what I would have expected from ngAria for enhanced accessibility for links. This results in the same behavior for mouse clicks, the space key, and the enter key. The I would suggest that the previous behavior might be more than an inconvenience. We have written our own custom directives to work around these shortcomings, since the current Angular behavior fails accessibility standards. Feedback welcome, @gkalpak. |
LeeAdcock
changed the title
| elem[0].click(); | ||
| } | ||
|
|
||
| function callback() { |
There was a problem hiding this comment.
It looks like callback is never used inside this code path.
|
Thanks for working on this! Keep in mind the expected keyboard interaction for links is ENTER, not SPACE. That is more of a typical button interaction. |
| if ($aria.config('bindRoleForClick') && !attr.href && !attr.xlinkHref && !attr.role) { | ||
| elem.attr('role', 'link'); | ||
| } | ||
| if ($aria.config('bindKeypress')) { |
There was a problem hiding this comment.
To avoid unnecessary work, we should check for !attr.ngKeypress here (instead of inside the listener below).
But native buttons do respond to SPACE too. So, our "custom button band-aid" should handle that as well, right ? |
| } | ||
| if ($aria.config('bindKeypress')) { | ||
| elem.on('keypress', function(event) { | ||
| var keyCode = event.which || event.keyCode; |
There was a problem hiding this comment.
This section looks an awful lot like https://github.com/LeeAdcock/angular.js/blob/ngclick-accessible-11053/src/ngAria/aria.js#L367 above. I wonder if we could combine those two sections somehow, perhaps with a reusable function?
Enhances ngAria to trigger ngClick for 'space' keypresses on anchor and button elements, and
for 'enter' keypresses on anchor elements when there is no value for the href attribute.
Closes #11053