jquense / react-widgets

Polished, feature rich, accessible form inputs built with React
http://jquense.github.io/react-widgets/
MIT License
2.34k stars 395 forks source link

Combobox: 'Create new' option when no match found #89

Closed andru closed 9 years ago

andru commented 9 years ago

First up thanks for your work on this awesome library! It's by far the best engineered set of form components I've come across for React, particularly in terms of the thought put into extensibility.

I think the behaviour of the combobox when entering a value not present in data is ambiguous.

Current behaviour: If you enter an value for which there is no match in data it maintains a selection for the first item in the list. Hitting enter selects that value while hitting tab or clicking outside the element closes the dropdown leaving the unmatched value in the field.

Proposal: Add a new option to make this component behaviour match the behaviour of multiselect.

When this option is enabled, display a list item at the bottom which permits creating a new item with the current value. If there are no partial matches for the current value, select the 'Create` item by default.

When this option is disabled, only values present in data are permitted, and the behaviour of navigating away from the component should be consistent. When the component blurs, the currently selected menu item should be selected, or it should be reverted to the previous valid value (which could be blank).

I need this functionality for an app so I'm happy to make a pull request if it's desirable.

jquense commented 9 years ago

Hey there,

thanks for the kind words and thorough issue!

Having a "Select the typed value" item in the popup list seems like a good idea to me! It probably should just always be there all the time, but I could see it being configurable. I am a bit shaky on some of the other parts here though.

I don't think I want to limit Combobox values to just items in the list (even as a configurable prop), since typing stuff outside the list is the Combobox 's key distinguishing feature from the DropdownList. I think that if do need to limit the value to just the list the DropdownList works great for that. I'd love to hear your thoughts on why you'd use a Combobox for "limited" selections though.

Right now typing anything at all triggers the same onChange handler, (and 'onSelect' for a list item selection) which is in keeping with how onChange works on normal inputs in React. Again I think dis-aggregating that into two handlers (onChange, onTyped) complicates the core functionality of a Combobox . It is a bit awkward to have both sorts of values come through the same handler but they are easy to distinguish in the onChange handler. Wherever I can I want to keep the default semantics consistent and intact, which means avoiding "flushing" values onBlur, which brings me to the next thought.

It seems like you'd like some ability to limit changes until the user is "done" (via blur) or explicitly chooses the non-item from the list? I am a reticent to bake that sort of thing in to the Combobox myself as it could be implemented as a wrapper component (I can demonstrate if you'd like).

in all I think I agree that the behavior is ambiguous when dealing with items not in the list, some of that though may be unavoidable, but I definitely have room to improve on the current situation.

andru commented 9 years ago

If you've got an example of a wrapper component it's be great to see, I certainly appreciate the idea of complexity through composition over a component with endless configuration options.

The dropdown isn't a good solution for a very large list, and provides no intuitive means to enter an unspecified value.

I need a component which allows a user to filter a large list to a small number of values (or scroll the list if they wish). Each item in the list is a record with an id, so the ideal would be for the component to pass that id property on to the onSelect handler or, if the value entered is not a valid list item, trigger a handler to allow the app to create a new record based on their input, and set that new record as the active item in the list.

The filtering and item-creation aspect of the multiselect widget is actually much closer to this use case than the combobox, except that it's clearly purposed for multiple selection.

jquense commented 9 years ago

The dropdown isn't a good solution for a very large list, and provides no intuitive means to enter an unspecified value.

True. I keep going back and forth on adding a filter input to the dropdown, or at least making sure it can be done in user land, never enough time :P

Here is a really quick stab at a wrapper component, that limits selection to only list items. It doesn't do anything to create new values but hopefully you can see the general idea. It only gets you part of the way there, if you wanted the "Create Item" in the popup list the easiest way is probably to add it to core here (I'd be open to a PR for that piece), of you also probably could implement it via a the listComponent prop, but that's not the best documented area of the suite.

var MyComboBox = React.createClass({

  getInitialState(){
    return { value: this.props.value }
  },

  componentWillReceiveProps(props){
    this._needsValueFlush = false
    this.setState({ value: props.value })
  },

  render(){
    var value = this.state.value;

    return <ComboBox {...this.props} 
      value={value}
      onChange={ v => this._change(v) } 
      onBlur={ e => this._blur(e) }/>
  },

  _change(value){
    // this _should_ work and be performant, but you can always just check by id (like _.find())
    var isListItem = this.props.data.indexOf(value) !== -1 

    if ( isListItem)
      return this.props.onChange(value)

    this._needsValueFlush = true
    this.setState({ value })
  },

  _blur(){
    // the user moved away from the widget and its still not a list item
    if (this._needsValueFlush) {
      this._needsValueFlush = false
      // reset the value, props.value, should be the last "good" one
      // this would also be a good spot to fire an `onCreate()` with this.state.value
      // if you wanted to mimic the multiselect behavior 
      this.props.onChange(this.props.value) 
    }
  }
andru commented 9 years ago

Great, I'll have a play around and get back to you with a proposal if there's anything I can only achieve with a change to core.

ps. uncontrollable is super awesome. I've got so much wiring in my app ui components to keep state in the parent; uncontrollable is making my life so much easier.

jquense commented 9 years ago

Hehe, I'm glad to hear it, It has certainly made my life a lot easier as well.

andru commented 9 years ago

So I've played around and I'm actually using multiselect to achieve the functionality I need just by wrapping it to only accept the last selected value in the onChange handler, and adding some custom styling to remove the 'tag' style. It already supports all the other functionality I need.

As easy as it was to customize it with a wrapper, it feels a bit awkward using a multiple select component and then disabling the multiple part! Having to reunite the onChange and onSearch events to keep track of an unmatched value to save/delete in onBlur feels particularly hacky, but it works! I figure these wouldn't need to be separated for a single value component, since the onChange/onSelect duo used by combobox would cover the bases.

Maybe the majority of the functionality could be spun off into a base component which multiselect and other components could wrap?