From 2b8dc5fb3ea46de84a7f56c91831c85cd9232aa2 Mon Sep 17 00:00:00 2001 From: AlexKVal Date: Mon, 4 May 2015 23:06:02 +0300 Subject: [PATCH 1/2] Fix the subtle bug with deprecation warning. When deprecated `Collaps_a_bleNav` is imported it also tags `Collaps_i_bleNav` element as deprecated, because it is done by using pointers which points to the same object. ```js let CollapsableNav = CollapsibleNav; CollapsableNav.__deprecated__ = true; ``` https://github.com/react-bootstrap/react-bootstrap/blob/92c57ef7ee/src/CollapsableNav.js#L5 The right and simplest way to make deprecation in the case of component renaming `CollapsableNav => CollapsibleNav` by using general for both components part - `classSpecifications`. Like this: ```js // NewNameComponent.js const classSpecificationsFor_NewNameComponent = { /* existing code */ }; const NewNameComponent = React.createClass( classSpecificationsFor_NewNameComponent ); export {classSpecificationsFor_NewNameComponent}; // for old component export default NewNameComponent; // OldNameComponent.js import {classSpecificationsFor_NewNameComponent} from 'NewNameComponent'; const classSpecsFor_OldNameComponent = assign({}, classSpecificationsFor_NewNameComponent, { // here we add deprecation warning }); const OldNameComponent = React.createClass( classSpecsFor_OldNameComponent ); export default OldNameComponent; ``` --- src/CollapsableNav.js | 17 ++++++++++++++--- src/CollapsibleNav.js | 18 +++++------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/CollapsableNav.js b/src/CollapsableNav.js index 07a7a2f928..3f3ee153c5 100644 --- a/src/CollapsableNav.js +++ b/src/CollapsableNav.js @@ -1,7 +1,18 @@ -import CollapsibleNav from './CollapsibleNav'; +import React from 'react'; +import deprecationWarning from './utils/deprecationWarning'; +import assign from './utils/Object.assign'; +import {specCollapsibleNav} from './CollapsibleNav'; -let CollapsableNav = CollapsibleNav; +const specCollapsableNav = assign({}, specCollapsibleNav, { + componentDidMount() { + deprecationWarning( + 'CollapsableNav', + 'CollapsibleNav', + 'https://github.com/react-bootstrap/react-bootstrap/issues/425#issuecomment-97110963' + ); + } +}); -CollapsableNav.__deprecated__ = true; +const CollapsableNav = React.createClass(specCollapsableNav); export default CollapsableNav; diff --git a/src/CollapsibleNav.js b/src/CollapsibleNav.js index 879c74ccd0..fe193e9f69 100644 --- a/src/CollapsibleNav.js +++ b/src/CollapsibleNav.js @@ -3,12 +3,11 @@ import BootstrapMixin from './BootstrapMixin'; import CollapsibleMixin from './CollapsibleMixin'; import classNames from 'classnames'; import domUtils from './utils/domUtils'; -import deprecationWarning from './utils/deprecationWarning'; import ValidComponentChildren from './utils/ValidComponentChildren'; import createChainedFunction from './utils/createChainedFunction'; -const CollapsibleNav = React.createClass({ +const specCollapsibleNav = { mixins: [BootstrapMixin, CollapsibleMixin], propTypes: { @@ -43,16 +42,6 @@ const CollapsibleNav = React.createClass({ return height; }, - componentDidMount() { - if (this.constructor.__deprecated__) { - deprecationWarning( - 'CollapsableNav', - 'CollapsibleNav', - 'https://github.com/react-bootstrap/react-bootstrap/issues/425#issuecomment-97110963' - ); - } - }, - render() { /* * this.props.collapsable is set in NavBar when a eventKey is supplied. @@ -127,6 +116,9 @@ const CollapsibleNav = React.createClass({ } ); } -}); +}; + +const CollapsibleNav = React.createClass(specCollapsibleNav); +export {specCollapsibleNav}; export default CollapsibleNav; From 51a205f530c177e341b2dfde3116025443437c90 Mon Sep 17 00:00:00 2001 From: AlexKVal Date: Mon, 4 May 2015 23:07:29 +0300 Subject: [PATCH 2/2] [changed] collapsable => collapsible property Discussion is here #425. Components are involved: - Nav - Panel - CollapsibleNav Current property type checking for `collapsable` in `PanelGroup` is needless and has been removed. Tests for deprecated `collapsable` property for all three components has been placed into one file `CollapsableNavSpec.js` --- docs/examples/PanelListGroupFill.js | 2 +- src/CollapsibleNav.js | 13 +++-- src/Nav.js | 12 ++-- src/Navbar.js | 2 +- src/Panel.js | 14 +++-- src/PanelGroup.js | 3 +- src/utils/deprecatedProperty.js | 13 +++++ test/CollapsableNavSpec.js | 90 +++++++++++++++++++++++++++++ test/CollapsibleNavSpec.js | 2 +- test/PanelSpec.js | 20 +++---- 10 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 src/utils/deprecatedProperty.js create mode 100644 test/CollapsableNavSpec.js diff --git a/docs/examples/PanelListGroupFill.js b/docs/examples/PanelListGroupFill.js index 20cd1f2104..fcb1c94cfc 100644 --- a/docs/examples/PanelListGroupFill.js +++ b/docs/examples/PanelListGroupFill.js @@ -1,5 +1,5 @@ const panelInstance = ( - + Some default panel content here. Item 1 diff --git a/src/CollapsibleNav.js b/src/CollapsibleNav.js index fe193e9f69..4f5fa7300d 100644 --- a/src/CollapsibleNav.js +++ b/src/CollapsibleNav.js @@ -3,6 +3,7 @@ import BootstrapMixin from './BootstrapMixin'; import CollapsibleMixin from './CollapsibleMixin'; import classNames from 'classnames'; import domUtils from './utils/domUtils'; +import collapsable from './utils/deprecatedProperty'; import ValidComponentChildren from './utils/ValidComponentChildren'; import createChainedFunction from './utils/createChainedFunction'; @@ -14,7 +15,8 @@ const specCollapsibleNav = { onSelect: React.PropTypes.func, activeHref: React.PropTypes.string, activeKey: React.PropTypes.any, - collapsable: React.PropTypes.bool, + collapsable, + collapsible: React.PropTypes.bool, expanded: React.PropTypes.bool, eventKey: React.PropTypes.any }, @@ -44,9 +46,10 @@ const specCollapsibleNav = { render() { /* - * this.props.collapsable is set in NavBar when a eventKey is supplied. + * this.props.collapsible is set in NavBar when a eventKey is supplied. */ - let classes = this.props.collapsable ? this.getCollapsibleClassSet() : {}; + const collapsible = this.props.collapsible || this.props.collapsable; + let classes = collapsible ? this.getCollapsibleClassSet() : {}; /* * prevent duplicating navbar-collapse call if passed as prop. * kind of overkill... @@ -55,13 +58,13 @@ const specCollapsibleNav = { */ if (this.props.className === undefined || this.props.className.split(' ').indexOf('navbar-collapse') === -2) { - classes['navbar-collapse'] = this.props.collapsable; + classes['navbar-collapse'] = collapsible; } return (
{ValidComponentChildren.map(this.props.children, - this.props.collapsable ? + collapsible ? this.renderCollapsibleNavChildren : this.renderChildren )} diff --git a/src/Nav.js b/src/Nav.js index 8633769269..b3097cc9a4 100644 --- a/src/Nav.js +++ b/src/Nav.js @@ -3,7 +3,7 @@ import BootstrapMixin from './BootstrapMixin'; import CollapsibleMixin from './CollapsibleMixin'; import classNames from 'classnames'; import domUtils from './utils/domUtils'; - +import collapsable from './utils/deprecatedProperty'; import ValidComponentChildren from './utils/ValidComponentChildren'; import createChainedFunction from './utils/createChainedFunction'; @@ -18,7 +18,8 @@ const Nav = React.createClass({ stacked: React.PropTypes.bool, justified: React.PropTypes.bool, onSelect: React.PropTypes.func, - collapsable: React.PropTypes.bool, + collapsable, + collapsible: React.PropTypes.bool, expanded: React.PropTypes.bool, navbar: React.PropTypes.bool, eventKey: React.PropTypes.any, @@ -45,11 +46,12 @@ const Nav = React.createClass({ }, render() { - let classes = this.props.collapsable ? this.getCollapsibleClassSet() : {}; + const collapsible = this.props.collapsible || this.props.collapsable; + let classes = collapsible ? this.getCollapsibleClassSet() : {}; - classes['navbar-collapse'] = this.props.collapsable; + classes['navbar-collapse'] = collapsible; - if (this.props.navbar && !this.props.collapsable) { + if (this.props.navbar && !collapsible) { return (this.renderUl()); } diff --git a/src/Navbar.js b/src/Navbar.js index de7f300551..785c2518a2 100644 --- a/src/Navbar.js +++ b/src/Navbar.js @@ -85,7 +85,7 @@ const Navbar = React.createClass({ renderChild(child, index) { return cloneElement(child, { navbar: true, - collapsable: this.props.toggleNavKey != null && this.props.toggleNavKey === child.props.eventKey, + collapsible: this.props.toggleNavKey != null && this.props.toggleNavKey === child.props.eventKey, expanded: this.props.toggleNavKey != null && this.props.toggleNavKey === child.props.eventKey && this.isNavExpanded(), key: child.key ? child.key : index }); diff --git a/src/Panel.js b/src/Panel.js index 66a9b0620d..067207d670 100644 --- a/src/Panel.js +++ b/src/Panel.js @@ -3,12 +3,14 @@ import classNames from 'classnames'; import BootstrapMixin from './BootstrapMixin'; import CollapsibleMixin from './CollapsibleMixin'; +import collapsable from './utils/deprecatedProperty'; const Panel = React.createClass({ mixins: [BootstrapMixin, CollapsibleMixin], propTypes: { - collapsable: React.PropTypes.bool, + collapsable, + collapsible: React.PropTypes.bool, onSelect: React.PropTypes.func, header: React.PropTypes.node, id: React.PropTypes.string, @@ -55,13 +57,14 @@ const Panel = React.createClass({ render() { let classes = this.getBsClassSet(); + const collapsible = this.props.collapsible || this.props.collapsable; return (
+ id={collapsible ? null : this.props.id} onSelect={null}> {this.renderHeading()} - {this.props.collapsable ? this.renderCollapsableBody() : this.renderBody()} + {collapsible ? this.renderCollapsableBody() : this.renderBody()} {this.renderFooter()}
); @@ -144,15 +147,16 @@ const Panel = React.createClass({ renderHeading() { let header = this.props.header; + const collapsible = this.props.collapsible || this.props.collapsable; if (!header) { return null; } if (!React.isValidElement(header) || Array.isArray(header)) { - header = this.props.collapsable ? + header = collapsible ? this.renderCollapsableTitle(header) : header; - } else if (this.props.collapsable) { + } else if (collapsible) { header = cloneElement(header, { className: classNames(this.prefixClass('title')), diff --git a/src/PanelGroup.js b/src/PanelGroup.js index 164e0c481b..6b6a3eda62 100644 --- a/src/PanelGroup.js +++ b/src/PanelGroup.js @@ -10,7 +10,6 @@ const PanelGroup = React.createClass({ mixins: [BootstrapMixin], propTypes: { - collapsable: React.PropTypes.bool, accordion: React.PropTypes.bool, activeKey: React.PropTypes.any, defaultActiveKey: React.PropTypes.any, @@ -51,7 +50,7 @@ const PanelGroup = React.createClass({ }; if (this.props.accordion) { - props.collapsable = true; + props.collapsible = true; props.expanded = (child.props.eventKey === activeKey); props.onSelect = this.handleSelect; } diff --git a/src/utils/deprecatedProperty.js b/src/utils/deprecatedProperty.js new file mode 100644 index 0000000000..06732193c3 --- /dev/null +++ b/src/utils/deprecatedProperty.js @@ -0,0 +1,13 @@ +import React from 'react'; +import deprecationWarning from './deprecationWarning'; + +export default function collapsable(props, propName, componentName) { + if (props[propName] !== undefined) { + deprecationWarning( + `${propName} in ${componentName}`, + 'collapsible', + 'https://github.com/react-bootstrap/react-bootstrap/issues/425' + ); + } + return React.PropTypes.bool.call(null, props, propName, componentName); +} diff --git a/test/CollapsableNavSpec.js b/test/CollapsableNavSpec.js new file mode 100644 index 0000000000..99b1d13ae6 --- /dev/null +++ b/test/CollapsableNavSpec.js @@ -0,0 +1,90 @@ +import React from 'react'; +import ReactTestUtils from 'react/lib/ReactTestUtils'; +import CollapsibleNav from '../src/CollapsibleNav'; +import Nav from '../src/Nav'; +import Panel from '../src/Panel'; +import {shouldWarn} from './helpers'; + +describe('Deprecations for collapsable property in CollapsibleNav', function () { + it('Should not warn about deprecation when collaps_i_ble property is used', function () { + let Component = React.createClass({ + render: function() { + return ( + + ); + } + }); + ReactTestUtils.renderIntoDocument(); + + console.warn.called.should.be.false; + }); + + it('Should warn about deprecation when collaps_a_ble property is used', function () { + let Component = React.createClass({ + render: function() { + return ( + + ); + } + }); + ReactTestUtils.renderIntoDocument(); + + shouldWarn('deprecated'); + }); +}); + +describe('Deprecations for collapsable property in Panel', function () { + it('Should not warn about deprecation when collaps_i_ble property is used', function () { + let Component = React.createClass({ + render: function() { + return ( + + ); + } + }); + ReactTestUtils.renderIntoDocument(); + + console.warn.called.should.be.false; + }); + + it('Should warn about deprecation when collaps_a_ble property is used', function () { + let Component = React.createClass({ + render: function() { + return ( + + ); + } + }); + ReactTestUtils.renderIntoDocument(); + + shouldWarn('deprecated'); + }); +}); + +describe('Deprecations for collapsable property in Nav', function () { + it('Should not warn about deprecation when collaps_i_ble property is used', function () { + let Component = React.createClass({ + render: function() { + return ( +