search

fmoo / react-typeahead

Pure react-based typeahead and typeahead-tokenizer
ISC License
677 stars 232 forks source link

props.disable is a breaking change in 1.1.7 #173

Closed nosilleg closed 8 years ago

nosilleg commented 8 years ago

Previously we were passing in disabled to props.inputProps, but with the change in 1.1.7 this is now overwritten by props.disable which we are not passing in (because it didn't exist in prior releases.)

As such, our previously disabled inputs are no longer disabled.

See https://github.com/fmoo/react-typeahead/commit/478dbfcbc8325d26c5ac6dbfaf26ade7b3ec517a#diff-f431d994af3ed5b8d4a8628b2978088aR316

There are at least 3 solutions to this:

  1. Everyone updates to use the new prop.
  2. The new prop is removed because it was redundant anyway as it could be specified in props.inputProps.
  3. The new disable prop can be set on <InputElement before the spread is done on props.inputProps, so that inputProps takes priority.

Thoughts?

fmoo commented 8 years ago

Not a fan of 1 on a release revision.

If we go with 2, it's arguable we should clean up all the redundant methods (onKeyUp, onKeyDown, etc), but this isn't a release revision change either. We should probably update the docs so people stop submitting this type of change.

3 makes the most sense in the short-term, I think. I'll merge in and cut a release ASAP if you put a PR up.

nosilleg commented 8 years ago

Sounds good.

nosilleg commented 8 years ago

At a computer now. Have created the PR #175

nosilleg commented 8 years ago

Note that this is a distinct issue from 'disabled' vs 'disable'