Warn when using onclick on non-clickable elements

[tricinel]

Describe the new a11y feature or project enhancement

I see this quite often in code bases, where devs will attach an onclick to some div or span and call it a day. This breaks keyboard navigation. Worse...sometimes there are onclick handlers that will just navigate the user to another page. This specifically fails Success Criterion 1.3.1 and 2.1.1.

Describe the solution you'd like

I quickly mocked this up in codepen: https://codepen.io/tricinel/pen/VwjZEPK.

I'm not sure if we should be that aggressive and ban all elements except buttons and anchor tags.

Link(s)

https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/F42

[jackdomleo7]

I like this idea and I like when you said, "I'm not sure if we should be that aggressive and ban all elements except buttons and anchor tags."

Consider the below CodePen for example, it uses onlick on the overlay <div> to trigger the functionality to close the modal. This is perfectly fine because the user has an alternative means of closing the modal in an "accessible way" by clicking or tabbing to the close <button>. I wouldn't want to flag up something like this as being false or inaccessible. 😊

CodePen: https://codepen.io/jackdomleo7/pen/yLJLOQr

Maybe we could show them as warnings?

[tricinel]

Yes, I agree. Hm...the problem with that approach is that clickable elements still need to be focusable. onclick turns the div into a clickable element, but it cannot receive keyboard focus (https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Keyboard). So we'd need to make sure they have keyboard handlers or make sure it has the aria-hidden attribute.

I think you're right. A warning like you're suggesting would work better. There's too many variables to consider.

PS: For this specific example, I'd still use a button as the overlay and style it to not look like a button :)

[jackdomleo7]

I think this may need a little more thought. 😊 I'd strongly argue the overlay doesn't need to be a <button> because it would confuse the keyboard user. Clickable divs are ok as long as you provide alternatives. In this case, the user does not need to click the overlay because the close <button> is an alternative for them.

[tricinel]

Yep, I was agreeing :) Probably just warning would suit this better.

[jackdomleo7]

Sorry, I misudnerstood 😅 Yes this request has sparked another idea I have, I'll have to think on it. But maybe a warning message like "WARNING: Using a click event here may be inaccessible. Either make this a clickable element such as or or ensure you have provided an alternative accessible means of performing the same fuctionality."

That message is probably a bit long.

[jackdomleo7]

What elements should we check for or not check for? Only <button> or <a>?

[tricinel]

...a warning message like "WARNING: Using a click event here may be inaccessible. Either make this a clickable element such as or or ensure you have provided an alternative accessible means of performing the same fuctionality."

How about

WARNING: Potentially inaccessible click event used on non-clickable element. Ensure you have an accessible alternative

[tricinel]

What elements should we check for or not check for? Only <button> or <a>?

This a good question! If we go with a warning, would it be sensible to check for all elements that are not button or anchors?

[alvaromontoro]

#48 is a similar issue. I'll close it as a duplicate of this one because the conversation here goes beyond the other one.

From that issue, consider adding more events than [onclick]. Some of them can't be triggered with the keyboard at all (not even in the case of a button/a):

[onclick],
[ondblclick],
[onmouseover],
[onmouseenter],
[onmouseleave],
[onmousedown],
[onmouseup] {
  /* warning/error */
}

Also, the selector from the shared demo would need to be changed slightly. The current one will throw false positives as it targets a and button separately. It should consider both together in the same rule:

*:not(a)[onclick]::after,
*:not(button)[onclick]::after {
  /* this will give false positives for `<button onclick="doSomething()">text</button>` which is valid */
}

[onclick]:not(a):not(button)::after {
  /* this will target elements with inline onclick that are not `a` or `button` */
}

[tricinel]

@alvaromontoro very good point, thank you! I will incorporate those as well.

EDIT: Wait...I don't know if this is a go yet :D

[alvaromontoro]

About the overlay exception, I would still show the warning because –and I understand that this is a personal choice and not exactly related to a11y:

• Inline event handlers should be avoided when possible, to have a separation between functionality and presentation (see unobstructive JavaScript).
• The overlay would probably be hidden by default so no error would be displayed on the screen anyway.

[tricinel]

I would agree with that. 👍 I'll put together a pen to see how it would work, if any other exceptions come to mind.

[jackdomleo7]

Awesome! I'll assign you for the time being 😊

[tricinel]

That's perfect, thanks @jackdomleo7!

This is what I came up with and seems to do the job: https://codepen.io/tricinel/pen/VwjZEPK

And then I figured, well, with some work you can make a div be accessible, keyboard and all, so I tried an experiment: https://codepen.io/tricinel/pen/KKMKYya. But I haven't figured out yet why the selector doesn't work. :( If you have any ideas, I'd appreciate them.

The experiment might be not worth it in the end, but still figured it was worth the hassle to at least see if it's possible.

Thanks!

[jackdomleo7]

It's always good to do some proof of concept! 😊

Hmm... That is odd. I'm determined to get that second one working. Let me take a closer look and see what's going on.

If anyone else can figure it out, please shout out!

[tricinel]

Weirdly enough, different combinations of the :not selector will work, but I haven't yet figured out why...Order shouldn't matter as far as I know...

Looking forward to seeing what you come up with.

[alvaromontoro]

Based on the previous examples, I tried a slightly different approach (hopefully not too overcomplicated). It has an explanation of the rules: https://codepen.io/alvaromontoro/pen/JjKdXBQ.

But it doesn't take into account aria-label and aria-labelledby. Do we need to check for them? I don't think we do, but I may be missing something.

[tricinel]

@alvaromontoro I think it works without, as long as the div has an actual text element, or something labelled properly. Maybe we can discount that...as in, that should be a separate issue/warning.

Your demo looks good!

[tricinel]

@alvaromontoro @jackdomleo7 what are we thinking on this? I played around a little bit more on a side project with Alvaro's implementation for onclick and it worked fine.

Would we use that onclick implementation, combined with the other on* mouse/touch events, showing the same warning?

[jackdomleo7]

I think I'm confident with @alvaromontoro's build on your build @tricinel. I think you've both established something reliable there and from the testing I've been able to do, there are no immediate things that I can see as "wrong". So I'm happy with this 😊

[tricinel]

Sweet! Sounds great! @alvaromontoro do you want to take it or should I?

[tricinel]

FYI, @alvaromontoro @jackdomleo7, I'm working on this...should be done today. 🚀