Skip to content

Commit

Permalink
feat(props): add onInputValueChange (#230)
Browse files Browse the repository at this point in the history
* fix: cursor jumping to end of input

provide an onInputValueChange callback to allow to update controlled prop
before its old value is used to re-render the children
causing the cursor in the input to jump to the end in the next render

* Update downshift.js

* docs: add documentation for onInputValueChange
  • Loading branch information
robin-drexler authored and Kent C. Dodds committed Oct 22, 2017
1 parent 12f9d6d commit 24b3f9d
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 2 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,16 @@ but differ slightly.
> so it's not recommended to use it if you can avoid it. If you need it, please
> open an issue to discuss solidifying the API.
### onInputValueChange

> `function(inputValue: string, stateAndHelpers: object)` | optional, no useful default
Called whenever the input value changes. Useful to use instead or in combination of `onStateChange` when `inputValue` is a controlled prop to [avoid issues with cursor positions](https://github.com/paypal/downshift/issues/217).

- `inputValue`: The current value of the input
- `stateAndHelpers`: This is the same thing your `children` prop
function is called with (see [Child Callback Function](#child-callback-function))

### itemCount

> `number` | optional, defaults the number of times you call getItemProps
Expand Down
33 changes: 33 additions & 0 deletions src/__tests__/downshift.props.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,39 @@ test('can override onOuterClick callback to maintain isOpen state', () => {
)
})

test('onInputValueChange called when changes contain inputValue', () => {
const handleInputValueChange = jest.fn()
const {selectItem} = setup({
onInputValueChange: handleInputValueChange,
})
selectItem('foo')
expect(handleInputValueChange).toHaveBeenCalledTimes(1)
expect(handleInputValueChange).toHaveBeenCalledWith(
'foo',
expect.any(Object),
)
})

test('onInputValueChange not called when changes do not contain inputValue', () => {
const handleInputValueChange = jest.fn()
const {openMenu} = setup({
onInputValueChange: handleInputValueChange,
})
openMenu()

expect(handleInputValueChange).toHaveBeenCalledTimes(0)
})

test('onInputValueChange called with empty string on reset', () => {
const handleInputValueChange = jest.fn()
const {reset} = setup({
onInputValueChange: handleInputValueChange,
})
reset()
expect(handleInputValueChange).toHaveBeenCalledTimes(1)
expect(handleInputValueChange).toHaveBeenCalledWith('', expect.any(Object))
})

function mouseDownAndUp(node) {
node.dispatchEvent(new window.MouseEvent('mousedown', {bubbles: true}))
node.dispatchEvent(new window.MouseEvent('mouseup', {bubbles: true}))
Expand Down
29 changes: 27 additions & 2 deletions src/downshift.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Downshift extends Component {
itemToString: PropTypes.func,
onChange: PropTypes.func,
onStateChange: PropTypes.func,
onInputValueChange: PropTypes.func,
onUserAction: PropTypes.func,
onClick: PropTypes.func,
onOuterClick: PropTypes.func,
Expand Down Expand Up @@ -62,6 +63,7 @@ class Downshift extends Component {
id: generateId('downshift'),
itemToString: i => (i == null ? '' : String(i)),
onStateChange: () => {},
onInputValueChange: () => {},
onUserAction: () => {},
onChange: () => {},
onOuterClick: () => {},
Expand Down Expand Up @@ -268,11 +270,23 @@ class Downshift extends Component {
internalSetState(stateToSet, cb) {
let onChangeArg
const onStateChangeArg = {}
const isStateToSetFunction = typeof stateToSet === 'function'

// we want to call `onInputValueChange` before the `setState` call
// so someone controlling the `inputValue` state gets notified of
// the input change as soon as possible. This avoids issues with
// preserving the cursor position.
// See https://github.com/paypal/downshift/issues/217 for more info.
if (!isStateToSetFunction && stateToSet.hasOwnProperty('inputValue')) {
this.props.onInputValueChange(stateToSet.inputValue, {
...this.getStateAndHelpers(),
...stateToSet,
})
}
return this.setState(
state => {
state = this.getState(state)
stateToSet =
typeof stateToSet === 'function' ? stateToSet(state) : stateToSet
stateToSet = isStateToSetFunction ? stateToSet(state) : stateToSet
// this keeps track of the object we want to call with setState
const nextState = {}
// this is just used to tell whether the state changed
Expand All @@ -287,6 +301,7 @@ class Downshift extends Component {
onChangeArg = stateToSet.selectedItem
}
stateToSet.type = stateToSet.type || Downshift.stateChangeTypes.unknown

Object.keys(stateToSet).forEach(key => {
// onStateChangeArg should only have the state that is
// actually changing
Expand All @@ -308,6 +323,16 @@ class Downshift extends Component {
nextState[key] = stateToSet[key]
}
})

// if stateToSet is a function, then we weren't able to call onInputValueChange
// earlier, so we'll call it now that we know what the inputValue state will be.
if (isStateToSetFunction && stateToSet.hasOwnProperty('inputValue')) {
this.props.onInputValueChange(stateToSet.inputValue, {
...this.getStateAndHelpers(),
...stateToSet,
})
}

return nextState
},
() => {
Expand Down

0 comments on commit 24b3f9d

Please # to comment.