Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

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

share|improve this question

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
    }
}
share|improve this answer
    
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. – Jeemusu Jun 22 '16 at 6:30

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.