Skip to content

Improve click event handler accessibility warning #8001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brunnerh opened this issue Nov 8, 2022 · 16 comments · Fixed by #9358
Closed

Improve click event handler accessibility warning #8001

brunnerh opened this issue Nov 8, 2022 · 16 comments · Fixed by #9358

Comments

@brunnerh
Copy link
Member

brunnerh commented Nov 8, 2022

Describe the problem

There is a this warning when adding a click event handler to non-interactive elements:

A11y: visible, non-interactive elements with an on:click event must be accompanied by an on:keydown, on:keyup, or on:keypress event.

For users that do not know what this is really about, the only clear instruction here is: Add more event handlers.

Which is backwards and will not even work half of the time because the element would also have to be focusable and semantics will still be missing for screen readers.

Describe the proposed solution

Start the warning with a suggestion to use the appropriate elements:
Use a button element for actions, use an a element for navigation.

A note on button behavior in forms may be helpful. I.e. the default type of buttons is submit and that one might want to add type="button" to change this.

(The documentation for the warning should also include this as the primary suggestion.)

Alternatives considered

N/A

Importance

nice to have

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Nov 8, 2022

This is not the only a11y warning where the text itself ironically is anything but accessible. This sounds almost like legal speak. They are using words such as "must be" or "cannot", but technically that's not true at all. Of course I can have a positive tabIndex (#7953), but it makes it less accessible. And I surely don't need to add any on:key* events when using on:click (e.g. for event bubbling the click events).

Even just rewording the current text without adding any new information makes it so much more accessible and friendly:

A11y: You've added an on:click event handler to a visible, non-interactive element. To make it accessible you should add on:keydown, on:keyup, or on:keypress events.

Now this is still confusing as hell (see #7958), so let's add some more context:

A11y: You've added an on:click event handler to a visible, non-interactive element (such as <div> or <span>). It's recommend to use appropriate elements such as <a> or <button>. If you cannot change the element type, you should add on:keydown, on:keyup, or on:keypress events to make them accessible (e.g. so that pressing the Enter key clicks them).

Much better. But even this leaves question marks even in my own face. Is Enter actually the only key we care about (so that you can click the element with the keyboard)? I honestly don't know. And I'm sure the three keyboard events aren't equal either (e.g. keydown vs keyup have different semantics just like you wouldn't expect a click from just a mousedown). That means even recommending adding on:key* events without any further guidance leads to things like #7958 and people will literally just add a noop event handler because they don't know any better.

@brunnerh
Copy link
Member Author

brunnerh commented Nov 8, 2022

Actually, button triggers on Space or Enter, while a only triggers on Enter. And as noted, the key events would not even do anything without adding tabindex to make the element focusable (which causes new warnings of its own).

@brennanyoung
Copy link

As well as key handlers and tabindex you also need to add an appropriate role attribute, because <div> and <span> do not support naming unless an appropriate semantic has been specified.

All operable elements must have machine-readable accessible names, semantic roles and (if applicable) state values to conform with WCAG SC 4.1.2 Name, Role, Value.

@archiewood
Copy link

Something I also discovered here, leaving in case useful for others.

If you change eg a <div> with an on:click event to a <button> to remove this warning, then you need to add a prop type=button too, or else the button can work unexpectedly!

@Prinzhorn
Copy link
Contributor

@archiewood

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attr-type

submit: The button submits the form data to the server. This is the default if the attribute is not specified for buttons associated with a <form>, or if the attribute is an empty or invalid value.

@brennanyoung
Copy link

Good catch @archiewood . So, a warning about the submit functionality would only be appropriate if the button is a descendant of a form, right?

BTW. The default behavior when pressing SPACE is to scroll the document unless an operable element (such as a button) is in focus, in which case, a click event should be sent to the element. This is handled automatically by user agents, without extra code if you use <button> rather than <div> or <span> or any other element with a non-operable semantic.

Ideally, any warning should "nudge" developers towards best practices: i.e. the first choice should be to use an element that is operable and focusable by default, namely <button>, <a>, <input>, <select>/<option> or <area> if at all possible. This gives a solid baseline for accessibility without having to worry about extra event handlers, tabindex or role attributes.

BTW: The reason <a> only responds to ENTER and not SPACE is because <a> was primarily imagined to be part of the text/content flow - i.e. hyperlink text is for primarily for browsing, rather than interaction - the navigation feature is secondary to the fact that a link contains (or should contain) actual content. (This is a subtle distinction which is very important for screen reader users). Therefore, pressing SPACE when <a> is in focus will (be expected to) scroll the document.

@tobylewis
Copy link

It seems that there is a lot of knowledge required to be able to fix this error. Unfortunately that is something most developers will not have and even those who would love to comply will struggle, and the results may actually be worse than ignoring it. The more I have searched for help the harder it seems to be to fix this.

