Skip to content

Commit

Permalink
Fix: Spread 'textFieldProps' last for 'getInputProps()' (fixes #24) (#25
Browse files Browse the repository at this point in the history
)

Bug #24 discussed that the 'id' for the rendered <input> element
couldn't be set contrary to what documentation said. The propblem was we
were overwriting the user input for the 'id' value by spreading
'textFieldProps' onto the object passed to 'getInputProps()' first and
then applying our own 'id'. This commit fixes the issue by spreading the
'textFieldProps' lastly after we set our own 'id'.
	modified:   src/MUIPlacesAutocomplete.jsx
	modified:   test/test.jsx
  • Loading branch information
Giners authored Feb 6, 2018
1 parent 6d6727d commit a6790c2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/MUIPlacesAutocomplete.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export default class MUIPlacesAutocomplete extends React.Component {
return (
<div>
<Manager tag={false}>
<TextField {...getInputProps({ ...textFieldProps, id: 'mui-places-autocomplete-input' })} />
<TextField {...getInputProps({ id: 'mui-places-autocomplete-input', ...textFieldProps })} />
<Target>{renderTarget()}</Target>
{isOpen ? MUIPlacesAutocomplete.renderSuggestionsContainer(
suggestions,
Expand Down
34 changes: 32 additions & 2 deletions test/test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ describe('React component test: <MUIPlacesAutocomplete>', function () {
)

expect(mpaWrapper).to.not.be.null
expect(mpaWrapper.length).to.equal(1)
})

describe('Renders correctly for given application state:', function () {
Expand Down Expand Up @@ -199,15 +200,44 @@ describe('React component test: <MUIPlacesAutocomplete>', function () {
})

describe('Provides expected UX:', function () {
it('\'value\' input prop can be used to control <input>', function () {
it('\'textFieldProps.value\' prop can be used to control <input>', function () {
const controlValue = 'LOL Bananas'

mpaWrapper.setProps({ textFieldProps: { value: controlValue } })

expect(mpaWrapper.find(`input[value="${controlValue}"]`).length).to.equal(1)
})

it('\'onChange\' input prop invoked when input changed', function (done) {
it('\'textFieldProps.id\' prop can be used to set \'id\' on the <input> element', function () {
const idValue = 'lol-bananas'

// When testing if the 'id' value gets set properly on the resulting <input> element you must
// set the 'id' input prop when first mounting <MUIPlacesAutocomplete>. That is because the
// first time the <Downshift> element is mounted it sets and maintains the 'id' value
// throughout re-renderings as seen here:
// https://github.com/paypal/downshift/blob/118a87234a9331e716142acfb95eb411cc4f8015/src/downshift.js#L623
//
// This fact is important as the 'id' value is the last value set on the props returned from
// the 'getInputProps()' function as seen here:
// https://github.com/paypal/downshift/blob/118a87234a9331e716142acfb95eb411cc4f8015/src/downshift.js#L661
//
// In other words setting the props on an already mounted <MUIPlacesAutocomplete> won't change
// the value of 'id' and will result in the test always failing.
mpaWrapper = mount(
<MUIPlacesAutocomplete
onSuggestionSelected={() => {}}
renderTarget={() => (<div />)}
textFieldProps={{ id: idValue }}
/>,
)

expect(mpaWrapper).to.not.be.null
expect(mpaWrapper.length).to.equal(1)

expect(mpaWrapper.find(`input[id="${idValue}"]`).length).to.equal(1)
})

it('\'textFieldProps.onChange\' prop invoked when input changed', function (done) {
// Setup our wrapper to signal that our test has completed successfully
const testSuccessCB = (inputValue) => {
try {
Expand Down

0 comments on commit a6790c2

Please sign in to comment.