cerner / terra-core

Terra offers a set of configurable React components designed to help build scalable and modular application UIs. This UI library was created to solve real-world issues in projects we work on day to day.
http://terra-ui.com
Apache License 2.0
183 stars 165 forks source link

[terra-form-select] - Support multiple selections #1362

Closed StephenEsser closed 6 years ago

StephenEsser commented 6 years ago

Issue Description

This is the general direction of the upcoming enhancements to the Select component. This tech-design will be updated as discussions evolve.

Third-Party Select Components for Reference / Research

Ant-Design

https://ant.design/components/select/

Vue

https://vue-multiselect.js.org/#sub-getting-started

Material-UI

http://www.material-ui.com/#/components/select-field

React-Select

https://jedwatson.github.io/react-select/

Select

Existing Select Props | Prop Name | Type | Is Required | Default Value | Description | | --------- |-------| -------------| ------------- | ----------- | | boundingRef | func | optional | none | Bounding container for the select menu, will use window if no value provided. | | children | node | required | none | Select options. | | defaultValue | string or number or array | optional | none | The value to start the select on. Use this to create an uncontrolled input. | | disabled | bool | optional | false | Indicates if the select should be disabled. | | isInvalid | bool | optional | false | Indicates if the select is in an invalid state. | | isPlaceholderHidden | bool | optional | false | Indicates if the placeholder is hidden | | name | string | optional | none | Name of the select field. | | onChange | func | optional | none | Function to trigger when the user changes the select value. Called with the parameters: 1) Click event triggering the change 2) New selected value 3) Name of the select component | | required | bool | optional | false | Whether the select is required or not. | | value | string or number or array | optional | none | The value of the select element. Use this to create a controlled input. | | releaseFocus | func | optional | none | A callback function to let the containing component (e.g. modal) to regain focus. | | requestFocus | func | optional | none | A callback function to request focus from the containing component (e.g. modal). |

New Select Props

Prop Name Type Is Required Default Value Description
dropdownAttrs object optional none Custom attributes to spread onto the dropdown. ( A common need is setting a custom max height of a drop down )
noResultContent node optional none Content to display when no search results are found.
onDeselect func optional none Callback function triggered when an option is deselected. function(value, option)
onSelect func optional none Callback function triggered when an option is selected. function(value, option)
onSearch func optional none Callback function triggered when the search criteria changes. function(value)
optionFilter func optional none Callback function for option filtering. function(searchValue, option) When provided, this function will be called once for every option in the list. If the function returns true the option will remain in the filtered list. False removes it.
placeholder string optional Select Hint text placeholder visible when no value is selected
variant string optional 'default' Select behavior. One of default, combobox, tag, multiple

Removed Select Props

Prop Name Type Is Required Default Value Description
boundingRef func optional none Bounding container for the select menu, will use window if no value provided.
isPlaceholderHidden bool optional false Indicates if the placeholder is hidden
releaseFocus func optional none A callback function to let the containing component (e.g. modal) to regain focus.
requestFocus func optional none A callback function to request focus from the containing component (e.g. modal).

Changed Select Props

onChange(event, value, name) -> onChange(value)

Variants

Default

Default select behavior. Allows only a single selection.

Combobox

The select will act as a standard input with a drop down menu that provides an optional set of selections to choose from. The options will filter as a user types into the input. Selecting a preexisting option from the dropdown is optional; the user can enter a new value.

Multiple

The user may choose multiple options from the dropdown. Each selected option can be deselected either by clicking the option again in the drop down or by clicking the x icon next to the option in the list of selected options.

Tagging

The user may choose multiple options from the dropdown or enter in their own value which will become an option.

Opt Group

Description

The opt group is used to group related options in a down-down list.

Native OptGroup

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/optgroup

Prop Name Type Is Required Default Value Description
children node required none The group options.
disabled bool optional false Whether the group is disabled.
label node required none The group label.
Option | Prop Name | Type | Is Required | Default Value | Description | | --------- |-------| -------------| ------------- | ----------- | | children | node | optional | none | This should be used when custom option display content is needed. If both children and display are set, only display will be used. | | disabled | bool | optional | false | Indicates if option should be disabled. | | display | string | optional | none | Option display text. | | value | string | required | none | Option value. |

