[fixed] Fix for #507#514
Conversation
There was a problem hiding this comment.
We should not be mutating this.props. Future version of react will fail when doing this. They have plans to incorporate immutablejs with props.
There was a problem hiding this comment.
Got it.
I will review and refactor this.
|
I haven't been able to find a better solution. |
There was a problem hiding this comment.
This is a great valid use case to present to the eslint folks. Have you considered submitting an issue with them so in the future we wouldn't need to do this?
There was a problem hiding this comment.
I've pondered about it.
The problem with that particular case is that only developer knows for sure is the var necessary or not. So it has to be explicitly marked.
|
I agree that this change is a crutch until #342 is tackled but we already know that so this will suffice till then. Can you squash the three commits? There's no need for three commits to reflect this change. |
In the case when `<Input />` type is `submit`, bootstrap style set in `bsStyle` property goes down to inner `<Button />` conponent. Consequently default bootstrap styles for `<Button />` are not suited for `<FormGroup />` component that is the root element for `<Input />` component. Therefore in that case we just need to prevent `bsStyle` passing on to <FormGroup />. Tests added and temporarily suspended test is unskipped.
I've done three stand-alone commits because I had thought it needs to be formed like this. Done. As for creating new issue to |
|
looks good |
3 participants
Beginning is at #509.
Actually the fix itself begins from
Add tests for fixing bug507552de20TravisCI would fail without merging #513.
And it would need to create additional PR for test unskipping.
With #513 this PR is
greenon TravisCI https://travis-ci.org/AlexKVal/react-bootstrap/builds/58300955