[Chip] Keyboard events regression

[AryamnovEugeniy]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The onClick event is fired after the onKeyDown event and it is fired as many times as onKeyDown instead of being fired only once after the onKeyUp event. It seems that this is a regression because a similar bug has already been fixed (#12449).

Expected Behavior 🤔

The onClick event should be fired once and only after onKeyUp as in the native button.

Steps to Reproduce 🕹

4.6.0. Chip doesn't work correctly
https://codesandbox.io/s/create-react-app-fmq1b
4.5.2. Chip works as expected.
https://codesandbox.io/s/create-react-app-8o25z

Steps:
Add the following code:

<Chip
  label="Focus me and press"
  onClick={() => console.log("onClick")}
  onKeyDown={() => console.log("onKeyDown")}
  onKeyUp={() => console.log("onKeyUp")}
/>

Your Environment 🌎

Tech	Version
Material-UI	v4.6.0

[eps1lon]

The onClick event is fired after the onKeyDown event and it is fired as many times as onKeyDown instead of being fired only once after the onKeyUp event.

You probably mean for Space. Because for Enter it works the same for the native button?

[AryamnovEugeniy]

The best way for me would be to call onClick after onKeyUp, as it worked in 4.5.2, since the only way I found to change this behaviour is to override component, which is not very suitable for me. Is it possible or should I override component? Or is there any other workaround to achieve the 4.5.2 version's behaviour?

[oliviertassinari]

@AryamnovEugeniy what browser are you using?

On macOS 10.14.6: Chrome 78.0.3904.70, Firefox 69.0.3, and Safari 13.0.3, the ButtonBase behaves like the native button for the Enter key and behaves the opposite for the Space key.

[AryamnovEugeniy]

@oliviertassinari,

I used Chrome 78.0.3904.87 and Firefox 69.0.3 and got the same behaviour: the ButtonBase behaves like the native button for the Enter key and behaves the opposite for the Space key. The problem for me is that the Chip behaved differently in the previous version. The question now is: is the behaviour you described correct and if it is, then, is there any other way to achieve the behaviour from the previous version except overriding Chip's component prop? Because I wasn't able to override completely the 'onKeyDown' event so that onClick wouldn't be called after 'onKeyDown' but would be called after onKeyUp.

[oliviertassinari]

@AryamnovEugeniy Why is the order important in your case?

[eps1lon]

The problem for me is that the Chip behaved differently in the previous version.

Then this was a bug. click is supposed to fire before keyup for Enter keys. For space click needs to fire after keyup.

[AryamnovEugeniy]

Hello again,

@oliviertassinari, sorry for late response. I have a certain event which is triggered on mouse click, space click or enter click. And it is desirable that this event should be triggered only once. In case onClick is fired after every onKeyDown, my event is also triggered every time onKeyDown is fired. And if a user holds enter or space button for too long, the event will be triggered many times. That's why the order is important for me. And the main problem is that, as it seems to me, I cannot completely override Chip's onKeyDown.

[eps1lon]

This issue was introduced in #17829 and uncovered an underlying issue with our ButtonBase implementation. ExpansionPanel has the same issue.

[oliviertassinari]

It seems that the problem was solved in #18319.

[eps1lon]

It seems that the problem was solved in #18319.

Correct: https://codesandbox.io/s/create-react-app-tzkvu

[AryamnovEugeniy]

Thank you very much for your quick responses!!