[EuiBasicTable] Trigger onClick on keypress not keyup

[timroes]

The EuiKeyboardAccessible currently triggers the onClick when Enter or Space is pressed in the onKeyUp method. I think this should rather happen in the onKeyPress instead to be aligned with browser behavior.

Browser will trigger button clicks after the keypress, and thus also properly work with repeating key events, i.e. having a button focused and keep pressing Enter will result in the following event order:

• keydown
• keypress
• click
• keydown
• keypress
• click
• ... (keep repeating those while button is down)
• keyup

I think we should align the EuiKeyboardAccessible to also trigger the click on keypress not on keydown to (a) align with the standard browser behavior and (b) the current implementation can cause some very unnice side-effects, since the browser is triggering still on keypress. The way I run currently into that problem is:

I have a regular button that is on click closing a flyout. After that closing the last element on the page will regain it's focus: it was the "fake button" (something wrapped in EuiKeyboardAccessible) that opened the flyout. Since the click is triggered before the keyup, the "fake button" will now receive the keyup and trigger the click on it, directly reopening the flyout. The only way to currently work around that is to prevent all your real buttons that might run into that problem from triggerig their click on keypress, and instead manually trigger click on keyup (i.e. changing the browser's default behavior). I think thus we should consider aligning better to the browser behavior and trigger onClick on keyup.

I am not sure if there might be edge-cases I am overseing that this would cause a problem, thus I wanted to put this issue up to see if there are any concerns around it.

cc @chandlerprall @myasonik

[myasonik]

Yeah, the component definitely has ways that it could be improved but with such a broad charge as making accessible whatever it wrapped, it ultimately is next-to-impossible to make accessible for all cases and was often misused making matters worse.

Because of that, we deprecated EuiKeyboardAccessible (in #4135) and it is scheduled for removal in June.

It sounds like it would be best to migrate your case to using a native browser <button> element but I can't be sure without more details.

If you think there's a missing feature for EUI to cover that you're using EuiKeyboardAccessible for today, I'd open a new issue for the specific problem you want solved.

[timroes]

My "fake button" is actually a EuiBasicTable, with onClick listeners on its rows. Looking at the linked issue, it seems there is no plans right now for EuiBasicTable and the deprecation of EuiKeyboardAccessible, thus I'd consider that issue to still be relevant, since EuiBasicTable is a widely used component, that this problems applies then to. Reopening the issue and renaming it.

[myasonik]

++ for sure, this would be good to shim in EuiBasicTable until we figure out how to migrate it away from EuiKeyboardAccessible

[github-actions]

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

[github-actions]

❌ We're automatically closing this issue due to lack of activity. Please comment if you feel this was done in error.