From a6790c21309204aa0242c6496be366abdaa3e370 Mon Sep 17 00:00:00 2001 From: Chris Austin Date: Tue, 6 Feb 2018 09:05:07 -0800 Subject: [PATCH] Fix: Spread 'textFieldProps' last for 'getInputProps()' (fixes #24) (#25) Bug #24 discussed that the 'id' for the rendered 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 --- src/MUIPlacesAutocomplete.jsx | 2 +- test/test.jsx | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/MUIPlacesAutocomplete.jsx b/src/MUIPlacesAutocomplete.jsx index 330c52e..f3fe523 100644 --- a/src/MUIPlacesAutocomplete.jsx +++ b/src/MUIPlacesAutocomplete.jsx @@ -241,7 +241,7 @@ export default class MUIPlacesAutocomplete extends React.Component { return (
- + {renderTarget()} {isOpen ? MUIPlacesAutocomplete.renderSuggestionsContainer( suggestions, diff --git a/test/test.jsx b/test/test.jsx index 167d753..3c8cf07 100644 --- a/test/test.jsx +++ b/test/test.jsx @@ -74,6 +74,7 @@ describe('React component test: ', function () { ) expect(mpaWrapper).to.not.be.null + expect(mpaWrapper.length).to.equal(1) }) describe('Renders correctly for given application state:', function () { @@ -199,7 +200,7 @@ describe('React component test: ', function () { }) describe('Provides expected UX:', function () { - it('\'value\' input prop can be used to control ', function () { + it('\'textFieldProps.value\' prop can be used to control ', function () { const controlValue = 'LOL Bananas' mpaWrapper.setProps({ textFieldProps: { value: controlValue } }) @@ -207,7 +208,36 @@ describe('React component test: ', function () { 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 element', function () { + const idValue = 'lol-bananas' + + // When testing if the 'id' value gets set properly on the resulting element you must + // set the 'id' input prop when first mounting . That is because the + // first time the 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 won't change + // the value of 'id' and will result in the test always failing. + mpaWrapper = mount( + {}} + renderTarget={() => (
)} + 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 {