[changed] collapsable => collapsible property #603
Conversation
Discussion is here react-bootstrap#425. Components are involved: - Nav - Panel - CollapsibleNav Current property type checking for `collapsable` in `PanelGroup` is needless and has been removed.
| @@ -144,15 +147,16 @@ const Panel = React.createClass({ | |||
|
|
|||
| renderHeading() { | |||
| let header = this.props.header; | |||
| let collapsible = this.props.collapsible || this.props.collapsable; | |||
There was a problem hiding this comment.
don't know if we should be using "const" when possible.
|
Looks good! |
Agree. I don't have enough experience with ES6 yet 😉
My thoughts were: 'It is a local scope, hence I should use let instead of var',
and I've seen let classes = ... from other places and thought about one coding style.
But I hadn't given a thought about const.
Now looking at it again I see that if we change let collapsible => const collapsible,
then it has to be changed in other places too.
E.g. https://github.com/AlexKVal/react-bootstrap/blob/f1a5d04a23/src/CollapsibleNav.js#L62
https://github.com/AlexKVal/react-bootstrap/blob/f1a5d04a23/src/CollapsibleNav.js#L104
https://github.com/AlexKVal/react-bootstrap/blob/f1a5d04a23/src/CollapsibleNav.js#L118
and so on.
@mtscout6 what do you think about it ? Sure I will rewrite it all with no delay if it would need.
|
There will be merge conflict between #603 and #608 here If both are approved and any is merged I'll promptly fix merging conflict with another PR of course. (BTW I don't know is it possible to resolve such merge conflicts between two PRs before they are merged into master ? What is the best practice in such case ?) |
Discussion is here #425.
Components are involved:
Current property type checking for
collapsableinPanelGroupis needless and has been removed.
Closes #425