Refactoring to suppress eslint warnings jsx-a11y/click-events-have-key-events

[christyjacob4]

Please review and merge #3974 before this.

Refactoring to suppress eslint warnings

Upon enabling jsx-a11y/click-events-have-key-events flag in .eslintrc , a couple of warnings are raised because it is recommended
to provide a onKeyPress, onKeyDown or onKeyUp event handler for every onClick event handler.

The code has been refactored to follow the eslint spec.

Fixes #3926

Signed-off-by: Christy Jacob christyjacob4@gmail.com

[cpretzer]

Ran the code locally and everything worked as expected with no eslint warnings.

Nice work @christyjacob4

[cpretzer]

@christyjacob4 I see that you merged master in to this branch in order to resolve conflicts with .eslintrc

Another option is to rebase your branch from master. The merge is totally fine, just want to let you know that rebasing is different option with different effects: https://git-scm.com/book/en/v2/Git-Branching-Rebasing

The main difference is that rebasing results in a more uniform history with master and sibling branches created from the same master.

[christyjacob4]

@cpretzer Thanks a lot for the insight. I will keep that in mind from next time 👍

[Pothulapati]

@christyjacob4 Thanks for working on this,
I just built the dashboard and it works fine. Is there a user-facing change on this i.e making some action be performed by pressing Enter? I tried it for some popups i.e at Linkerd Check, etc but it didn't work. :)

[christyjacob4]

@Pothulapati I didn't quite get you.. could you please repeat?

[Pothulapati]

@christyjacob4 Ah Sorry, Looked like the fix was to add a key event handler for every onClick event and the key is Enter. Does this allow us to press enter button instead of performing a click. I tried doing that on the dashboard but it did not work. :)

[christyjacob4]

@Pothulapati I will send you a gif to make it clear.
The Popover.js file is used only in Tap
So this allows us to use the Enter key only on the Tap Component

[christyjacob4]

@Pothulapati @cpretzer Please take a look at this gif for more context.

[Pothulapati]

Awesome, Thanks :)

[alpeb]

Thanks @christyjacob4 , looks good to me 👍
Before we merge, could you please remove the "jsx-a11y/click-events-have-key-events" entry from .eslintrc instead of setting it to 1?

[christyjacob4]

@alpeb Thanks for noticing that. I will do that :)

[christyjacob4]

@alpeb I think the changes are ready :)

[alpeb]

Thanks @christyjacob4 , this looks good to me!
Have you already received some swag from us? If not, do you mind filling out this survey to get you some? 😉

[christyjacob4]

@alpeb Thanks a lot :) . This is my second issue on linkerd :) I will fill out the survey now.
I'm currently working on #3204
So could you please check my comment and help me out over there?
Thanks again

[Pothulapati]

@christyjacob4 Looks like we have merge conflicts again. :\
Can you fix those conflicts? :)

[christyjacob4]

@Pothulapati Its done :)