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

EuiSuperSelect #921

Merged
merged 37 commits into from
Aug 16, 2018
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
0c6aa97
Revert "Added logo for EMS (Elastic Map Service)"
Jun 7, 2018
fde6cb0
Revert "Revert "Added logo for EMS (Elastic Map Service)""
Jun 11, 2018
4130ef0
Basic `EuiSuperSelect` created
Jun 11, 2018
26e1bf4
Increasing shadow of combobox dropdown
Jun 12, 2018
8d95237
Moving super_select to super_select_control and
Jun 12, 2018
a6e1737
Moved mapping option to inside super select
Jun 12, 2018
cd66733
Splitting up examples
Jun 12, 2018
0297577
start on keyboard nav
chandlerprall Jul 18, 2018
8007fab
keyboard navigation for EuiSuperSelect
chandlerprall Jul 20, 2018
80ae4c5
fix incorrect 'this' access
chandlerprall Jul 20, 2018
22581e5
wait for the option to be visible before focusing
chandlerprall Jul 25, 2018
c966c74
Fix sass var name
Jul 25, 2018
548e529
remove arrow
Jul 25, 2018
b254617
Set EuiSuperSelect's menu width to match the select control's
chandlerprall Jul 25, 2018
70163a3
Allowing `EuiPopover` arrow to be optional
Jul 30, 2018
87a22a3
Don't reset the known superselect width on close
chandlerprall Aug 3, 2018
85dfc4d
One last fix to popover shadow
Aug 3, 2018
d7bd5b0
Fixed EuiPopover BEM naming
Aug 6, 2018
8eb1e8b
Addressing PR comments
Aug 6, 2018
5596d70
kinda working
snide Aug 8, 2018
36feea2
regex the input display and read as text
snide Aug 9, 2018
95e87e0
update tests
snide Aug 9, 2018
e8c3fa0
Allowing `label` to be passed from `EuiFormGroup`
Aug 10, 2018
3d98cbc
Allowing more props to be passed to each option
Aug 10, 2018
1dd88f4
Revert "Allowing `label` to be passed from `EuiFormGroup`"
Aug 10, 2018
a001055
Using mulitple id’s in `aria-labelledby` instead
Aug 10, 2018
2a7f5da
use switch over if...else
chandlerprall Aug 6, 2018
70d08c3
Convert the hidden <select> into an input[type=hidden]
chandlerprall Aug 8, 2018
5e86527
Add example of disabled and data-test-subj attributes to a super sele…
chandlerprall Aug 14, 2018
249fa1a
Merge pull request #12 from chandlerprall/274-super-select-pr-feedback
cchaos Aug 14, 2018
648f2a0
Update EuiSuperSelect unit tests to open the options before snapshot
chandlerprall Aug 14, 2018
9884c1e
Merge pull request #13 from chandlerprall/274-super-select-unit-tests
cchaos Aug 14, 2018
ab9bd72
Merge branch 'master' into 274-super-select
cchaos Aug 16, 2018
a5b1d33
Remove obsolete test & changelog
Aug 16, 2018
d927eef
Revert "Remove obsolete test & changelog"
Aug 16, 2018
724486a
Revert "Revert "Remove obsolete test & changelog""
Aug 16, 2018
f0e4f39
Wrong test suite update
Aug 16, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src-docs/src/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ import { XYChartLineExample }
import { Changelog }
from './views/package/changelog';

import { SuperSelectExample }
from './views/super_select/super_select_example';

