Single select

[kmcfaul]

What: Added select wrapper and single select variant along with tests and TS declarations. TS was tested locally.

Tried to structure Select for future use with other variants. Select control/state is outside of the component for now, matching Dropdown, but we may want to move that into Select itself and instead issue a callback when the selected items are updated.

Keyboard navigation is similar to Dropdown.

Refers to issue: #1214

[patternfly-build]

PatternFly-React preview: https://1277-pr-patternfly-react-patternfly.surge.sh

[coveralls]

Pull Request Test Coverage Report for Build 4594

• 10 of 23 (43.48%) changed or added relevant lines in 2 files are covered.
• 31 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+1.0%) to 79.834%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/patternfly-4/react-core/src/components/Select/SelectOption.js	7	20	35.0%

Files with Coverage Reduction	New Missed Lines	%
packages/patternfly-4/react-core/src/components/Dropdown/Separator.js	1	80.0%
packages/patternfly-4/react-core/src/components/Dropdown/DropdownItem.js	4	34.52%
packages/patternfly-4/react-core/src/helpers/util.ts	26	14.29%

Totals
Change from base Build 4568:	1.0%
Covered Lines:	3379
Relevant Lines:	3949

---

💛 - Coveralls

[mcarrano]

@kmcfaul The behavior of this looks good with one exception. Is there any way to fix the width of the control (or make that an option)? I wouldn't necessarily want this to resize whenever I select a new value, especially if it's used in a layout with other elements following it.

[kmcfaul]

@mcarrano @tlabaj @jschuler
I agree about the behavior that width should be controllable, and I can add an optional width prop that controls a style attribute on the container div, but I'm not sure if it would be better to just let the user pass the style object into the component instead. Anyone have any thoughts about which method would be better?

[mcoker]

I would probably expect the default width of the select to match the widest item in the select menu. I guess that's a little weird when the initial value (ie, "please choose one") isn't in the menu. In that case, I would probably still expect the menu width to match the width of "please choose one", and not change when you select something else in the menu.

I could also see allowing the menu to be as wide as the parent container, so it's basically 100% width, and that's constrained by the container it's in. And additionally, in this state, you could use a width prop and specify a width and overwrite the 100% width.

So maybe 2 modes. 1) The width matches the width of the widest thing in it, so that it doesn't change when you select things, and 2) it's 100% width and fills the width of it's parent element, and you could use this with a width prop to overwrite the width with something specific passed by the user.

[mcarrano]

This sounds good @mcoker . I think it's important to be able to constrain the width independent of the content. For example, we might use the select to select a server name where the names could be anything and possibly long. Or worse the available values might change based on something else in the page. In this case I would not want the width of the Select to shrink or grow dependent on the values that are loaded.

On a related note, can I assume that long values will truncate (i.e. if they do not fit)?

[mcoker]

@mcarrano

On a related note, can I assume that long values will truncate (i.e. if they do not fit)?

Currently the text in the toggle will truncate, but the text in the dropdown options will not. The menu will just grow to match the width of the text inside of it.

Looking at Carbon and Bootstrap, Carbon always displays the dropdown menu as wide as the toggle (the entire dropdown is 100% width by default, so it consumes all of the available space it's in), then wraps the text if it exceeds the dropdown menu width.

Bootstrap behaves the same way we do.

[mcarrano]

In that case @mcoker I like it the way we have it. So long object names can be displayed at full length in the menu without wrapping.

[kmcfaul]

So maybe 2 modes. 1) The width matches the width of the widest thing in it, so that it doesn't change when you select things, and 2) it's 100% width and fills the width of it's parent element, and you could use this with a width prop to overwrite the width with something specific passed by the user.

I'm having trouble finding a good way to implement case 1 so far, but case 2 is very straightforward. I'm curious if having the width default to 100% would be the best solution overall?

Currently the text in the toggle will truncate

@mcoker The current build doesn't seem to truncate, if you define a long placeholder text the toggle div expands vertically. Am I missing a css class?

[mcoker]

We would likely need to add width: 100% to it in core if we're all on board with that change.

The current build doesn't seem to truncate, if you define a long placeholder text the toggle div expands vertically. Am I missing a css class?

Yep, looks like you're missing .pf-c-select__toggle-text in .pf-c-select__toggle-wrapper

[christiemolloy]

I agree with what @mcoker said above. Also the button that says "Clear" next to the select is confusing. IMO it would be better to have the select component handle the clearing of a selected option. Because in this example it looks as though the button is required to clear the selected option. In Boostrap their Select has a first option called: "Choose..." which returns the select back to default. And in Material-UI their Select has a first option that says "None". The button you chose is used as one of the primary buttons on the page and in this case its more of a clear button. @mcoker @mcarrano what do you think?

[mcoker]

Agree re: the clear button. I think the default "none" option or whatever the text is in the toggle is also represented as an option in the menu to de-select would work. I also wonder if we could use the fa-times-circle button to clear the select, similar to what we have in the typeahead examples. Another example is in this react select component.

[redallen]

Your Travis build is fine, just merge master to get the latest .travis.yml changes to ignore the coveralls error.

[tlabaj]

@kmcfaul Looking good. I still don't understand why you are providing an interface for the SelectToggle. It is an internal component.

[kmcfaul]

Oh yeah, when I was first implementing the component I planned to follow Dropdown in that the user defines the toggle, but I found it wasn't really necessary and changed it to an internal component. I can remove that interface.

[tlabaj]

LGTM

[christiemolloy]

Great job on this one!!!

Fixed