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

ngAria should make ng-click as accessible for anchor tags as it does for DIV/LI #11053

Open
jorupp opened this issue Feb 13, 2015 · 12 comments
Open

Comments

@jorupp
Copy link

jorupp commented Feb 13, 2015

Changes for #10388 got rid of the extra events for BUTTON and A tags, but I think more could be done to make the accessibility more consistent. The browser already triggers the click event for A tags on enter if the href attribute is set, but never triggers the click event on space.

I think ngAria should hook keypress and trigger ng-click bindings on space, and trigger on enter if no href attribute is set.

This would result in ng-click behaving the same on BUTTON (implemented by the browser), A (browser + this change), DIV, and LI - triggering on click, enter, or space.

@Narretz
Copy link
Contributor

Narretz commented Feb 16, 2015

@marcysutton can you take a look at this? Thanks!

@Narretz
Copy link
Contributor

Narretz commented Feb 16, 2015

Just evaluating if it makes sense, that is ;)

@Narretz Narretz added this to the Ice Box milestone Feb 16, 2015
@marcysutton
Copy link
Contributor

I actually don't think this is a good idea. Anchors shouldn't have ng-click on them, that's the job for <button>. I made a comment about that here: #10388 (comment)

An anchor should have an href (or ng-href) taking the user to a resource or internal link. That's the purpose of an anchor. It fires a click event on ENTER when it has the required href attribute, which is the expected behavior for a screen reader user. Enabling an anchor tag without an href to behave more like a button doesn't make sense to me.

@jorupp
Copy link
Author

jorupp commented Feb 16, 2015

Agreed. Buttons should be BUTTONs.

I'm just saying it seems inconsistent to make ng-click trigger for space and enter for DIV and LI (which I don't see used very often), but not to do the same for A, which I see ng-click on all the time. Surely using DIV or LI where you should use a BUTTON is at least as wrong as using an A?

@marcysutton
Copy link
Contributor

I see your point. The purpose of ng-click support on DIV and LI was to "fix" existing code attached to those elements. If you're seeing anchors used this way in the wild, I can start to see how enabling invalid A tags fits in to the ngAria use case (thus creating a better "BandAid").

@marcysutton
Copy link
Contributor

Actually now I'm seeing the makings of accessibility problems, right in the Angular docs for a:

Modifies the default behavior of the html A tag so that the default action is prevented when the href attribute is empty. This change permits the easy creation of action links with the ngClick directive without changing the location or causing page reloads, e.g.: <a href="" ng-click="list.addItem()">Add Item</a>

Not only has a has been promoted over button for years, it's also really easy to leave off the href="". I do think ngAria could do more for keyboard users here. Thanks for talking through this, @jorupp.

@marcysutton
Copy link
Contributor

Just adding this here so I don't forget: if we do add support for anchors without href, we'll also have to add role="link" as they otherwise have the text role.

@motzko
Copy link

motzko commented Sep 11, 2015

hey guys--any update on this? semantically i agree buttons should be buttons but enabling ngAria for li and div elements kind of moots that argument.

if you're leaning towards not supporting i'll just create a css reset for inline button "links."

@gkalpak
Copy link
Member

gkalpak commented Sep 14, 2015

@marcysutton, wdyt should be done here ? Move it forward or close it as won't fix ?

@marcysutton
Copy link
Contributor

If someone wants to submit a PR, we'd be open to it. I'm not taking it on as I have other PRs to submit first, and this is a pretty low priority.

@gkalpak
Copy link
Member

gkalpak commented Sep 14, 2015

OK, leaving it open then. Thx @marcysutton !

@LeeAdcock
Copy link
Contributor

Could I get your feedback @marcfallows, @jorupp on #13996

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.

6 participants