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.

Sign up for GitHub

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

Jump to bottom

(Accessibility) [Controls] Range slider a11y + rendering issues #135466

Closed
Tracked by #135468
Heenawter opened this issue Jun 29, 2022 · 5 comments · Fixed by #159271
Closed
Tracked by #135468

(Accessibility) [Controls] Range slider a11y + rendering issues #135466

Heenawter opened this issue Jun 29, 2022 · 5 comments · Fixed by #159271
Assignees
@Heenawter
Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Accessibility Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas WCAG A

Comments

@Heenawter
Copy link
Contributor

Problems with the main component

Because of the our custom styling for invalid selections, rather than using the default EuiDualRange with showInput="inputWithPopover", we had to implement our own custom button for the range slider component. However, this results in both the outer button and the two inner min/max values all being selected, which isn't compliant with WCAG.

To handle this, we could either:

a. File an enhancement request with the EUI team to support the custom styling we want or
b. Figure out how to make only the inner components interactive and skip over the parent button

Nested interactive controls are not announced by screen readers
https://dequeuniversity.com/rules/axe/4.4/nested-interactive?application=AxeChrome

image

Buttons must have discernible text
https://dequeuniversity.com/rules/axe/4.4/button-name?application=AxeChrome

Problems with the popover component

Although we could currently fix this by adding an aria-label to the EuiDualRange component that we are currently using, depending on how we fix the above issue, this fix will change.

Nested interactive controls are not announced by screen readers
https://dequeuniversity.com/rules/axe/4.0/aria-input-field-name?application=axeAPI
Related: #135266

image
@Heenawter Heenawter added Feature:Dashboard Dashboard related features Project:Accessibility Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas WCAG A loe:large Large Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:Controls labels Jun 29, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@Heenawter Heenawter mentioned this issue Jun 29, 2022
Closed
@Heenawter Heenawter added the Feature:Input Control Input controls visualization label Jun 29, 2022
@Heenawter Heenawter mentioned this issue Jun 29, 2022
Closed
7 tasks
@Heenawter Heenawter added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. and removed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Feb 16, 2023
@Heenawter
Copy link
Contributor Author

Related: #154297

@Heenawter Heenawter mentioned this issue Apr 5, 2023
Closed
@Heenawter
Copy link
Contributor Author

Heenawter commented Apr 24, 2023 •
edited
Loading

Now that elastic/eui#6704 is merged, once Kibana gets upgraded to EUI v77.2.0+, we should be able to overcome the a11y concerns by replacing our custom version of the range slider popover with the EUI version. We should be able to add custom styles for invalid selections via CSS styles now, which was impossible before because the popover button did not receive information about whether or not the selections were invalid (and hence why we made our own version).

@Heenawter
Copy link
Contributor Author

Now that Kibana has been upgraded to EUI v77.2.0+, our custom range slider no longer renders correctly:

image

By switching to the native EUI component, we will not only be fixing the a11y concerns mentioned here - we will also be improving the control's appearance 👍

@Heenawter Heenawter changed the title (Accessibility) [Controls] Range slider a11y issues (Accessibility) [Controls] Range slider a11y issues May 17, 2023
@Heenawter Heenawter changed the title (Accessibility) [Controls] Range slider a11y issues (Accessibility) [Controls] Range slider a11y + rendering issues May 17, 2023
@Heenawter Heenawter self-assigned this May 25, 2023
This was referenced Jun 5, 2023
Closed
Merged
Closed
Heenawter added a commit that referenced this issue Jun 15, 2023
@Heenawter
[Controls] Range slider a11y and performance improvements (#159271)
9b0f106 
Closes #135466

## Summary

The main goal of this PR is to fix the serious "Buttons must have
discernible text" a11y failure - this is accomplished by switching from
building our own range slider button using `EuiFlexGroup` to instead
using `EuiFormControlLayoutDelimited`, which both resolves these a11y
issues and also fixes a rendering regression:

| Before | After |
|--------|-------|
|
![image](https://github.com/elastic/kibana/assets/8698078/49ea1516-db74-46af-baa5-4ad0a31d5b5a)
|
![image](https://github.com/elastic/kibana/assets/8698078/71bc61f2-f10d-4f8c-8ad2-2681f7faf921)
|

As part of this, I also took some time to clean up some of the range
slider code, which hasn't really been touched in awhile - this
includes...
- moving the debounce on range selections from the embeddable's `input$`
subscription to the component itself, as described
[here](#159271 (comment)).
- fixing a bug where resetting the range slider would unnecessarily
cause unsaved changes, as described
[here](#159271 (comment)).
- improving the `onClick` behaviour (with some notable limitations), as
described
[here](#159271 (comment)).

As a follow up, we need to move the "clear selections" button [to a
hover action](#159395), which
will enable us to then fully move forward with our transition to the
`EuiDualRange` component.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
   > **Note**
> Details provided
[here](#159271 (comment))
on why only partial keyboard support is currently supported
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Heenawter Heenawter

Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Accessibility Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas WCAG A
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Controls] Range slider a11y and performance improvements Heenawter/kibana
2 participants
@Heenawter @elasticmachine