Skip to content

Allow input to be updated with onInputChange#907

Merged
JedWatson merged 2 commits intoJedWatson:masterfrom
brettdewoody:feature/update-on-oninputchange
Apr 26, 2016
Merged

Allow input to be updated with onInputChange#907
JedWatson merged 2 commits intoJedWatson:masterfrom
brettdewoody:feature/update-on-oninputchange

Conversation

@brettdewoody
Copy link
Contributor

Allows the input to be modified by returning a value from onInputChange.

Refs #893.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 90.728% when pulling ef44593 on brettdewoody:feature/update-on-oninputchange into 1787a7e on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 90.949% when pulling 4e87373 on brettdewoody:feature/update-on-oninputchange into 1787a7e on JedWatson:master.

@bruderstein
Copy link
Collaborator

@brettdewoody see bruderstein@f964825 for my suggestion :)

@brettdewoody
Copy link
Contributor Author

As expected, much better. Should I close this PR in favor of bruderstein@f964825?

@bruderstein
Copy link
Collaborator

No, just grab my commit if you like it, and add it to your branch - it's
still your PR. Glad I could help out.

On Thu, 21 Apr 2016, 08:35 Brett DeWoody, notifications@github.com wrote:

As expected, much better. Should I close this PR in favor of
bruderstein/react-select@f964825
bruderstein@f964825
?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#907 (comment)

onInputChange is now called from `handleInputChange`, which is less
surprising than `componentDidUpdate`, and should stop the double render,
and hence the flash of typed content, as opposed to what is returned
from `onInputChange`.

Simplified the tests by using the existing `createComponentWithWrapper`
function, and using a stub. This should make the tests clearer.
Added a couple of tests for returning different things (e.g. null,
numbers), and one to check the new value is actually rendered in the
DOM.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 90.949% when pulling 8483f48 on brettdewoody:feature/update-on-oninputchange into 1787a7e on JedWatson:master.

@JedWatson
Copy link
Owner

Thanks @brettdewoody! Looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants