maranran / eslint-plugin-vue-a11y

Static AST checker for accessibility rules on elements in .vue
MIT License
161 stars 21 forks source link

click-events-have-key-events: v-on:keyup.enter already triggers v-on:click event. #28

Closed SveinungTorresdal closed 3 years ago

SveinungTorresdal commented 4 years ago

Describe the bug When using a @click event, I'm prompted to add a @keyup.enter or other keyboard event. However, this frequently leads to "doubleclicks", i.e.:

<button @click="doSomething" @keyup.enter="doSomething">

In this example, pressing Enter triggers both @keyup.enter and @click.

To Reproduce Steps to reproduce the behavior:

  1. Create element with both @click and @keyup.enter
  2. Create function, logging when function is fired by the above.
  3. Use Enter key on element.
  4. Open Developer Tools -> Console.
  5. See function has fired twice: A keyboard event and a mouse event, if you examine the event.

Expected behavior Following eslint rules shouldn't cause strange, actual sideeffects.

Desktop (please complete the following information):

Additional context In my case, I detect the generated click event and return out of function with:

if(!event.sourceCapabilities) {return}

...or relevant variants where appropriate.

d13 commented 4 years ago

I'm also running into this same issue.

SveinungTorresdal commented 4 years ago

Following up on this, I discovered that only Chrome appears to have sourceCapabilities defined. So to not disrupt inputs in other browsers I needed to do some tweaking.

At minimum, if(!event.sourceCapabilities) {return} needs to be if(event.sourceCapabilities === null) {return}! This means in browsers other than Chrome they'll still pass, since it's undefined, not null.

Reversely, to address the original problem of fictional clicks, the fictive click is always sporting screenY and screenX as 0. So in my case, we are now trying if (event.sourceCapabilities === null || event.screenX === 0).

Hopefully that'll do it...

SveinungTorresdal commented 4 years ago

Okay, so after some more headache: The above doesn't work in Safari. All browsers tested generate a click when a keyup enter triggers. Detecting the fake click is a headache. So instead, here are a few alternatives.

Disable eslint for the html block

<!-- eslint-disable vue-a11y/click-events-have-key-events -->
<button @click="yourFunc">
    Your text
</button>
<!-- eslint-enable vue-a11y/click-events-have-key-events -->

Comply maliciously (but probably don't)

<button @click="yourFunc" @keyup.enter="() => true">
    Your text
</button>

Test for and discord keyboard input (not tested in Safari)

if (event.type === 'keyup' && event.code === 'Enter') {return}
TheJaredWilcurt commented 4 years ago

Are there any A11Y downsides to eslint-plugin-vue-a11y not reporting this error when the event is on a <button>?

SveinungTorresdal commented 4 years ago

Chrome, Firefox, Safari, Edge and IE11 all generated the @click event when pressing enter on a button. These browsers account for literally 90% of all users according to statcounter.

If you want to try and account for the remaining 10%, I recommend heavily investing in your alcoholic beverage of choice before diving in.

TheJaredWilcurt commented 4 years ago

A work around I have found is:

  <button
    @click="doThing"
    @keydown.prevent.enter=""
    @keyup.prevent.enter="doThing"
    @keyup.prevent.space="doThing"
  >
    Text
  </button>

Example: https://jsfiddle.net/frscqt0p

Explanation:

// Click event bound to method
@click="doThing"
// When you hold down enter, this prevents it from spamming the doThing function
@keydown.prevent.enter=""
// This binds the enter key to doThing, but prevents it from triggering the click event
@keyup.prevent.enter="doThing"
// This binds the space key to doThing, but prevents it from triggering the click event
@keyup.prevent.space="doThing"
SveinungTorresdal commented 4 years ago

That appears to a good alternative. I wish I'd thought of that. 😭 It completely escaped my mind to do that at the time.

AustinGil commented 4 years ago

@SveinungTorresdal Are you saying that in some browsers, the @click event is definitely NOT triggered for a <button> when the enter or space key is pressed?

AustinGil commented 4 years ago

After some chats with folks on Twiiter, apparently the updated version of this eslint plugin is https://github.com/vue-a11y/eslint-plugin-vuejs-accessibility

SveinungTorresdal commented 4 years ago

@SveinungTorresdal Are you saying that in some browsers, the @click event is definitely NOT triggered for a <button> when the enter or space key is pressed?

No, I'm saying that in all browsers I've tested the @click is generated when using enter key on a button, that these browsers account for literally 90% of all users, and if there is an exception in some obscure browser you should probably ignore it. 😅

Though @TheJaredWilcurt's workaround (adding .prevent) seems effective in bypassing the issue.

AustinGil commented 4 years ago

Gotcha. Well everyone that I spoke with, and whom I consider experts in a11y said that this is a false negative. That click events are triggered by default on buttons, so it's not necessary to add the key event handlers. And as per my previous comment, there is a more updated plugin for vue a11y eslint.

SveinungTorresdal commented 3 years ago

I just wanted to get back to this and point out that you probably shouldn't use @keyup.enter.prevent just to satisfy eslint since it (a) causes code duplication and added maintenance, and (b) causes side-effects that don't occur if you just let @click do its job itself.

We had a component where a button opens a dialog. This dialog had a button to confirm. An obnoxious issue we discovered is that pressing the Enter key on this original button caused the dialog to open, and immediately pressed the confirm button as well.

Simply not having @keyup.enter functionality reduced headaches; no more code duplication, less maintenance while developing, and the desired functionality with mouse and keyboard is still retained.

All said, this should mostly be moot if you're already using the newer plugin though.