[WIP] EuiSelectable #1699

cchaos · 2019-03-07T20:45:57Z

Summary

The idea behind this new component is to generalize a selectable (and searchable) list of objects that can be used anywhere and will remain consistent across usages.

I've throughly documented it in the component props and the docs pages. So please pull down the branch to walk through it (or read the files).

Essentially it can look like:

This should also fix #1498 by providing the pattern than can be added to any container including a popover.

Checklist

This was checked in mobile
This was checked in IE11
This was checked in dark mode
Any props added have proper autodocs
Documentation examples were added
A changelog entry exists and is marked appropriately
~~[ ] This was checked for breaking changes and labeled appropriately~~
Jest tests were updated or added to match the most common scenarios
This was checked against keyboard-only and screenreader scenarios
~~[ ] This required updates to Framer X components~~

cchaos · 2019-03-07T20:52:37Z

What I really need on this is engineering support to comb through the TS defs and such.

@chandlerprall and/or @thompsongl There is no rush on this, but I'd really like your support on this. You can push directly to the branch and/or make PR's against mine to clean up anything you see.

I think that this might be an ever-evolving component with added features, as has the combo box been, but in essence most of the option object manipulation like putting selected options first, should be done by the consumer. We can possible add helpers for it, but should not be the job of the component. It should be mostly dumb.

I know I still have some design cleanup to do, but would also love some visual feedback from @ryankeairns and/or @snide.

ryankeairns · 2019-03-08T16:39:53Z

This is really nice! I have just one general design question...

Is this intended to be used in a standalone fashion (i.e. in open space) or only within a container such as a popover or flyout? If the former is a potential use case, then additional styling might need to be considered.

For example, the flyout example has the search input contained in separate section and uses the shadow along its bottom edge which provides some nice separation and affordance for scrolling. On the contrary, the examples in the docs feel a bit tight between the search input and the list of items.

With regards to the docs, the page struck me as being rather text-heavy and the use of bold and bulleted text felt uncommon. Perhaps some of that content could be trimmed/moved to Props tab/placed in a callout, etc.

cchaos · 2019-03-11T20:28:36Z

Thanks @ryankeairns

Is this intended to be used in a standalone fashion (i.e. in open space) or only within a container such as a popover or flyout?

Yeah I need to add some optional props for styling when it is the case of it not being in a container.

With regards to the docs, the page struck me as being rather text-heavy and the use of bold and bulleted text felt uncommon. Perhaps some of that content could be trimmed/moved to Props tab/placed in a callout, etc.

Can you give some more concrete examples? Or feel free to point them out directly in the code review.

If you are also specifically talking about that first block where I write out the Option object props, it's because we can't currently import them into the props tab. I have an open issue for it #1688.

thompsongl · 2019-03-12T19:28:04Z

src/components/selectable/selectable.tsx

