From 976112331b4b7a81f66d26a0d147e3bfc2a58877 Mon Sep 17 00:00:00 2001 From: Don Denton Date: Fri, 15 Apr 2016 13:13:06 -0500 Subject: [PATCH 1/2] Fix nested tabs issue #54 This builds off of @msliberty's commit at https://github.com/msliberty/react-tabs/commit/c4b4eafec51c40874f70f6aeb8d8f52d62068655 This one, however, doesn't assume that "that the Tab node will always be the grandchild of the Tabs component node". --- lib/components/Tabs.js | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/components/Tabs.js b/lib/components/Tabs.js index ad99d53fbe..26ce2262ff 100644 --- a/lib/components/Tabs.js +++ b/lib/components/Tabs.js @@ -5,11 +5,6 @@ import jss from 'js-stylesheet'; import uuid from '../helpers/uuid'; import childrenPropType from '../helpers/childrenPropType'; -// Determine if a node from event.target is a Tab element -function isTabNode(node) { - return node.nodeName === 'LI' && node.getAttribute('role') === 'tab'; -} - // Determine if a tab node is disabled function isTabDisabled(node) { return node.getAttribute('aria-disabled') === 'true'; @@ -70,7 +65,7 @@ module.exports = React.createClass({ handleClick(e) { let node = e.target; do { - if (isTabNode(node)) { + if (this.isTabNode(node)) { if (isTabDisabled(node)) { return; } @@ -83,7 +78,7 @@ module.exports = React.createClass({ }, handleKeyDown(e) { - if (isTabNode(e.target)) { + if (this.isTabNode(e.target)) { let index = this.state.selectedIndex; let preventDefault = false; @@ -305,12 +300,33 @@ module.exports = React.createClass({ )} onClick={this.handleClick} onKeyDown={this.handleKeyDown} + role="tabs" > {this.getChildren()} ); }, + // Determine if a node from event.target is a Tab element for the current Tabs container. + isTabNode(node) { + // return immediately off this simple check. + if ((node.nodeName === 'LI' && node.getAttribute('role') === 'tab') === false) { + return false; + } + + let nodeAncestor = node.parentElement; + let lastAncestor = node; + do { + if (nodeAncestor === findDOMNode(this)) { + return true; + } + lastAncestor = nodeAncestor; + nodeAncestor = nodeAncestor.parentElement; + } while (lastAncestor.getAttribute('role') !== 'tabs'); + + return false; + }, + // This is an anti-pattern, so sue me copyPropsToState(props) { let selectedIndex = props.selectedIndex; From dae2d73703c18d24686175346975764faa78afa7 Mon Sep 17 00:00:00 2001 From: Don Denton Date: Fri, 15 Apr 2016 13:31:21 -0500 Subject: [PATCH 2/2] Better comments on Tabs.js --- lib/components/Tabs.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/components/Tabs.js b/lib/components/Tabs.js index 26ce2262ff..b87d0495ad 100644 --- a/lib/components/Tabs.js +++ b/lib/components/Tabs.js @@ -308,12 +308,15 @@ module.exports = React.createClass({ }, // Determine if a node from event.target is a Tab element for the current Tabs container. + // If the clicked element is not a Tab, it returns false. + // If it finds another Tabs container between the Tab and `this`, it returns false. isTabNode(node) { - // return immediately off this simple check. + // return immediately if the clicked element is not a Tab. if ((node.nodeName === 'LI' && node.getAttribute('role') === 'tab') === false) { return false; } + // Check if the first occurrence of a Tabs container is `this` one. let nodeAncestor = node.parentElement; let lastAncestor = node; do {