/**
* Lowercases input and replaces spaces with hyphens:
* e.g. 'GridView Example' -> 'gridview-example'
Expand Down Expand Up @@ -353,6 +356,7 @@ const navigation = [{
FormLayoutsExample,
FormControlsExample,
FormValidationExample,
SuperSelectExample,
ComboBoxExample,
ColorPickerExample,
CodeEditorExample,
Expand Down
8 changes: 8 additions & 0 deletions src-docs/src/views/form_controls/form_controls_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ export const FormControlsExample = {
type: GuideSectionTypes.HTML,
code: selectHtml,
}],
text: (
<p>
This component renders a basic HTML <code>&lt;select&gt;</code> element. If you need more customization
for how the options and/or selected values render, use the <EuiLink href="/#/forms/superselect">EuiSuperSelect</EuiLink>.
Another option is to use the <EuiLink href="/#/forms/combo-box">EuiComboBox</EuiLink>, which has search and multi-select
capabilities, but also has restrictions on how items are rendered.
</p>
),
props: {
EuiSelect,
},
Expand Down
91 changes: 91 additions & 0 deletions src-docs/src/views/super_select/super_select.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import React, {
Component,
Fragment,
} from 'react';

import {
EuiSuperSelect,
EuiSpacer,
} from '../../../../src/components';

export default class extends Component {
constructor(props) {
super(props);

this.options = [
{
value: 'option_one',
inputDisplay: 'Option one',
disabled: true,
'data-test-subj': 'option one',
},
{
value: 'option_two',
inputDisplay: 'Option two',
},
{
value: 'option_three',
inputDisplay: 'Option three has a super long text to see if it will truncate or what',
},
];

this.state = {
value: this.options[1].value,
};
}

onChange = (value) => {
this.setState({
value: value,
});
};

render() {
return (
<Fragment>
<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
/>

<EuiSpacer size="m" />

<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
disabled
/>

<EuiSpacer size="m" />

<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
isLoading
/>

<EuiSpacer size="m" />

<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
isLoading
disabled
/>

<EuiSpacer size="m" />

<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
compressed
/>
</Fragment>
);
}
}
65 changes: 65 additions & 0 deletions src-docs/src/views/super_select/super_select_basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React, {
Component,
} from 'react';

import {
EuiSuperSelect,
EuiHealth,
} from '../../../../src/components';

export default class extends Component {
constructor(props) {
super(props);

this.options = [
Copy link
Contributor

@cjcenizal cjcenizal Aug 3, 2018

Choose a reason for hiding this comment

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

Can we add an example of disabled options and the application of pass-through props?

    this.options = [
      {
        value: 'option_one',
        inputDisplay: 'Option one',
        disabled: true,
        'data-test-subj': 'option one',
      },
      {
        value: 'option_two',
        inputDisplay: 'Option two',
      },
      {
        value: 'option_three',
        inputDisplay: 'Option three has a super long text to see if it will truncate or what',
      },
    ];

It looks like unexpected props aren't passed through.

{
value: 'warning',
inputDisplay: (
<EuiHealth color="subdued" style={{ lineHeight: 'inherit' }}>
Warning
</EuiHealth>
),
'data-test-subj': 'option-warning',
disabled: true,
},
{
value: 'minor',
inputDisplay: (
<EuiHealth color="warning" style={{ lineHeight: 'inherit' }}>
Minor
</EuiHealth>
),
'data-test-subj': 'option-minor',
},
{
value: 'critical',
inputDisplay: (
<EuiHealth color="danger" style={{ lineHeight: 'inherit' }}>
Critical
</EuiHealth>
),
'data-test-subj': 'option-critical',
},
];

this.state = {
value: this.options[1].value,
};
}

onChange = (value) => {
this.setState({
value: value,
});
};

render() {
return (
<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
/>
);
}
}
82 changes: 82 additions & 0 deletions src-docs/src/views/super_select/super_select_complex.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import React, {
Component,
Fragment,
} from 'react';

import {
EuiSuperSelect,
EuiSpacer,
EuiText,
} from '../../../../src/components';

export default class extends Component {
constructor(props) {
super(props);

this.options = [
{
value: 'option_one',
inputDisplay: 'Option one',
dropdownDisplay: (
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of putting inputDisplay and dropdownDisplay on each option, I think it might make more sense to treat these options as data-only, without any responsibility for rendering. Then we could provide props to the EuiSuperSelect instance itself to render the input and the dropdown. This will allow these options to be defined by an API response without requiring us to then decorate each option. This is similar to how EuiComboBox allows you customize the way each option is rendered (https://github.com/elastic/eui/blob/master/src-docs/src/views/combo_box/render_option.js#L117).

I'm thinking along these lines:

const healthToColorMap = {
  0: 'warning',
  1: 'subdued',
  2: 'danger',
};

const healthToNameMap = {
  0: 'Warning',
  1: 'Minor',
  2: 'Critical',
};

export default class extends Component {
  constructor(props) {
    super(props);

    this.options = [
      {
        value: 'option_one',
        // You can put anything you want inside of data.
        data: {
          name: 'Option one',
          description: 'Has a short description giving more detail to the option.',
          health: 0,
        },
      },
      {
        value: 'option_two',
        data: {
          name: 'Option two',
          description: 'Has a short description giving more detail to the option.',
          health: 1,
        },
      },
      {
        value: 'option_three',
        data: {
          name: 'Option three',
          description: 'Has a short description giving more detail to the option.',
          health: 2,
        },
      },
    ];

    this.state = {
      value: this.options[1].value,
    };
  }

  onChange = (value) => {
    this.setState({
      value: value,
    });
  };

  renderOption = option => {
    const { name, description, health } = option.data;

    return (
      <Fragment>
        <strong>{name}</strong>

        <EuiHealth color={healthToColorMap[health]} style={{ lineHeight: 'inherit' }}>
          healthToNameMap[health]
        </EuiHealth>

        <EuiSpacer size="xs" />
        <EuiText size="s" color="subdued">
          <p className="euiTextColor--subdued">{description}</p>
        </EuiText>
      </Fragment>
    );
  };

  renderSelectedOption = option => {
    return option.data.name;
  };

  render() {
    return (
      <Fragment>
        <EuiSuperSelect
          options={this.options}
          valueOfSelected={this.state.value}
          onChange={this.onChange}
          itemLayoutAlign="top"
          renderOption={this.renderOption}
          renderSelectedOption={this.renderSelectedOption}
          hasDividers
          aria-label="Use aria labels when no actual label is in use"
        />
      </Fragment>
    );
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's valuable to have render-purposeful show this in the select box and show this in the drop down values. However, I would agree that also allowing a data param to help track the represented data would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My worry with this option, is that each option is no longer customizable, unless you add a bunch of if statements in the render methods. You could still do a similar thing with the current mapping by using the render methods as calls within the options declaration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the flexibility is nice in the way its set up now. I see us switching things out more from item to item with this one than the combobox.

Copy link
Contributor

Choose a reason for hiding this comment

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

My worry with this option, is that each option is no longer customizable

Do we have an example use case where each option is completely different? Do you think this will be a common requirement? I think we should optimize for the common use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it is right now, no. But it can be altered down the road to support this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @cchaos though I was more thinking from the technical perspective, e.g. is there a challenge with decorating hundreds (or thousands) of options with inputDisplay and dropdownDisplay properties, would it play well with react-virtualized, etc?

Copy link
Contributor

@snide snide Aug 14, 2018

Choose a reason for hiding this comment

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

@cjcenizal That screen looks like EuiFilterGroup, which has a current implementation within our table / search components (which you can see on those pages respectively). It even adds search once it gets beyond 10 items. Will that not cover what you need? I don't know about it's scaling abilities, but it's a 1-1 design against the screen you showed, so my guess is that's the component we'd update further.

By design EuiSuperSelect should not have search in it and is really just a stylized select of choices. We have combo box and the filter group stuff for the pattern you describe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Example. I believe it also allows for multi-item selection, not just one item.

Copy link
Contributor

Choose a reason for hiding this comment

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

@snide Good call! Yes that could work, but it needs to be broken out into its own component. Looks like the table example you shared has it baked into its interface via the search prop, and the standalone EuiFilterGroup example is just using a hand-rolled popover.

If we were to break it out into its own component, then at first glance it looks like the only difference between it and EuiSuperSelect would be that one has search and the other doesn't.

<Fragment>
<strong>Option one</strong>
<EuiSpacer size="xs" />
<EuiText size="s" color="subdued">
<p className="euiTextColor--subdued">Has a short description giving more detail to the option.</p>
</EuiText>
</Fragment>
),
},
{
value: 'option_two',
inputDisplay: 'Option two',
dropdownDisplay: (
<Fragment>
<strong>Option two</strong>
<EuiSpacer size="xs" />
<EuiText size="s" color="subdued">
<p className="euiTextColor--subdued">Has a short description giving more detail to the option.</p>
</EuiText>
</Fragment>
),
},
{
value: 'option_three',
inputDisplay: 'Option three',
dropdownDisplay: (
<Fragment>
<strong>Option three</strong>
<EuiSpacer size="xs" />
<EuiText size="s" color="subdued">
<p className="euiTextColor--subdued">Has a short description giving more detail to the option.</p>
</EuiText>
</Fragment>
),
},
];

this.state = {
value: this.options[1].value,
inputDisplay: this.options[1].inputDisplay,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this isn't being consumed anywhere, so I think we can remove it?

};
}

onChange = (value) => {
this.setState({
value: value,
inputDisplay: this.options.inputDisplay,
Copy link
Contributor

Choose a reason for hiding this comment

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

Then we can simplify this to:

this.setState({ value });

});
};

render() {
return (
<EuiSuperSelect
options={this.options}
valueOfSelected={this.state.value}
onChange={this.onChange}
itemLayoutAlign="top"
hasDividers
/>
);
}
}
95 changes: 95 additions & 0 deletions src-docs/src/views/super_select/super_select_example.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import React from 'react';

import { renderToHtml } from '../../services';

import {
GuideSectionTypes,
} from '../../components';

import {
EuiCode,
EuiSuperSelect,
} from '../../../../src/components';

import SuperSelect from './super_select';
const superSelectSource = require('!!raw-loader!./super_select');
const superSelectHtml = renderToHtml(SuperSelect);

import SuperSelectBasic from './super_select_basic';
const superSelectBasicSource = require('!!raw-loader!./super_select_basic');
const superSelectBasicHtml = renderToHtml(SuperSelectBasic);

import SuperSelectComplex from './super_select_complex';
const superSelectComplexSource = require('!!raw-loader!./super_select_complex');
const superSelectComplexHtml = renderToHtml(SuperSelectComplex);

export const SuperSelectExample = {
title: 'SuperSelect',
sections: [{
source: [{
type: GuideSectionTypes.JS,
code: superSelectBasicSource,
}, {
type: GuideSectionTypes.HTML,
code: superSelectBasicHtml,
}],
text: (
<div>
<p>
This is a simple replacement component for <EuiCode>EuiSelect</EuiCode> if you
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice if over on the EuiSelectdocs we back to this EuiSuperSelect page as well. Guessing a lot of people would find it from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

need more customization in either the display of the input or option. Simply pass
an array of option objects:
</p>
<ul>
<li><EuiCode>value</EuiCode>: for storing unique value of item, </li>
<li><EuiCode>inputDisplay</EuiCode>: what shows inside the form input when selected, </li>
<li><EuiCode>dropdownDisplay</EuiCode>: (optional) what shows for the item in the dropdown</li>
</ul>
<p>
&hellip; and the component will create a select styled button
that triggers a popover of selectable items.
</p>
</div>
),
props: { EuiSuperSelect },
demo: <SuperSelectBasic />,
},
{
title: 'More complex',
source: [{
type: GuideSectionTypes.JS,
code: superSelectComplexSource,
}, {
type: GuideSectionTypes.HTML,
code: superSelectComplexHtml,
}],
text: (
<p>
Both <EuiCode>inputDisplay</EuiCode> and <EuiCode>dropdownDisplay</EuiCode> accept
React nodes. Therefore you can pass some descriptions with each option to show
in the dropdown. If your options will most likely be multi-line, add
the <EuiCode>hasDividers</EuiCode> prop to show borders between options.
</p>
),
props: { },
demo: <SuperSelectComplex />,
},
{
title: 'States',
source: [{
type: GuideSectionTypes.JS,
code: superSelectSource,
}, {
type: GuideSectionTypes.HTML,
code: superSelectHtml,
}],
text: (
<p>
You can pass the same props as you normally would to <EuiCode>EuiSelect</EuiCode> like
disabled, isLoading, compressed, etc&hellip;
</p>
),
props: { EuiSuperSelect },
demo: <SuperSelect />,
}],
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ exports[`CollapsedItemActions render 1`] = `
/>
}
closePopover={[Function]}
hasArrow={true}
id="id-actions"
isOpen={false}
ownFocus={false}
Expand All @@ -29,12 +30,14 @@ exports[`CollapsedItemActions render 1`] = `
<EuiContextMenuItem
disabled={false}
icon={undefined}
layoutAlign="center"
onClick={[Function]}
toolTipPosition="right"
>
default1
</EuiContextMenuItem>,
<EuiContextMenuItem
layoutAlign="center"
onClick={[Function]}
toolTipPosition="right"
/>,
Expand Down
Loading