Reactjs Tab Component

Question

I made a simple bare bones Tab component, i'm a beginner when it comes to ReactJS so any advice regarding the codes functionality is greatly appreciated.

Tab.js Component

import React from 'react';
class Tabs extends React.Component {
 constructor() {
 super();
 this.state = {
 activeIndex : 0
 }
 }
 handleOnClick(key, event) {
 event.preventDefault();
 this.setState({
 activeIndex : key
 });
 }
 renderNavItem(key) {
 let tab = this.props.children[key];
 return (
 <li key={ key } className={ this.state.activeIndex == key ? 'active' : ''}>
 <a href="#" onClick={ this.handleOnClick.bind(this, key) }>{ tab.props.title }</a>
 </li>
 );
 }
 render() {
 let index = 0;
 let active = this.state.activeIndex;
 let tabs = React.Children.map(this.props.children, function (child) {
 return React.cloneElement(child, {
 active : child.props.active === true ? true : (active == index++)
 });
 });
 return (
 <div className={ this.props.className }>
 <ul className="tabs-nav">
 { Object.keys(this.props.children).map(this.renderNavItem.bind(this)) }
 </ul>
 <div className="tabs-content">
 { tabs }
 </div>
 </div>
 )
 }
}
class Tab extends React.Component {
 render() {
 return (
 <div className={ "tab-panel" + (this.props.active ? ' active' : '') }>
 { this.props.children }
 </div>
 )
 }
}
Tab.defaultProps = { 
 active : false 
};
export default {
 Tabs,
 Tab
};

Usage

import React from 'react';
import {Tabs, Tab} from './Tabs';
class App extends React.Component {
 render() {
 return ( 
 <Tabs className="tabs-wrapper">
 <Tab active="true" title="Tab One">Tab One content</Tab>
 <Tab title="Tab Two">
 <div>Tab Two Content</div>
 </Tab>
 </Tabs> 
 );
 }
}
React.render(
 <App/>,
 document.getElementById('react_example')
);

Example

http://jsbin.com/hajokofavu/edit?js,output

Answer 1

As there's not all that much code here to review, I've reviewed some style points:

You have some strange use of whitespace throughout your code:

handleOnClick(key, event) {
 event.preventDefault();
 this.setState({
 activeIndex : key
 // ^-- whitespace shouldn't be before a property colon
 });
}

and not necessarily that it's wrong, but there's a lot of empty whitespace lines that make your program look a lot beefier than it should.

child.props.active === true ? true : (active == index++)

You don't need to compare properties to booleans as simply specifying the variable without comparison performs a boolean comparison:

var thing = true;
console.log(thing === true ? 1 : 2); // identical
console.log(thing ? 1 : 2); // identical

You've also got some inconsistency in your use of semicolons:

constructor() {
 super();
 this.state = {
 activeIndex : 0
 }
}

• Jeemusu: Thanks for the style advice. I'm not too well versed on the style guidelines for modern JavaScript, so it was quite helpful. Some of the whitespace I like to have for readability, it all gets compiled in the end so no need to worry about the code being beefy.

Answer 2

Every thing looks good except one performance issue in the code. You have already activeIndex, so no need to traverse the whole map again to render tab content.

Here is the modified Tabs class as below:

class Tabs extends React.Component {
constructor() {
 super();
 this.state = {
 activeIndex : 0
 }
}
handleOnClick(key, event) {
 event.preventDefault();
 this.setState({
 activeIndex : key
 });
}
renderNavItem(key) {
 let tab = this.props.children[key];
 return (
 <li key={ key } className={ this.state.activeIndex == key ? 'active' : ''}>
 <a href="#" onClick={ this.handleOnClick.bind(this, key) }>{ tab.props.title }</a>
 </li>
 );
}
render() {
 let active = this.state.activeIndex;
 let tabs = this.props.children[active];
 return (
 <div className={ this.props.className }>
 <ul className="tabs-nav">
 { Object.keys(this.props.children).map(this.renderNavItem.bind(this)) }
 </ul>
 <div className="tabs-content">
 { tabs.props.children }
 </div>
 </div>
 )
 }
 }