Problem
EDIT: Version 2 is now available here
I am just getting to grips with ReactJs using es2015 classes, i have just converted a simple portion of an app i maintain into react code and tried a few things out, this displays a list of customers with a sub-list of the animals that they own.
Am i utilising es2015 features correctly, are there more techniques i could be using to make my code more lean, easy to read and efficient?
"use strict"
import {Input, Glyphicon} from 'react-bootstrap'
export class CustomerView extends React.Component {
constructor(props) {
super(props)
this.state = {
data: props.initialData,
searchText: '',
helpText: 'Type to search for'
}
}
validationState() {
let length = this.state.searchText.length;
let dataLength = this.state.data.length
console.log("DataLength: " + dataLength)
if (dataLength == 0) return 'error'
else if (length > 2) return 'success'
else if (length > 0) return 'warning'
}
handleChange() {
//split the search text by space delimiter
//return customers where the name contains all values in the array
//only search when 3 chars or more
let allText = this.refs.input.getValue()
this.state.searchText = allText
let searchText = allText.toLowerCase().split(' ')
if (allText.length > 2) {
console.log("it changed")
// This could also be done using ReactLink:
// http://facebook.github.io/react/docs/two-way-binding-helpers.html
let filteredCustomers = this.props.initialData.filter(customer =>
searchText.filter(text =>
customer.name.toLowerCase().indexOf(text) >= 0).length
== searchText.length)
this.setState({
data: filteredCustomers,
helpText: 'Found ' + filteredCustomers.length + ' customers.'
})
}
else {
this.setState({
data: this.props.initialData,
helpText: 'Enter three or more characters to search'
})
}
}
render() {
return (
<div className="customerApp">
<h1>Customers</h1>
<Input type="text"
placeholder="Enter text"
label="Customer search"
help={this.state.helpText}
hasFeedback
ref="input"
bsStyle={this.validationState()}
groupClassName="group-class"
labelClassName="label-class"
onChange={this.handleChange.bind(this)} />
<CustomerList customers={this.state.data} />
</div>
);
}
}
module.exports = CustomerView
class CustomerList extends React.Component {
render() {
let customers = this.props.customers;
return (
<div className="customerList">
{
customers.map(function (customer, i) {
return (
<Customer name={customer.name} animals={customer.animals} key={i}></Customer>
);
})
}
</div>
);
}
}
class Customer extends React.Component {
render() {
return (
<TogglePanel title={this.props.name}>
<AnimalList animals={this.props.animals}></AnimalList>
<TogglePanel title="Addresses">
Some stuff about addresses
</TogglePanel>
</TogglePanel>
);
}
}
class TogglePanel extends React.Component {
render() {
return(
<div className="panel panel-default">
<div className="panel-heading">
<h2 className="customerName panel-title">
<TogglePill onClick={this.toggleBody.bind(this)} ref="bodyToggle"/>
{this.props.title}
</h2>
</div>
<div className="panel-body" ref="panelBody" style={{display: "none"}}>
{this.props.children}
</div>
</div>
);
}
toggleBody() {
let body = this.refs.panelBody
let toggle = this.refs.bodyToggle
if (!this.bodyHeight) {
//get the original height of the block
body.style.display = "block"
this.bodyHeight = this.refs.panelBody.clientHeight + "px"
body.style.maxHeight = "0px"
body.style.paddingTop = "0px"
body.style.paddingBottom = "0px"
body.style.overflow = "hidden"
}
if (body.style.maxHeight == "0px") {
this.refs.bodyToggle.open()
body.style.maxHeight = "0px"
body.style.display = "block"
body.style.transition = "all 0.3s ease-in"
window.requestAnimationFrame(function () {
body.style.maxHeight = this.bodyHeight
body.style.paddingTop = "15px"
body.style.paddingBottom = "15px"
}.bind(this));
}
else {
this.refs.bodyToggle.close()
body.transition = "all 0.3s ease-out"
window.requestAnimationFrame(function () {
body.style.maxHeight = "0px"
body.style.paddingTop = "0px"
body.style.paddingBottom = "0px"
});
}
}
}
class TogglePill extends React.Component {
constructor(props) {
super(props)
}
open() {
this.refs.pillIcon.className = "glyphicon glyphicon-triangle-bottom"
}
close() {
this.refs.pillIcon.className = "glyphicon glyphicon-triangle-right"
}
render() {
return(
<span ref="pillIcon"
onClick={this.props.onClick.bind(this)}
className="glyphicon glyphicon-triangle-right"
style={{
fontSize: "0.7em",
marginLeft: "-10px",
marginRight: "10px"
}}>
</span>
);
}
}
class AnimalList extends React.Component {
render() {
let animals = this.props.animals;
return (
<div>
<em>{animals.length} Animal{(animals.length !== 1) ? "s":""}:</em>
<ul className="animalList">
{
animals.map(function (animal, i) {
return (<Animal name={animal.name} key={i}></Animal>);
})
}
</ul>
</div>
);
}
}
class Animal extends React.Component {
render() {
return(
<li className="animal">{this.props.name}</li>
);
}
}
CustomerView accepts initialData in the following format:
{"initialData":[
{
"name":"Mr Joe Bloggs",
"animals":[
{"name":"Alec"},
{"name":"Flo"},
{"name":"Diesel"}
]
},
{
"name":"Mrs. Jane Bloggs",
"animals":[
{"name":"Maddy"},
{"name":"Queenie"},
{"name":"Pluto"}
]
}
]}
Solution
There’s a lot to say about this, so I won’t be able to cover everything in this review.
getInitialState
This is very bad in React:
this.state = {
data: props.initialData,
searchText: '',
helpText: 'Type to search for'
}
Instead, you should create getInitialState method for this component. All this method has to do is just return the initial state in object form. So, it’d look like this:
getInitialState() {
return {
data: props.initialData,
searchText: '',
helpText: 'Type to search for'
}
}
EDIT:
In fact, I was wrong about the state. For ES6 classes, it is okay to set state like that in the constructor. Otherwise, you should be using getInitialState.
Stateless functional components
For every component you have that does not have some sort of state, you should turn that into a functional component – basically, it’s just a plain-old function that takes in an object of props and returns the JSX output; no state involved.
One, very simple example is the Animal component. That could be turned into this:
const Animal = ({ name }) => <li className="animal">{name}</li>
Inline styles
Inline styles are bad:
body.style.display = "block"
this.bodyHeight = this.refs.panelBody.clientHeight + "px"
body.style.maxHeight = "0px"
body.style.paddingTop = "0px"
body.style.paddingBottom = "0px"
body.style.overflow = "hidden"
They make code much harder to maintain. Either way, this is why we have style sheets; they help with organization, too.
You should try to refactor as much of this style setting code as possible into CSS classes that could be toggled through the JavaScript.