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

[DO NOT MERGE] Selection Components FAQ #6999

Closed
wants to merge 7 commits into from

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Jul 26, 2023

Related to #6880

❗ I'm using a draft PR for team review, do not merge this PR!

This is a draft of The Ultimate Guide to EUI Selection Components, an FAQ surrounding the different types of selection components EUI offers and what they do. I would love feedback on the copy and feel free to make notes of info that should be included or removed. I will be adding images of each component to the actual discussion prior to posting.

QA:

  • Read the FAQ as it would be displayed here. _psst! Did you like the meme? 🕸️ _
  • Add suggestions and comments inline or in this Google doc

@breehall breehall requested a review from a team July 26, 2023 19:56
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6999/

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ This was such a fun PR to review @breehall

Let me know if you have any questions or want to discuss my suggestions.

wiki/temp/selection-components.md Outdated Show resolved Hide resolved
wiki/temp/selection-components.md Outdated Show resolved Hide resolved
wiki/temp/selection-components.md Outdated Show resolved Hide resolved
wiki/temp/selection-components.md Outdated Show resolved Hide resolved
Comment on lines 35 to 49
| | `EuiSelect` | `EuiSuperSelect` | `EuiSelectable` | `EuiComboBox` | `EuiSuggest` |
|---|:---:|:---:|:---:|:---:|:---:|
| Select a single option | ✅ | ✅ | ✅ | ✅ | ✅ |
| Select multiple options | ❌ | ❌ | ✅ | ✅ | ❌ |
| Accepts custom values from users | ❌ | ❌ | ❌ | ✅ | ❌ |
| Option exclusion | ❌ | ❌ | ✅ | ❌ | ❌ |
| Customizable option display | ❌ | ✅ | ❌ | ✅ | ❌
| Controllable loading state | ✅ | ✅ | ✅ | ✅ | ✅ |
| Customizable loading state | ✅ | ✅ | ❌ | ✅ | ✅ |
| Customizable error handling | ❌ | ❌ | ✅ | ❌ | ❌
| Searchable | ❌ | ❌ | ✅ | ✅ | ❌ |
| Accepts custom values from users | ❌ | ❌ | ❌ | ✅ | ❌ |
| `disabled` state | ✅ <br/> (Disables whole component) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables whole component) | ❌ |
| `readOnly` state | ❌ | ✅ | ❌ | ❌ | ❌ |
| Built in utility function for clearing user input | ❌ | ❌ | ✅ | ✅ | ✅
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the page through VoiceOver to see how it handled the icons. They read back as "Checkmark" and "Exe" which offer pretty good semantic clues this component does or does not offer this feature. I tried out adding "Yes" and "No" text to a few cells. It did add clarity for screen reader use, so I'll float it as an enhancement idea. Would look roughly like this sketch in code:

Screen Shot 2023-07-27 at 8 42 14 AM


EUI offers different selection solutions and choosing the right one may seem tricky. Welcome to a comprehensive guide on the EUI selection components. This guide will include explanations and comparisons for each component.

