Col - fix disabled width props from being passed to div#283
Merged
eddywashere merged 6 commits intoreactstrap:masterfrom Jan 11, 2017
hosmelq:master
Merged
Col - fix disabled width props from being passed to div#283eddywashere merged 6 commits intoreactstrap:masterfrom hosmelq:master
eddywashere merged 6 commits intoreactstrap:masterfrom
hosmelq:master
Conversation
TheSharpieOne
requested changes
Jan 11, 2017
| cssModule: PropTypes.object, | ||
| }; | ||
|
|
||
| const defaultProps = { |
Member
There was a problem hiding this comment.
defaultProps is used below
eddywashere
reviewed
Jan 11, 2017
| } | ||
| }); | ||
|
|
||
| if (attributes['xs']) { |
Member
There was a problem hiding this comment.
I'd be happy to add back support for false as I think Col should have some sort of default that takes care of the typical use case. I recently and foolishly moved delete attributes[colWidth]; after the prop check, it should instead revert back to the following in colWidths.forEach(colWidth => {:
// always clear out width props from attributes
delete attributes[colWidth];
if (!columnProp) {
return;
}
Contributor
Author
There was a problem hiding this comment.
Did you already moved this ?
Member
There was a problem hiding this comment.
It was previously before the columnProp check, see commit
eddywashere
requested changes
Jan 11, 2017
|
|
||
| const defaultProps = { | ||
| xs: true | ||
| xs: true |
Member
There was a problem hiding this comment.
nit: can you remove the extra spaces?
eddywashere
changed the title
eddywashere
approved these changes
Jan 11, 2017
Member
eddywashere
left a comment
eddywashere
left a comment
There was a problem hiding this comment.
I appreciate the quick changes! lgtm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sometimes you don't want the
colclass by default, and setting xs={false} works but i got this errorUnknown prop 'xs' on <div> tag. Remove this prop from the element.This prop is not removed from attributes.
https://github.com/reactstrap/reactstrap/blob/master/src/Col.js#L63