+
+ clearActiveOption = () => {
+ this.setState({
+ activeOptionIndex: undefined,


Looks like some things in this file are now out of date with how EuiComboBox works (since #1695)

Has there been any fixes to EuiComboBox, that you think are needed in EuiSelectable?

thompsongl · 2019-03-12T19:28:25Z

src/components/selectable/selectable.tsx

+
+ case TAB:
+ // Disallow tabbing when the user is navigating the options.
+ // TODO: Can we force the tab to the next sibling element?


When this.hasActiveOption() is true or false?

So right now there's a weird behavior between tabbing and using the arrow keys to navigate the items.

The following gif starts with tabbing, then when it reaches Iapetus, the arrow keys are used to navigate

My thoughts would be that you tab to focus in and out of the whole list and then only the arrow keys allow for navigating the list.

thompsongl · 2019-03-12T19:28:58Z

src/components/selectable/selectable.tsx

+// @ts-ignore
+import { EuiLoadingChart } from '../loading';
+// @ts-ignore
+import { getMatchingOptions } from '../combo_box/matching_options';


This should probably be pulled out of combo_box and moved to services

I'll look into generalizing that into a service

I couldn't completely generalize it into a global service because it was depending on there being an item.label so I just set the types to be Options and Option[] and put a version of it in the selectable folder. This still means that EuiComboBox has it's own matching_options function, but I did that on purpose again because that conversion will be at a later date.

thompsongl · 2019-03-25T15:14:54Z

Getting back around to looking at this.

General impression is that there needs to be more shared internal components, specifically with EuiComboBox. Just in the time this component has been in the works, there's been divergence due to bugfixes. Were you intendong for that to be part of a later PR or did you see them as entirely separate entities?

cchaos · 2019-03-25T16:45:43Z

In a separate PR, we can try to replace the EuiComboBox parts (and some of the FilterGroup parts), but that might make this too gigantic. I'd rather make sure this component works on it's own first.

cchaos · 2019-03-25T16:47:59Z

We can put a "beta" label on it for now, until we are able to peicemeal it off with EuiComboBox. That way if we need to make breaking changes to it in order to make it work with EuiComboBox, we've set that expectation.

thompsongl · 2019-03-25T18:19:27Z

I like that direction:

Beta badge in the docs for now
Follow-up PR that abstracts list & filter logic to share with EuiComboBox, etc., using EuiComboBox as the source of truth (unless this PR can pull in its upstream bugfix changes)

src-docs/src/views/selectable/selectable_example.js

cchaos · 2019-03-27T21:00:18Z

I added the beta badge to the docs and I think I'm done with all the options. I'm going to start filling out the tests now. So 👍 or specific code review fixes would be appreciated.

chandlerprall · 2019-03-28T14:56:25Z

Hey neat! The I18n token verification works :-D

22:10:53 Token euiComboBoxOptionsList.noAvailableOptions has two differing defaults:
22:10:53 No options available
22:10:53 There aren't any options available

Need to rename the token in selectable.tsx

thompsongl · 2019-03-29T17:48:26Z

@cchaos @chandlerprall PR to remove prop mutations: cchaos#18

snide

Did some accessibility checks with screen readers and it's got some hiccups. I'll put together a PR to see what I can fix.

src/components/selectable/selectable_list/selectable_list.tsx

src/components/selectable/selectable.tsx

src/components/selectable/selectable_search/selectable_search.tsx

chandlerprall

Needs a changelog 😀

LGTM

snide

I've added #1789 to document the issues I noticed with accessibility. I don't want that to hold up this PR (since it's been around awhile with a lot of work in it) since it's a beta PR and things mostly work. I have a branch running that I'll continue with and get a patch going this week.

- Fixed single selection (mostly) - Fixed on Enter of first item - Removed `selectedOptions` where unnecessary - Fixed defaultValue passed to search - Added `append` and `prepend` to Option type - Fixed `…listProps`

and icon type TS issue

This reverts commit ae9e3bd.

cchaos · 2019-04-03T16:56:09Z

Darn it! I'm getting an error in IE11:

Not really sure how to debug, but since it won't even render any of the doc sections I'm wondering if it has to do with the beta badge?

cchaos · 2019-04-03T16:59:11Z

Nope, not from the beta badge... hmmm

cchaos added the work in progress label Mar 7, 2019

elastic deleted a comment from cchaos Mar 12, 2019

thompsongl reviewed Mar 12, 2019

View reviewed changes

chandlerprall reviewed Mar 26, 2019

View reviewed changes

src-docs/src/views/selectable/selectable_example.js Show resolved Hide resolved

cchaos force-pushed the eui-selectable branch from 3c56ee0 to 3494c66 Compare March 27, 2019 20:38

snide reviewed Mar 29, 2019

View reviewed changes

thompsongl mentioned this pull request Apr 1, 2019

Drag and Drop #1733

Merged

9 tasks

chandlerprall suggested changes Apr 2, 2019

View reviewed changes

cchaos force-pushed the eui-selectable branch from 3d3ae08 to 8e941e9 Compare April 2, 2019 18:48

chandlerprall reviewed Apr 2, 2019

View reviewed changes

src/components/selectable/selectable_search/selectable_search.tsx Outdated Show resolved Hide resolved

chandlerprall approved these changes Apr 2, 2019

View reviewed changes

snide approved these changes Apr 3, 2019

View reviewed changes

cchaos added 7 commits April 3, 2019 12:46

Start of EuiAvatarGroup

61b281e

Combobox working with FilterSelectItem

f9ef94a

Initial files of EuiSelectable* and styles for list item

cc344d4

List is searchable, clickable, and navigatable

235d941

On enter works

983298f

Messaging hooked up

f41cdac

Fixed some TS types

68851b5

cchaos and others added 20 commits April 3, 2019 12:46

Fixed some more TS stuff

1f61784

Fixed the rest of the TS errors

f656983

Almost working for the header space menu except for single selection

485622b

Some fixes

d6275a8

- Fixed single selection (mostly) - Fixed on Enter of first item - Removed `selectedOptions` where unnecessary - Fixed defaultValue passed to search - Added `append` and `prepend` to Option type - Fixed `…listProps`

Finished fix to singleSelection

f0734ef

and icon type TS issue

Added lots of docs examples and documentation

2c7b4ae

Add links

1329f47

Revert "Start of EuiAvatarGroup"

b8cd021

This reverts commit ae9e3bd.

Fix calculated height

45c98c7

Fix calculated height x2

df76abb

Added a bordered option

3c9b1d7

Created a general, yet Option specific version of matching_options

d1fde67

Added a universal Beta label for guide pages

def5781

Added basic prop tests and remove async

fa65b92

propagate options changes to consumer; update examples

f73bc75

Chandler feedback round 1

4a1e58b

Chandler feedback round 2

af029bc

Searchable with SearchProps

feb5e92

Fix PropsOf<EuiFieldSearch>

333499a

cl

0272a68

cchaos force-pushed the eui-selectable branch from 1ba22f3 to 0272a68 Compare April 3, 2019 16:48

Fix IE and move to forms section

c5f00fd

cchaos merged commit 26d8686 into elastic:master Apr 3, 2019

cchaos deleted the eui-selectable branch July 8, 2019 20:35

snide added the typescript label Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] EuiSelectable #1699

[WIP] EuiSelectable #1699

cchaos commented Mar 7, 2019 •

edited

Loading

cchaos commented Mar 7, 2019 •

edited

Loading

ryankeairns commented Mar 8, 2019

cchaos commented Mar 11, 2019

thompsongl Mar 12, 2019

cchaos Mar 27, 2019

thompsongl Mar 12, 2019

cchaos Mar 27, 2019

thompsongl Mar 12, 2019

cchaos Mar 27, 2019

cchaos Mar 27, 2019

thompsongl commented Mar 25, 2019

cchaos commented Mar 25, 2019

cchaos commented Mar 25, 2019

thompsongl commented Mar 25, 2019

cchaos commented Mar 27, 2019

chandlerprall commented Mar 28, 2019

thompsongl commented Mar 29, 2019

snide left a comment

chandlerprall left a comment

snide left a comment

cchaos commented Apr 3, 2019

cchaos commented Apr 3, 2019

[WIP] EuiSelectable #1699

[WIP] EuiSelectable #1699

Conversation

cchaos commented Mar 7, 2019 • edited Loading

Summary

Checklist

cchaos commented Mar 7, 2019 • edited Loading

ryankeairns commented Mar 8, 2019

cchaos commented Mar 11, 2019

thompsongl Mar 12, 2019

Choose a reason for hiding this comment

cchaos Mar 27, 2019

Choose a reason for hiding this comment

thompsongl Mar 12, 2019

Choose a reason for hiding this comment

cchaos Mar 27, 2019

Choose a reason for hiding this comment

thompsongl Mar 12, 2019

Choose a reason for hiding this comment

cchaos Mar 27, 2019

Choose a reason for hiding this comment

cchaos Mar 27, 2019

Choose a reason for hiding this comment

thompsongl commented Mar 25, 2019

cchaos commented Mar 25, 2019

cchaos commented Mar 25, 2019

thompsongl commented Mar 25, 2019

cchaos commented Mar 27, 2019

chandlerprall commented Mar 28, 2019

thompsongl commented Mar 29, 2019

snide left a comment

Choose a reason for hiding this comment

chandlerprall left a comment

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

cchaos commented Apr 3, 2019

cchaos commented Apr 3, 2019

cchaos commented Mar 7, 2019 •

edited

Loading

cchaos commented Mar 7, 2019 •

edited

Loading