HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Building a multi-panel display with React and Bootstrap

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
multiwithbootstrapreactbuildingpanelanddisplay

Problem

The exercise is part of a React course in which I'm currently enrolled.

Description:

The way it shall look on desktop-screen:

How it shall look on mobile:

Here's my solution:

HTML



JSX

```
// Manager bundles the single components together.
var React = require('react');
var ReactDOM = require('react-dom');
var PanelWithoutHeading = require('./components/PanelWithoutHeading.jsx');
var PanelCentered = require('./components/PanelCentered.jsx');
var PanelColoredHeading = require('./components/PanelColoredHeading.jsx');
var PanelLarge = require('./components/PanelLarge.jsx');

var largePanelVals = [
{
headline: 15080,
text: 'Shot Views'
},
{
headline: 12000,
text: 'Likes'
},
{
headline: 5100,
text: 'Comments'
}
];

var PanelManager = React.createClass({
render: function() {
return (


















);
}
});

// Render the bundle into HTML.
ReactDOM.render(, document.getElementById('display'));

// -- THE COMPONENTS ----------------------------

// 1. Component
var React = require('react');

var PanelCentered = React.createClass({
render: function() {
var divStyle = {
'marginTop': '10px'
} ;

var panelStyles = {
'background': this.props.backgroundColor,
'textAlign': 'center',
'color': 'white',
'minHeight': '100px'
};

var h3Styles = {
fontSize: '36px'
};

return (



{ this.props.headline }
{ this.props.text }



);
}
});

module.exports = PanelCentered;

// 2. Component
var React = require('react');

var PanelColoredHeading = React.createClass({
render: function() {
var divStyle =

Solution

Move variables out of render

All the variables you are storing are sitting in the render method of the respective component. This means that the variables are constantly created and destroyed.

It would be better to either:

  • Set them as properties of the object you are passing into React.createClass()



  • Write the values inline with the returned elements.



Modify special loop cases separately

In this loop:

var lastElement = this.props.vals.length - 1;

var panels = this.props.vals.map(function(val, i) {
  var leftPadding;
  var rightPadding;
  // First panel (i === 0) gets NO left-padding
  !i ? leftPadding = '15px' : leftPadding = '0';

  i === lastElement ? rightPadding = '15px' : rightPadding = '0';

  return 
});


The special cases are the first and last element: you've written two conditionals and are storing extra data just so you can handle these two separately from the rest.

Rather than that, it would be a lot simpler and a lot cleaner in the loop if you instead modified the elements after the loop:

var panels = this.props.vals.map(function(val, i) {
    return 
});
panels[0].props.paddingLeft = '0';
panels[panels.length - 1].paddingRight = '15px';


Stateless Functional Components

None of your components hold a state. Therefore, it is better practice to write them in the form of a stateless functional component:

function MyComponent(props) {
    return 
}


ES6

Small, side recommendation: learn ES6 and use it with React. React (and libraries like Flux and Redux) work a lot better, cleaner, and easier with ES6, e.g., creating classes is more natural in ES6 since you use the actual class system.

Also, things are just starting to move away from ES5 anyway, so it would be good to get comfortable with ES6.

Code Snippets

var lastElement = this.props.vals.length - 1;

var panels = this.props.vals.map(function(val, i) {
  var leftPadding;
  var rightPadding;
  // First panel (i === 0) gets NO left-padding
  !i ? leftPadding = '15px' : leftPadding = '0';

  i === lastElement ? rightPadding = '15px' : rightPadding = '0';

  return <PanelLargePart paddingLeft={ leftPadding }
                         paddingRight={ rightPadding }
                         headline={ val.headline }
                         text={ val.text } 
                         key={ i }/>
});
var panels = this.props.vals.map(function(val, i) {
    return <PanelLargePart paddingLeft={ leftPadding }
                         paddingRight={ rightPadding }
                         headline={ val.headline }
                         text={ val.text } 
                         key={ i }/>
});
panels[0].props.paddingLeft = '0';
panels[panels.length - 1].paddingRight = '15px';
function MyComponent(props) {
    return <my-tags/>
}

Context

StackExchange Code Review Q#138060, answer score: 5

Revisions (0)

No revisions yet.