Added dependencies

Removed dependencies

Issue Type

Expected Behavior

The Select component should support multi-select.

Current Behavior

The Select component does not support multi-select.

emilyrohrbough commented 6 years ago

Adding numbered list for easy reference:

  1. Should it be noResultsContent instead of noResultContent.

  2. I think onFilter might better articulate that the proposed optionFilter is a callback function for option filtering.

  3. Why are we dropping the requestFocus and releaseFocus props? Are we dropping the terra-popup dependency? Or rolling our own request & release focus methods?

  4. The event object should be abstracted as it no longer contains true change information.

Would it make sense to pass the event information down to the consumer, even if its the the 'true' event?

  1. Is Opt Group a new component within select?
  2. Are the props listed under Option the props for the current SelectOption component props? I notice isSelected is not included. Or is Option A new component as well?
emilyrohrbough commented 6 years ago

You had mentioned the implementation behind select would be similar for each variant type, will the prop combinations (outside of default) be similar as well?

How do you expect to handling removing a user-defined option from the list of options? I did not see any props or description of this behavior. For example, what if I meant to add component but accidentally added compon

StephenEsser commented 6 years ago
  1. Because the search can return a single result or multiple results or no result at all I'm not strongly opinionated either way. Does anyone else have an opinion between noResultsContent or noResultContent?

For additional information Vue uses the singular approach: https://vue-multiselect.js.org/#sub-slots Ant avoids the plural / singular issue by using notFoundContent: https://ant.design/components/select/#Select-props

  1. In general the onHandler event pattern is used to react to a single change that has already happened and is bubbling up through the ancestors. It feels a little awkward for the return value of the handler to conditionally filter options. Do you have any examples where the onHandler callback return value is used in this way?

  2. The popup is being removed as a dependency and the focus requests are no longer necessary.

  3. Passing the event object back to the user in this specific situation breaks the abstraction of the event. The event should callback with the changed value. In this case I strongly believe the user should not care or even know about the original event object. Doing a search for how users are implementing the onChange callback for the select now shows every user being forced to define the event as a parameter so they can access the second parameter which is the value, but no user's are using the event object.

  4. OptGroup is a new component within Select. But will be accessed in the same way as Option. Select.OptGroup. The OptGroup is a native browser component. Here is some documentation: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/optgroup

  5. Option is the current Select Option props table. The isSelected prop still exists internally. This is one of the props that falls into a category that shouldn't be exposed to the user. We haven't come up with a way to hide "private" props yet so I just hid it from the props table for now. The isSelected prop will always be determined and overwritten by the Select component value. So a user has no control over this prop even if they set it.

For your second comment I'm not sure if I understand either question. As the tech design stands now, this is all one component that flexes based on the variant.

emilyrohrbough commented 6 years ago

Talked offline with @StephenEsser Qs - 1, 3-6 👍

  1. (again) It was clarified the filterOptions prop is a callback function that takes the list of filtered options, verifies which options should actually be filtered and return the true options to be displayed on filter. I do not have a strong option but something like verifyFilteredOptions or filteredOptions might make sense here.

Regarding second comment: Q1. I was wondering if each variant should be their own component to pull corresponding variant props up, such that the props passed tot he variant component would directly be passed to Select. However after discussing with Stephen each variant will likely use the same props.

Q2. Sounds like this might be a question for @neilpfeiffer on how the options can be added and removed on an uncontrolled default multiselect:

How do you expect to handle add/removing a user-defined option from the full list of options? I did not see any props or description of this behavior. For example, what if I meant to add component but accidentally added compon so I needed to remove compon from the list of options.

emilyrohrbough commented 6 years ago

+1 on tech design. Well thought out!

bjankord commented 6 years ago

+1 on tech design

PaulSolDev commented 6 years ago

JIRA created