Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(native): Support both onChange and onChangeText props #462

Merged
merged 1 commit into from Jun 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -17,6 +17,7 @@ exports[`renders with React Native components 1`] = `
autoComplete="off"
id="downshift-0-input"
onBlur={[Function]}
onChange={[Function]}
onChangeText={[Function]}
onKeyDown={[Function]}
value=""
Expand Down
76 changes: 76 additions & 0 deletions other/react-native/__tests__/onChange-tests.js
@@ -0,0 +1,76 @@
import {Text, TextInput, View} from 'react-native'
import React from 'react'

// Note: test renderer must be required after react-native.
import TestRenderer from 'react-test-renderer'

import Downshift from '../../../dist/downshift.native.cjs'

const RootView = ({innerRef, ...rest}) => <View ref={innerRef} {...rest} />

test('calls onChange when TextInput changes values', () => {
const onChange = jest.fn()
const Input = jest.fn(props => <TextInput {...props} />)

const element = (
<Downshift>
{({getRootProps, getInputProps, getItemProps}) => (
<RootView {...getRootProps({refKey: 'innerRef'})}>
<Input {...getInputProps({onChange})} />
<View>
<Text {...getItemProps({item: 'foo', index: 0})}>foo</Text>
<Text {...getItemProps({item: 'bar', index: 1})}>bar</Text>
</View>
</RootView>
)}
</Downshift>
)
TestRenderer.create(element)

const [[firstArg]] = Input.mock.calls
expect(firstArg).toMatchObject({
onChange: expect.any(Function),
})
const fakeEvent = {nativeEvent: {text: 'foobar'}}
firstArg.onChange(fakeEvent)

expect(onChange).toHaveBeenCalledTimes(1)
expect(onChange).toHaveBeenCalledWith(fakeEvent)
})

test('calls onChangeText when TextInput changes values', () => {
const onChangeText = jest.fn()
const Input = jest.fn(props => <TextInput {...props} />)

const element = (
<Downshift>
{({getRootProps, getInputProps, getItemProps}) => (
<RootView {...getRootProps({refKey: 'innerRef'})}>
<Input {...getInputProps({onChangeText})} />
<View>
<Text {...getItemProps({item: 'foo', index: 0})}>foo</Text>
<Text {...getItemProps({item: 'bar', index: 1})}>bar</Text>
</View>
</RootView>
)}
</Downshift>
)
TestRenderer.create(element)

const [[firstArg]] = Input.mock.calls
expect(firstArg).toMatchObject({
onChangeText: expect.any(Function),
})
const fakeEvent = 'foobar'
firstArg.onChangeText(fakeEvent)

expect(onChangeText).toHaveBeenCalledTimes(1)
expect(onChangeText).toHaveBeenCalledWith(fakeEvent)
})

/*
eslint
react/prop-types: 0,
import/extensions: 0,
import/no-unresolved: 0
*/
33 changes: 0 additions & 33 deletions other/react-native/__tests__/render-tests.js
@@ -1,5 +1,3 @@
/* eslint-disable react/prop-types */
// eslint-disable-next-line import/no-unassigned-import
import {Text, TextInput, View} from 'react-native'
import React from 'react'

Expand Down Expand Up @@ -49,37 +47,6 @@ test('can use children instead of render prop', () => {
expect(childrenSpy).toHaveBeenCalledTimes(1)
})

test('calls onChange when TextInput changes values', () => {
const onChange = jest.fn()
const Input = jest.fn(props => <TextInput {...props} />)

const RootView = ({innerRef, ...rest}) => <View ref={innerRef} {...rest} />
const childrenSpy = jest.fn(({getRootProps, getInputProps, getItemProps}) => (
<RootView {...getRootProps({refKey: 'innerRef'})}>
<Input {...getInputProps({onChange})} />
<View>
<Text {...getItemProps({item: 'foo', index: 0})}>foo</Text>
<Text {...getItemProps({item: 'bar', index: 1})}>bar</Text>
</View>
</RootView>
))
const element = <Downshift>{childrenSpy}</Downshift>
TestRenderer.create(element)
expect(childrenSpy).toHaveBeenCalledTimes(1)

const [[firstArg]] = Input.mock.calls
expect(firstArg).toMatchObject({
// TODO: We shouldn't need to know about the internals of how we're affecting the TextInput and what props we're supplying.
// See https://github.com/paypal/downshift/issues/361
onChangeText: expect.any(Function),
})
const fakeEvent = 'foobar'
firstArg.onChangeText(fakeEvent)

expect(onChange).toHaveBeenCalledTimes(1)
expect(onChange).toHaveBeenCalledWith(fakeEvent)
})

/*
eslint
react/prop-types: 0,
Expand Down
62 changes: 44 additions & 18 deletions src/downshift.js
Expand Up @@ -639,31 +639,49 @@ class Downshift extends Component {

/////////////////////////////// INPUT

getInputProps = ({onKeyDown, onBlur, onChange, onInput, ...rest} = {}) => {
getInputProps = ({
onKeyDown,
onBlur,
onChange,
onInput,
onChangeText,
...rest
} = {}) => {
let onChangeKey
let eventHandlers = {}

/* istanbul ignore next (preact) */
if (preval`module.exports = process.env.BUILD_PREACT === 'true'`) {
onChangeKey = 'onInput'
/* istanbul ignore next (react-native) */
} else if (
preval`module.exports = process.env.BUILD_REACT_NATIVE === 'true'`
) {
onChangeKey = 'onChangeText'
} else {
onChangeKey = 'onChange'
}
const {inputValue, isOpen, highlightedIndex} = this.getState()
const eventHandlers = rest.disabled
? {}
: {
[onChangeKey]: callAllEventHandlers(
onChange,
onInput,
this.input_handleChange,
),
onKeyDown: callAllEventHandlers(onKeyDown, this.input_handleKeyDown),
onBlur: callAllEventHandlers(onBlur, this.input_handleBlur),
}

if (!rest.disabled) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love how this forking logic came out, so I'd love some feedback here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I don't think it's all that bad 👍

eventHandlers = {
[onChangeKey]: callAllEventHandlers(
onChange,
onInput,
this.input_handleChange,
),
onKeyDown: callAllEventHandlers(onKeyDown, this.input_handleKeyDown),
onBlur: callAllEventHandlers(onBlur, this.input_handleBlur),
}
}

/* istanbul ignore if (react-native) */
if (preval`module.exports = process.env.BUILD_REACT_NATIVE === 'true'`) {
eventHandlers = {
...eventHandlers,
onChangeText: callAllEventHandlers(
onChangeText,
onInput,
this.input_handleTextChange,
),
}
}

return {
'aria-autocomplete': 'list',
'aria-activedescendant':
Expand Down Expand Up @@ -692,11 +710,19 @@ class Downshift extends Component {
type: Downshift.stateChangeTypes.changeInput,
isOpen: true,
inputValue: preval`module.exports = process.env.BUILD_REACT_NATIVE === 'true'`
? /* istanbul ignore next (react-native) */ event
? /* istanbul ignore next (react-native) */ event.nativeEvent.text
: event.target.value,
})
}

input_handleTextChange /* istanbul ignore next (react-native) */ = text => {
this.internalSetState({
type: Downshift.stateChangeTypes.changeInput,
isOpen: true,
inputValue: text,
})
}

input_handleBlur = () => {
// Need setTimeout, so that when the user presses Tab, the activeElement is the next focused element, not the body element
setTimeout(() => {
Expand Down