Skip to content
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

[Accessibility] Remove Availability Zone button does not respond to return or enter key #28159

Closed
aphelionz opened this issue Jan 7, 2019 · 4 comments
Assignees
Labels
Feature:Metrics UI Metrics UI feature Project:Accessibility Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services WCAG A

Comments

@aphelionz
Copy link
Contributor

aphelionz commented Jan 7, 2019

Steps to reproduce (assumes ChromeVox or similar)

  1. Navigate to Infrastructure
  2. Using the keyboard or mouse, add some items to the group by filter
  3. Using the keyboard ONLY, try to remove them by tabbing and pressing enter

screenshot_20190106_200155

Actual Result

  1. Items cannot be removed

Expected Result

  1. Items are removed via the enter key

Meta Issue #26854
Kibana Version: 6.4
Relevant WCAG Criteria: 2.1.1 Keyboard

@aphelionz aphelionz added Project:Accessibility Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Jan 7, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infrastructure-ui

@aphelionz aphelionz mentioned this issue Jan 7, 2019
6 tasks
@Kerry350
Copy link
Contributor

Kerry350 commented Jan 22, 2019

The root cause of this is conflicts between two onClick handlers in the EUI components being used.

This particular part of the infrastructure UI renders a nested structure like so:

screenshot 2019-01-22 15 10 26

The <EuiFilterButton> renders out a <button> which will automatically handle keyboard controls, calling the onClick prop when Enter is pressed, in this case onClick handles toggling of the context menu. And it wraps all of the children.

And then within the <EuiFilterButton> there exists the <EuiIcon> for the "X" icon which attaches keyboard controls manually via tabIndex and event handlers (the wrapping <EuiKeyboardAccessible /> component provides / adds this functionality).

The reported behaviour happens as the <EuiFilterButton> (outer component) callback fires, whilst the <EuiIcon> (inner component) callback doesn't (although only sometimes).

Ideally we'd want to see behaviour where the <EuiIcon> callback responds first, as that's the element tabbed to, and then propagation is stopped.

I'll chase up if this is a problem with the way we're using / nesting EUI components, or if there's a bug within the EUI components themselves that needs fixing.

@chandlerprall
Copy link
Contributor

Explanation of what's happening -

The badge's x has a keyup event listener applied by EuiKeyboardAccessibility, if it detects a [Space] or [Enter] key it triggers the iconOnClick callback prop. This iconOnClick handles removing the selected item from the group by.

Wrapping the badge, the EuiFilterButton has a click handler.

For accessibility reasons, browsers will trigger a click event in response to space or enter key presses. For some reason, the order of keydown, keyup, and click events generated by a key press is different between [Space] and [Enter]. Tested this in Chrome, Firefox, and Safari:

Tab to an element and press [Space]:

  • keydown
  • keyup
  • click

Tab to an element and press [Enter]:

  • keydown
  • click
  • keyup

In this case, when [Enter] is pressed the event bubbles up to the event filter button triggering its onClick before the badge's iconOnClick is triggered by the keyup event.

Elements that are not input-like (form fields, buttons, etc) do not have click events fire on them and only receive keydown and keyup. The badge's x icon is not input-like, which is the reason the UX manifests in this way -

  • tab to x icon and press [Enter]
  • keydown fires on icon
  • click event triggers, starting at the closest input-like ancestor - EuiFilterButton

This is, unfortunately, working as designed because of the event order change that browsers implement. As a workaround, Kibana could wrap the badge with an element that will process the click event as it bubbles up and stop propagation. On the EUI side we could change EuiKeyboardAccessibility to trigger the onClick prop on keydown instead of keyup, though I'm not sure if this could have undesirable consequences.

@Kerry350
Copy link
Contributor

@chandlerprall Thanks for taking a look in to this. This exact event order issue has bitten me before in previous work, should have spotted it 😬

As a workaround, Kibana could wrap the badge with an element that will process the click event as it bubbles up and stop propagation. On the EUI side we could change EuiKeyboardAccessibility to trigger the onClick prop on keydown instead of keyup, though I'm not sure if this could have undesirable consequences.

Yeah, I'll look to add something on the Kibana side. I agree that, as innocent as it seems, changing from keyup to keydown within EUI could have undesirable, and cascading, consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature Project:Accessibility Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services WCAG A
Projects
None yet
Development

No branches or pull requests

7 participants