From e021050b57d0650bc94d6109cf30719e34a89096 Mon Sep 17 00:00:00 2001 From: AlexKVal Date: Sun, 24 May 2015 18:48:52 +0300 Subject: [PATCH] Refactor duplication-removing logic for CollapsibleNav. Because of `-2` this code `indexOf('navbar-collapse') === -2` hasn't been working anyway. But tests were green because the resulted logic was right someway. This commit refactors it to clarify deduplication logic by using `classnames'` [Alternate `dedupe` version](https://github.com/JedWatson/classnames/commit/9acbd433ee) `classes['navbar-collapse'] = collapsible` => `let classes = collapsible ? this.getCollapsibleClassSet('navbar-collapse')` `getDOMNode()` => `React.findDOMNode()` And test was refactored too. We can count class occurrences much simpler as ```js let occurrences = (classDOM.match(/navbar-collapse/g) || []).length ``` --- src/CollapsibleNav.js | 14 ++------------ test/CollapsibleNavSpec.js | 24 +----------------------- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/src/CollapsibleNav.js b/src/CollapsibleNav.js index 664e4049ad..1f4927f558 100644 --- a/src/CollapsibleNav.js +++ b/src/CollapsibleNav.js @@ -46,20 +46,10 @@ const specCollapsibleNav = { render() { /* - * this.props.collapsible is set in NavBar when a eventKey is supplied. + * this.props.collapsible is set in NavBar when an eventKey is supplied. */ 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... - * good cadidate to have check implemented as an util that can - * also be used elsewhere. - */ - if (this.props.className === undefined || - this.props.className.split(' ').indexOf('navbar-collapse') === -2) { - classes['navbar-collapse'] = collapsible; - } + let classes = collapsible ? this.getCollapsibleClassSet('navbar-collapse') : null; return (
diff --git a/test/CollapsibleNavSpec.js b/test/CollapsibleNavSpec.js index 096ebc2673..e8acc49b80 100644 --- a/test/CollapsibleNavSpec.js +++ b/test/CollapsibleNavSpec.js @@ -64,7 +64,7 @@ describe('CollapsibleNav', function () { }); let instance = ReactTestUtils.renderIntoDocument(); let collapsibleNav = ReactTestUtils.findRenderedComponentWithType(instance, CollapsibleNav); - assert.notOk(collapsibleNav.getDOMNode().className.match(/\navbar-collapse\b/)); + assert.notOk(React.findDOMNode(collapsibleNav).className.match(/\navbar-collapse\b/)); let nav = ReactTestUtils.findRenderedComponentWithType(collapsibleNav.refs.nocollapse_0, Nav); assert.ok(nav); }); @@ -89,26 +89,4 @@ describe('CollapsibleNav', function () { assert.ok(ReactTestUtils.findRenderedDOMComponentWithClass(instance.refs.collapsible_object.refs.collapsible_0, 'bar')); assert.ok(ReactTestUtils.findRenderedDOMComponentWithClass(instance.refs.collapsible_object.refs.collapsible_0, 'baz')); }); - - it('Should should not duplicate classes', function () { - let Parent = React.createClass({ - render() { - return ( - - - - - - ); - } - }); - let instance = ReactTestUtils.renderIntoDocument(); - let classDOM = ReactTestUtils.findRenderedDOMComponentWithTag(instance.refs.collapsible_object, 'DIV').props.className - , classArray = classDOM.split(' ') - , idx = classArray.indexOf('navbar-collapse'); - assert.equal(classArray.indexOf('navbar-collapse', idx + 1), -1); - }); });