The fundamental problem seems to be that html was invented as a publishing/reading medium and it is now being used as an interactive UI. To only have two clickable elements "A" and "BUTTON" seems a bit lacking. DIV is a block element that comes with no styling unlike P. We need a DIV equivalent for BUTTON, ie something recognised as clickable but without the styling of buttons and then readers could handle it intelligently. That is something for HTML standards to sort out.

But in the meantime, it would be nice if Svelte could offer something like <svlelte:interactive class="listitem" on:click={openitem}>Item text</svlelte:interactive>

The element would handle the boilerplate things that need to be done for an clickable element and ask the developer to provide attribute values if these are needed (things like alt text or role whatever cannot be determined generically). This would be a win win win as it is good for accessibility, developers and svelte.

@brunnerh
Copy link
Member Author

brunnerh commented Jan 9, 2023

I don't really see the problem. You can style buttons however you want, including the removal of all styling, and they can contain just about any content, including divs.

@tobylewis
Copy link

That is how I have approached it; just use buttons and remove styling. We could also get rid of DIVs out HTML and just use P and remove the styling too. It's just a question of semantics and convenience.

@brunnerh
Copy link
Member Author

brunnerh commented Jan 9, 2023

You cannot get rid of divs precisely because of the semantics. p has the semantics of "paragraph" and there is a need for an organizational element without any semantics. One could argue that there is some redundancy with div and span, i guess.

(Sorry for being a bit off-topic here.)

@brennanyoung
Copy link

brennanyoung commented Jan 10, 2023

The non-semantics of div (and span) are a feature not a bug.

They allow content to be structured and ordered (important for meeting WCAG's "Meaningful Sequence" criterion ) without cluttering the accessibility tree, and of course they provide valuable hooks for visual presentation (especially for CSS selectors).

Using <button> and overriding user agent styles is compellingly The Right Thing to do for clickable elements that do something other than navigate or download. There are only special cases where alternative approaches are more appropriate.

Various articles and stack overflow posts cover the topic of overriding user agent style for <button>, so the information is available, but not as unambiguously recommended as it might be. An error message can really help here.

Using div or span as operable elements without providing cues (i.e. role, tabindex and event handlers) for assistive tech users is simply a developer mistake (or -dare I say- just developer ignorance). The error text should briefly capture the essence of this guidance.

@benmccann
Copy link
Member

I tracked down the location of the message:

'A11y: visible, non-interactive elements with an on:click event must be accompanied by an on:keydown, on:keyup, or on:keypress event.'

@brunnerh
Copy link
Member Author

@benmccann Is that a subtle suggestion that someone should just go ahead and change it already? 😅

@benmccann
Copy link
Member

too late: #9358

@crummy
Copy link

crummy commented Apr 20, 2024

I don't really see the problem. You can style buttons however you want, including the removal of all styling, and they can contain just about any content, including divs.

My IDE complains Element h1 is not allowed here when I use it inside a <button>.

It seems that there is a lot of knowledge required to be able to fix this error. Unfortunately that is something most developers will not have and even those who would love to comply will struggle

This is exactly how I feel right now!

@brunnerh
Copy link
Member Author

You can put the button in the heading instead.

Information on what elements can contain can be found on MDN or in the specification.
For buttons:

Permitted content: Phrasing content but there must be no Interactive content

For headings it's also phrasing content.

List of elements from MDN
  • <abbr>
  • <audio>
  • <b>
  • <bdi>
  • <bdo>
  • <br>
  • <button>
  • <canvas>
  • <cite>
  • <code>
  • <data>
  • <datalist>
  • <dfn>
  • <em>
  • <embed>
  • <i>
  • <iframe>
  • <img>
  • <input>
  • <kbd>
  • <label>
  • <mark>
  • <math>
  • <meter>
  • <noscript>
  • <object>
  • <output>
  • <picture>
  • <progress>
  • <q>
  • <ruby>
  • <s>
  • <samp>
  • <script>
  • <select>
  • <slot>
  • <small>
  • <span>
  • <strong>
  • <sub>
  • <sup>
  • <svg>
  • <template>
  • <textarea>
  • <time>
  • <u>
  • <var>
  • <video>
  • <wbr>
  • plain text (including more than just whitespace characters)

A few other elements belong to this category, but only if a specific condition is fulfilled:

  • <a>, if it contains only phrasing content
  • <area>, if it is a descendant of a element
  • <del>, if it contains only phrasing content
  • <ins>, if it contains only phrasing content
  • <link>, if the itemprop attribute is present
  • <map>, if it contains only phrasing content
  • <meta>, if the itemprop attribute is present

If you combine elements the wrong way, your page will sometimes just break.
You can run into such problems in other places as well, so I think it's good to know how to work around or fix such issues in general.

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

Successfully merging a pull request may close this issue.

7 participants