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

[EuiSuggest] Rebuild using EuiSelectable #3733

Closed
myasonik opened this issue Jul 13, 2020 · 4 comments · Fixed by #5157
Closed

[EuiSuggest] Rebuild using EuiSelectable #3733

myasonik opened this issue Jul 13, 2020 · 4 comments · Fixed by #5157
Labels
accessibility - WCAG A Level A WCAG Accessibility Criteria accessibility

Comments

@myasonik
Copy link
Contributor

The existing EuiSuggest has quite a few a11y shortcomings (see #2404 for the original remediation plan).

Instead of effectively rewriting it from scratch, it should use EuiSelectable to inherit all the a11y work that went into it and to consolidate implementations of similar components.

On top of EuiSelectable, EuiSuggest largely provides:

  1. An opinionated rendering of individual items
  2. Some more states of the search field

I think the EUI team prefers to keep the EuiSuggest component and to wrap EuiSelectable within it but another implementation option is to add a "renderAs" prop (or something along those lines) to EuiSelectable to adjust styling.

@myasonik myasonik added accessibility accessibility - WCAG A Level A WCAG Accessibility Criteria labels Jul 13, 2020
@cchaos cchaos changed the title Rebuild [EuiSuggest] with EuiSelectable [EuiSuggest] Rebuild using EuiSelectable Sep 20, 2020
@github-actions
Copy link

👋 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.

@thompsongl
Copy link
Contributor

I've started exploring the conversion to EuiSelectable and the path looks straightforward, but I have questions about the purpose of EuiSuggest. Mainly, it is currently a fully controlled component (i.e., it does not handle filtering the list or managing selected items, but relies on a parent controller to do so). Is this a paradigm we want to continue? Are we converting to EuiSelectable strictly for the a11y benefits? Or is part of using EuiSelectable is to enable its search, filter, and selection features?

cc// @cchaos

@cchaos
Copy link
Contributor

cchaos commented Sep 2, 2021

This one is tricky and I'm honestly not sure if this component is even used anywhere (yet). But my guess is that we'd definitely have to support a mechanism of custom search queries and not just plain text filtering. what we'd probably do is just use the selectable list for rendering the items and keep the search input separate.

@thompsongl
Copy link
Contributor

what we'd probably do is just use the selectable list for rendering the items and keep the search input separate.

That's my read as well. I'll probably put up a PR with this type of conversion and we can discuss more there. At the very least it's a step in the right direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility - WCAG A Level A WCAG Accessibility Criteria accessibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants