From 800e3a9e93f61b08f2e005f9a155bdf8baf9cf56 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Fri, 29 Jul 2016 15:58:24 +0200 Subject: [PATCH 1/7] Allow other child types inside TabList --- examples/basic/app.js | 2 ++ src/components/TabList.js | 18 ++++++++++++------ src/components/Tabs.js | 14 ++++++++++---- src/helpers/childrenPropType.js | 4 ---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/examples/basic/app.js b/examples/basic/app.js index fe51d3e8da..b781df5c33 100644 --- a/examples/basic/app.js +++ b/examples/basic/app.js @@ -17,6 +17,8 @@ const App = React.createClass({ React Ember Angular + + + diff --git a/src/components/TabList.js b/src/components/TabList.js index f276ee3eb7..490947f87f 100644 --- a/src/components/TabList.js +++ b/src/components/TabList.js @@ -2,12 +2,18 @@ import React, { PropTypes } from 'react'; import cx from 'classnames'; function renderChildren(props) { - return React.Children.map(props.children, (child) => - React.cloneElement(child, { - activeTabClassName: props.activeTabClassName, - disabledTabClassName: props.disabledTabClassName, - }) - ); + return React.Children.map(props.children, (child) => { + let clonedProps = {}; + + if (child.type.displayName === 'Tab') { + clonedProps = { + activeTabClassName: props.activeTabClassName, + disabledTabClassName: props.disabledTabClassName, + }; + } + + return React.cloneElement(child, clonedProps); + }); } module.exports = React.createClass({ diff --git a/src/components/Tabs.js b/src/components/Tabs.js index 7b3707aa0b..959f90c707 100644 --- a/src/components/Tabs.js +++ b/src/components/Tabs.js @@ -207,15 +207,21 @@ module.exports = React.createClass({ const selected = state.selectedIndex === index; const focus = selected && state.focus; - index++; - - return cloneElement(tab, { + let props = { ref, id, panelId, selected, focus, - }); + }; + + if (tab.type.displayName !== 'Tab') { + props = {}; + } + + index++; + + return cloneElement(tab, props); }), }); diff --git a/src/helpers/childrenPropType.js b/src/helpers/childrenPropType.js index 94be6e64fe..f13e8a0282 100644 --- a/src/helpers/childrenPropType.js +++ b/src/helpers/childrenPropType.js @@ -25,10 +25,6 @@ module.exports = function childrenPropTypes(props, propName) { if (c.type === Tab) { tabsCount++; - } else { - error = new Error( - `Expected 'Tab' but found '${c.type.displayName || c.type}'` - ); } }); } else if (child.type.displayName === 'TabPanel') { From af03fd97fc4be84f570dd5ed865a8291a3d4f45d Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Fri, 29 Jul 2016 16:03:04 +0200 Subject: [PATCH 2/7] Updated tests --- src/components/__tests__/Tabs-test.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/components/__tests__/Tabs-test.js b/src/components/__tests__/Tabs-test.js index 7fcb5724f5..b7e5839919 100644 --- a/src/components/__tests__/Tabs-test.js +++ b/src/components/__tests__/Tabs-test.js @@ -201,7 +201,24 @@ describe('react-tabs', () => { expect(result instanceof Error).toBe(true); }); - it('should result with a warning when wrong element is found', () => { + it('should result with warning when tabs/panels are imbalanced ignoring non tab children', () => { + const wrapper = shallow( + + + Foo +
+
+ + + Hello Foo + Hello Bar + + ); + + const result = Tabs.propTypes.children(wrapper.props(), 'children', 'Tabs'); + expect(result instanceof Error).toBe(true); + }); + + it('should not throw a warning when wrong element is found', () => { const wrapper = shallow( @@ -213,7 +230,7 @@ describe('react-tabs', () => { ); const result = Tabs.propTypes.children(wrapper.props(), 'children', 'Tabs'); - expect(result instanceof Error).toBe(true); + expect(result instanceof Error).toBe(false); }); it('should be okay with rendering without any children', () => { From 70c5b3513f8a582c042455ad99bed156d4a71746 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Fri, 29 Jul 2016 16:05:19 +0200 Subject: [PATCH 3/7] Fix lint issue --- src/components/__tests__/Tabs-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/__tests__/Tabs-test.js b/src/components/__tests__/Tabs-test.js index b7e5839919..9a91bb6825 100644 --- a/src/components/__tests__/Tabs-test.js +++ b/src/components/__tests__/Tabs-test.js @@ -201,7 +201,8 @@ describe('react-tabs', () => { expect(result instanceof Error).toBe(true); }); - it('should result with warning when tabs/panels are imbalanced ignoring non tab children', () => { + it(`should result with warning when tabs/panels are imbalanced and + it should ignore non tab children`, () => { const wrapper = shallow( From d76292a01eedc300b417f25cfb87855ec9b527da Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Fri, 29 Jul 2016 16:29:28 +0200 Subject: [PATCH 4/7] Don't clone non Tabs elements --- src/components/TabList.js | 15 +++++++++------ src/components/Tabs.js | 24 +++++++++++------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/components/TabList.js b/src/components/TabList.js index 490947f87f..fad3832de3 100644 --- a/src/components/TabList.js +++ b/src/components/TabList.js @@ -3,15 +3,18 @@ import cx from 'classnames'; function renderChildren(props) { return React.Children.map(props.children, (child) => { - let clonedProps = {}; + // if child is not a tab we don't need to clone it + // since we don't need to add custom props - if (child.type.displayName === 'Tab') { - clonedProps = { - activeTabClassName: props.activeTabClassName, - disabledTabClassName: props.disabledTabClassName, - }; + if (child.type.displayName !== 'Tab') { + return child; } + const clonedProps = { + activeTabClassName: props.activeTabClassName, + disabledTabClassName: props.disabledTabClassName, + }; + return React.cloneElement(child, clonedProps); }); } diff --git a/src/components/Tabs.js b/src/components/Tabs.js index 959f90c707..096f3503f9 100644 --- a/src/components/Tabs.js +++ b/src/components/Tabs.js @@ -207,21 +207,19 @@ module.exports = React.createClass({ const selected = state.selectedIndex === index; const focus = selected && state.focus; - let props = { - ref, - id, - panelId, - selected, - focus, - }; - - if (tab.type.displayName !== 'Tab') { - props = {}; - } - index++; - return cloneElement(tab, props); + if (tab.type.displayName === 'Tab') { + return cloneElement(tab, { + ref, + id, + panelId, + selected, + focus, + }); + } + + return tab; }), }); From f55902034b5e43c94a370f4ed34edb639a448dcf Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Wed, 10 Aug 2016 17:20:27 +0200 Subject: [PATCH 5/7] Test that non tab elements are not cloned --- src/components/__tests__/Tabs-test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/components/__tests__/Tabs-test.js b/src/components/__tests__/Tabs-test.js index 9a91bb6825..0c295856cb 100644 --- a/src/components/__tests__/Tabs-test.js +++ b/src/components/__tests__/Tabs-test.js @@ -185,6 +185,27 @@ describe('react-tabs', () => { expect(wrapper.childAt(2).text()).toBe('Hello Bar'); expect(wrapper.childAt(3).text()).toBe('Hello Baz'); }); + + it('should not clone non tabs element', () => { + class Demo extends React.Component { + render() { + const plus =
+
; + + return ( + + Foo + {plus} + + + Hello Baz + ); + } + } + + const wrapper = mount(); + + expect(wrapper.ref('yolo').text()).toBe('+'); + }); }); describe('validation', () => { From 4c6dbd6e9e15f7a37cc0537f2324dc74fdd80d32 Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Wed, 14 Sep 2016 23:12:54 +0200 Subject: [PATCH 6/7] Don't compare element type by display name --- src/components/TabList.js | 3 ++- src/components/Tabs.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/TabList.js b/src/components/TabList.js index fad3832de3..d0d8f43fd4 100644 --- a/src/components/TabList.js +++ b/src/components/TabList.js @@ -1,12 +1,13 @@ import React, { PropTypes } from 'react'; import cx from 'classnames'; +import Tab from './Tab'; function renderChildren(props) { return React.Children.map(props.children, (child) => { // if child is not a tab we don't need to clone it // since we don't need to add custom props - if (child.type.displayName !== 'Tab') { + if (child.type !== Tab) { return child; } diff --git a/src/components/Tabs.js b/src/components/Tabs.js index 096f3503f9..a230f35db3 100644 --- a/src/components/Tabs.js +++ b/src/components/Tabs.js @@ -4,6 +4,7 @@ import cx from 'classnames'; import jss from 'js-stylesheet'; import uuid from '../helpers/uuid'; import childrenPropType from '../helpers/childrenPropType'; +import Tab from './Tab'; // Determine if a node from event.target is a Tab element function isTabNode(node) { @@ -209,7 +210,7 @@ module.exports = React.createClass({ index++; - if (tab.type.displayName === 'Tab') { + if (tab.type === Tab) { return cloneElement(tab, { ref, id, From b1e454549c5edcdc5d579b857a108ef0a4560d2c Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Wed, 14 Sep 2016 23:14:34 +0200 Subject: [PATCH 7/7] Disable multiple components lint inside Tabs tests --- src/components/__tests__/Tabs-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/__tests__/Tabs-test.js b/src/components/__tests__/Tabs-test.js index 0c295856cb..7094504d78 100644 --- a/src/components/__tests__/Tabs-test.js +++ b/src/components/__tests__/Tabs-test.js @@ -1,3 +1,4 @@ +/* eslint-disable react/no-multi-comp */ /* global jest, describe, it, expect */ import React from 'react'; import { shallow, mount } from 'enzyme';