![Spiderman Selection components meme](spiderman_selection.jpeg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this meme! It's such a fun way to communicate the similarities and differences in our components. Because it's such an important part of this page's context, could we expand on the alt text? Maybe a short description of multiple Spidermen pointing at each other, and an explanation of how that relates to our selection components.

Five Spidermen are in a room, pointing at one another, as if to say "It's you! No, it's you!" Each Spiderman has the name of an EUI selection component next to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first time I read this, my eyes saw "five Spidermen walk into a bar". The beginnings of a corny joke 😆

`EuiComboBox` allows multiple items to be selected and shown all at once. This should be used when the user has a number of options to choose from and needs to be able to search for them. `EuiComboBox` also allows users to specify a custom value in addition to selecting from a predetermined list.

### [`EuiSelectable`](https://elastic.github.io/eui/#/forms/selectable)
`EuiSelectable` renders a searchable list where multiple options can be selected or excluded. It should not be used for primary navigation, but can be used to simplify the construction of popover navigational menus. `EuiSelectable` does not appear in a popover or show all selected items in a list.
Copy link
Member

@cee-chen cee-chen Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EuiSelectable does not appear in a popover

This is not correct, EuiSelectable can and does appear in EuiPopovers frequently, and in fact its max-height and scrolling behavior lends itself really well to popover:

EDIT: Ah, wait, I think I see. Did you mean EuiSelectable does not appear by itself in a popover?

does not [...] show all selected items in a list

I worry this sentence may mislead consumers into thinking that "not all seleteced items are being shown" (the 'in list' part feels really vague). If we're contrasting that against EuiComboBox, then no, we don't render a pill display, but we should state very clearly that selection state is displayed by default next to the list item vs. anywhere else.

I honestly also think it's worth stressing that EuiSelectable is also by far our most accessible component in terms of screen reader support. While standalone consumer usage may not be terribly frequent, we can and do dogfood it across EUI under the hood because of how flexible its rendering can be. EuiSuggest is using EuiSelectable, as is EuiSearchBar. EuiComboBox will (eventually) dogfood EuiSelectable as well.

It's by far one of the more flexible selection components - for example, it allows choosing between search vs no search - but it requires some amount of work to be put in dev-wise to get there compared to the other components.

Copy link
Member

@cee-chen cee-chen Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be used for primary navigation, but can be used to simplify the construction of popover navigational menus.

Can I ask where you got this information or copy from? None of the selection components should be used for navigation, so I'm confused as to why we're even mentioning that. We should be pointing people towards EuiContextMenu for popover navigational menus.

edit: ah wait, I think I see in the context of EuiSelectableTemplateSitewide. I still don't think it makes sense to mention here though.

edit to edit: OK I see that's coming from our docs for EuiSelectable which is kinda garbage and needs to be rewritten 🥲

| Controllable loading state | ✅ | ✅ | ✅ | ✅ | ✅ |
| Customizable loading state | ✅ | ✅ | ❌ | ✅ | ✅ |
| Customizable error handling | ❌ | ❌ | ✅ | ❌ | ❌
| Searchable | ❌ | ❌ | ✅ | ✅ | ❌ |
Copy link
Member

@cee-chen cee-chen Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect - EuiSuggest dogfoods EuiSelectable and is 100% searchable.

Also, I don't feel EuiSelect deserves a solid red X - while it isn't technically searchable, it still follows native select rules, which means if you start to type in the option you want, the browser will automatically try and jump you to the option that matches what you're typing. (which is why I love native selects and get real annoyed at websites that use custom selects that don't have this behavior, especially for date/years of birth 😄)

| Customizable error handling | ❌ | ❌ | ✅ | ❌ | ❌
| Searchable | ❌ | ❌ | ✅ | ✅ | ❌ |
| Accepts custom values from users | ❌ | ❌ | ❌ | ✅ | ❌ |
| `disabled` state | ✅ <br/> (Disables whole component) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables whole component) | ❌ |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this nuance describing disabled state, nice work!

lmfao @ EuiSuggest not supporting disabled state though, that seems like such an oversight 💀 we seriously need to get rid of this component

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha so I just searched and there's 0 usages of EuiSuggest in Kibana. Zero! My desire to remove it just shot up tenfold

| Accepts custom values from users | ❌ | ❌ | ❌ | ✅ | ❌ |
| `disabled` state | ✅ <br/> (Disables whole component) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables individual option) | ✅ <br/> (Disables whole component) | ❌ |
| `readOnly` state | ❌ | ✅ | ❌ | ❌ | ❌ |
| Built in utility function for clearing user input | ❌ | ❌ | ✅ | ✅ | ✅
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built in utility function for clearing user input

The table documentation for this is not quite correct or consistent:

  • EuiSelectable allows clearing the current search value but not clearing all selections
  • EuiComboBox actually allows clearing all selections
  • EuiSuggest is also only clearing the search value (it has no concept of internal selection state and thus cannot clear selections)

You could separate this into "allows clearing search" vs "allows clearing selection", but I'm not sure how useful that would be. "Built in utility for clearing all selections" would probably be the most useful feature people want, and that would only go to EuiComboBox (although I know people have requested it for EuiSelectable).

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6999/

@breehall
Copy link
Contributor Author

breehall commented Aug 2, 2023

Editing copy in Github is...not fun and I'm sorry for subjecting y'all to it 😆
I found a fun Markdown extension for Google Drive, so here's a Google docs link for the remainder of the editing process!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6999/

@breehall breehall requested a review from a team August 4, 2023 18:20
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6999/

@breehall breehall marked this pull request as ready for review August 7, 2023 13:44
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6999/

@breehall
Copy link
Contributor Author

breehall commented Aug 7, 2023

Thank you all for your input. Closing this PR. This discussion has been posted - you can find it here.

@breehall breehall closed this Aug 7, 2023
@breehall breehall deleted the selection-components branch October 6, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants