Dropdown: call onAddItem after onChange

[idmadj]

Steps

(Using the testcase)

1. Use options with distinct text and value properties;
2. In a Dropdown with allowAdditions set, type a new entry and confirm addition;

Expected Result

A way to set a custom value to the newly added item is provided before it is auto-selected.

Actual Result

The newly added item has both its text and value properties set to the input text and is then selected based on that string.

This is caused by the item insertion logic in getMenuOptions that sets both the text and value properties of new items to the user-inserted text when the user confirms the entry. onAddItem is then called with the new value (which is fine), immediately followed by setValue, setSelectedIndex and handleChange, which attempt to select the item using the same value.

This behavior gives no opportunity to set an appropriate value property (a unique ID, for instance) to the newly inserted item before it is auto-selected. In the testcase, this is attempted in the onAddItem handler, but setting the value property there means the setValue, setSelectedIndex and handleChange calls fail as they use the old value (the user-inserted text). There is then no elegant way to select the newly added item.

Version

0.73.1

Testcase

https://codepen.io/anon/pen/xXEVqK

[layershifter]

Why don't use controlled components?

Modified codepen

[idmadj]

Hey layershifter! Thanks for looking at this.

It already is a controlled component. The currentValue state is used to store the value.

The modified pen allows selection of the newly added value because there are now two states that hold the value (state.currentValue and state.value), but then other items can no longer be selected as state.value has priority (the newly added item remains selected until another item is added).

If we use the same state to hold value in both the onAddItem and onChange handlers, the state change in onAddItem is ignored as the code in getMenuOptions immediately calls handleChange (which calls onChange), changing the state to the initial value (the user-inserted text). (codepen)

It's possible to work around this by storing the newly added value in a class property (in the onAddItem handler), then using that when changing the value state in the onChange handler, although that's not the cleanest of ways. (codepen)

My guess is that a new function prop that allows customizing the value for new items would resolve this, but I'd like to hear your thoughts first.

[layershifter]

My guess is that a new function prop that allows customizing the value for new items would resolve this

I will be first who down wote this solution 😄 Dropdown is already to complicated component.

I'm that there is a simple and elegant solution, it's a call onAddItem after onChange:

this.handleChange(e, newValue)

// Heads up! This event handler should be called after `onChange`
// Notify the onAddItem prop if this is a new value
if (item['data-additional']) _.invoke(this.props, 'onAddItem', e, { ...this.props, value }))

We also should add new test that asserts this:

const onChange = spy.sandbox()
const onAddItem = spy.sandbox()
wrapperMount(<Dropdown onAddItem={onAddItem} onChange={onChange) />

// chore work to add item

onChange.should.have.been.calledOnce()
onAddItem.should.have.been.calledOnce()
onAddItem.should.have.been.calledImmediatelyAfter(onChange)

I think this will fully solve your problem.

[layershifter]

I want to include this in next release, so picking up

[nathanhannig]

I don't see how the PR fixes the issue reported where we need to create and select the unique ID of a newly added option.

This is extremely important in multiple select dropdown