React-bootstrap-typeahead: Pass event instead of value for `onChange`

[piordanov]

It would be a lot more useful to pass an event object instead of simply the values when using the onChange prop. By just returning the value(s), there is no way to truly know what Typeahead called it originally, which makes implementing multiple Typaheads in a single form much more difficult (or at least convoluted). To make this functional now, I have to do the following:

The JSX from an overall larger form

<ControlLabel>List1</ControlLabel>
<Typeahead clearButton
        multiple
        type='text'
        name='item1'
        value={this.state.item1}
        onChange={this.handleItem1Change}
        options={this.state.list1}
/>

<ControlLabel>List2</ControlLabel>
<Typeahead clearButton
        multiple
        type='text'
        name='item2'
        value={this.state.item2}
        onChange={this.handleItem2Change}
        options={this.state.list2}
/>

These are the handlers I have to write.

    handleItem1Change(value) {
        this.handleTypeAheadChange('item1', value);
    }

    handleItem2Change(value) {
        this.handleTypeAheadChange('item2', value);
    }

    handleTypeAheadChange(name, values) {
        this.setState({
            [name]: values
        });
    }

This begs for a single handleChange function much like the example shown in React's Controlled Component Form. Having to do this workaround doesn't make any sense, but the way Typeahead's work right now there doesn't seem to be any other way that I could find. If there is a much simpler solution I would love to hear about it.

[troggo]

Here's a simpler way:

    handleTypeAheadChange = name => values => {
        this.setState({
            [name]: values
        });
    }

    <Typeahead
        onChange={this.handleTypeAheadChange('item1')}
        ...
    />
    <Typeahead
        onChange={this.handleTypeAheadChange('item2')}
        ...
    />

[ericgio]

I'd consider changing the component to return an event for onChange, but I'm not sure what the right API is. The analogous case is <select multiple />, which has e.target.selectedOptions to get the selected options, but that returns an array of HTMLElements rather than data. I'm open to suggestions.

[piordanov]

I notice in the docs that we have events for onFocus and onBlur, so passing in an event would be perfect. Not sure about e.target.selectedOptions but e.target.options does return HTMLElements
I wrote a gist using select multiple to get an idea how it currently looks maybe it'll help
https://gist.github.com/piordanov/69180a1378113dc61dacf4cc7cd7040a

[troggo]

In that case it's probably best to have a separate callback (onChangeEvent?) that returns the actual event. In most cases the current behaviour seems fine, so theres no need to force every handler to sift through html elements.

[piordanov]

I'm already worried about the fact there are two functions we already have, and we would have to add a third one. I'm not entirely clear about the difference of onChange and onInputChange.
Maybe onChange could instead have both a value and an event passed in.

[ericgio]

I would agree with @troggo at this point. The case for supplying an event is 1) not super-compelling, and 2) unclear in terms of implementation. I'm going to stick with the current API for now, but will leave this issue open for discussion.

I'm not entirely clear about the difference of onChange and onInputChange

From the props documentation:

• onChange: Invoked whenever items are added or removed
• onInputChange: Invoked when the input value changes

In other words, onInputChange is essentially the onChange handler for the actual input element and deals with changes to the string value. The top-level onChange handler returns the selected items, if any.

[dmytro-shchurov]

I really hope that that particular case (multiple typeaheads and simple handling logic - it's not always true that those values must go to a state, right?) won't cause changes to the API clearness. I'm personally tired of all those DOM objects and their wrappers, and all these :any. in TypeScript.
I work with data, and want to manage data.

[derwaldgeist]

I stumbled upon this when looking for an onSelected event. Is it really true that this doesn't exist, so you don't have a callback when the user actually selects one of the options?

And yes, I would also have expected that onInputChange would pass an event with the target element being part of it. That's just how these kind of APIs usually work.

[daschi]

I also came across this issue because I am trying to access the event via onInputChange, and I agree with @derwaldgeist that it would be great for it to pass the event as a second parameter along with the input value.

@ericgio I also have a question about onKeyDown, I'm wondering if this is supposed to be available to pass down from the Typeahead props, so I can see the key pressed in the input field. I tried to do this and it seems to not get fired from the TypeaheadInput when passed in via Typeahead. Thanks for your help and work on this!

[ericgio]

I just want to clarify a few points, since it seems like some issues are being conflated:

I stumbled upon this when looking for an onSelected event. Is it really true that this doesn't exist, so you don't have a callback when the user actually selects one of the options?

As noted further up the thread, onChange is the correct prop to use.

The problem with receiving an event from onChange is that it's really just a hook to signal a change in the component state and not a true DOM event. I could forward the initiating event ('click', 'keydown', etc.) but that seems inconsistent. I could create a custom event. I guess my question is, is an event really useful here or is it more of a consistency issue?

[I]t would be great for onInputChange to pass the event as a second parameter along with the input value

Sure, I don't see why not.

I'm wondering if onKeyDown is supposed to be available to pass down from the Typeahead props

onKeyDown was added in v2.0.0-alpha.1. It should be available if you have the up-to-date version.

[daschi]

Cool, thank you! I'll open a PR for passing event to onInputChange.

[derwaldgeist]

@ericgio Thanks for sharing your insights!

[ericgio]

As of v3.0.0, onInputChange passes the input's change event as the second param:

<Typeahead
  ...
  onInputChange={(text, event) => {
    ...
  }}
/>

[ericgio]

Closing, since there isn't a true underlying event associated with selections changing, and therefore no plans to provide an event. Further discussion around naming should happen in #302.