Recursive react.js navigation menu

Question

I'm very new to React.js and have to start converting an entire website at my work. It's fun, but I'm hoping to get some feedback about how I tackled building this navigation component as I don't fully understand best practices when it comes to structuring components as well as proper state and props management.

I have uploaded the full working example to me repo here if you want to clone and run locally: https://github.com/tayloraleach/recursive-react-material-ui-menu

Here are the two components I built that compose the navigation:

The main navigation component that holds all the children

MobileNavigation.jsx

import React from 'react';
import PropTypes from 'prop-types';
import { withStyles } from '@material-ui/core/styles';
import MobileNavigationMenuItem from './MobileNavigationMenuItem';
import classnames from 'classnames';
import List from '@material-ui/core/List';
class MobileNavigation extends React.Component {
 state = {
 currentOpenChildId: null
 };
 handleCurrentlyOpen = (id) => {
 this.setState({
 currentOpenChildId: id
 });
 };
 render() {
 const { classes } = this.props;
 // Loop through the navigation array and create a new component for each,
 // passing the current menuItem and its children as props
 const nodes = this.props.data.navigation.map((item) => {
 return (
 <MobileNavigationMenuItem
 key={item.id}
 node={item}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={this.state.currentOpenChildId}>
 {item.children}
 </MobileNavigationMenuItem>
 );
 });
 return (
 <List disablePadding className={classnames([this.props.styles, classes.root])}>
 {nodes}
 </List>
 );
 }
}
MobileNavigation.propTypes = {
 classes: PropTypes.object.isRequired,
 styles: PropTypes.string,
 data: PropTypes.object.isRequired
};
const styles = (theme) => ({
 root: {
 width: '100%',
 padding: 0,
 boxShadow: 'inset 0 1px 0 0 rgba(255, 255, 255, 0.15)',
 background: "#222"
 },
 link: {
 color: '#fff',
 textDecoration: 'none'
 }
});
export default withStyles(styles)(MobileNavigation);

And each item of the navigation that gets called recursively

MobileNavigationMenuItem.jsx

import React from 'react';
import { ListItem, Collapse, List } from '@material-ui/core';
import ArrowDropDown from '@material-ui/icons/ArrowDropDown';
import ArrowDropUp from '@material-ui/icons/ArrowDropUp';
import { withStyles } from '@material-ui/core/styles';
import classnames from 'classnames';
import PropTypes from 'prop-types';
class MobileNavigationMenuItem extends React.Component {
 state = {
 open: false,
 id: this.props.node.id,
 currentOpenChildId: null
 };
 handleClick = () => {
 if (this.props.currentlyOpen == this.props.node.id) {
 this.setState((state) => ({ open: !state.open }));
 } else {
 this.setState({ open: true }, this.props.passToParent(this.props.node.id));
 }
 };
 handleCurrentlyOpen = (id) => {
 this.setState({
 currentOpenChildId: id
 });
 };
 // These got separated due to having an inner div inside each item to be able to set a max width and maintain styles
 getNestedBackgroundColor(depth) {
 const styles = {
 backgroundColor: 'rgba(255, 255, 255, 0.05)'
 };
 if (depth === 1) {
 styles.backgroundColor = 'rgba(255, 255, 255, 0.1)';
 }
 if (depth === 2) {
 styles.backgroundColor = 'rgba(255, 255, 255, 0.15)';
 }
 return styles;
 }
 getNestedPadding(depth) {
 const styles = {
 paddingLeft: 0
 };
 if (depth === 1) {
 styles.paddingLeft = 15;
 }
 if (depth === 2) {
 styles.paddingLeft = 30;
 }
 return styles;
 }
 render() {
 const { classes } = this.props;
 let childnodes = null;
 // The MobileNavigationMenuItem component calls itself if there are children
 // Need to pass classes as a prop or it falls out of scope
 if (this.props.children) {
 childnodes = this.props.children.map((childnode) => {
 return (
 <MobileNavigationMenuItem
 key={childnode.id}
 node={childnode}
 classes={classes}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={this.state.currentOpenChildId}>
 {childnode.children}
 </MobileNavigationMenuItem>
 );
 });
 }
 // Return a ListItem element
 // Display children if there are any
 return (
 <React.Fragment>
 <ListItem
 onClick={this.handleClick}
 className={classes.item}
 style={this.getNestedBackgroundColor(this.props.node.depth)}>
 <div className={classes.wrapper}>
 <a
 href=""
 style={this.getNestedPadding(this.props.node.depth)}
 className={classnames([classes.link, !childnodes.length && classes.goFullWidth])}>
 {this.props.node.title}
 </a>
 {childnodes.length > 0 &&
 (this.props.currentlyOpen == this.props.node.id && this.state.open ? (
 <ArrowDropUp />
 ) : (
 <ArrowDropDown />
 ))}
 </div>
 </ListItem>
 {childnodes.length > 0 && (
 <Collapse
 in={this.props.currentlyOpen == this.props.node.id && this.state.open}
 timeout="auto"
 unmountOnExit>
 <List disablePadding>{childnodes}</List>
 </Collapse>
 )}
 </React.Fragment>
 );
 }
}
MobileNavigationMenuItem.propTypes = {
 classes: PropTypes.object.isRequired,
 node: PropTypes.object.isRequired,
 children: PropTypes.array.isRequired,
 passToParent: PropTypes.func.isRequired,
 currentlyOpen: PropTypes.string
};
const styles = (theme) => ({
 link: {
 color: '#fff',
 textDecoration: 'none'
 },
 goFullWidth: {
 width: '100%'
 },
 item: {
 minHeight: 48,
 color: '#fff',
 backgroundColor: 'rgba(255, 255, 255, 0.05)',
 padding: '12px 15px',
 boxShadow: 'inset 0 -1px 0 0 rgba(255, 255, 255, 0.15)',
 '& svg': {
 marginLeft: 'auto'
 }
 },
 wrapper: {
 width: '100%',
 display: 'flex',
 alignItems: 'center',
 maxWidth: '440px', // any value here
 margin: 'auto',
 [theme.breakpoints.down('sm')]: {
 maxWidth: '100%'
 },
 }
});
export default withStyles(styles)(MobileNavigationMenuItem);

I'll admit there is some code clean up I could do in regards to styling nested elements, but overall it works really well and I'm pretty proud of it.

The questions I have stemmed from how I'm closing and opening the children. Each menu item has an open state and acts as a 'parent' of any direct children. When you click an item, it passes the state up and if it the id matches it opens (closing all others).

Each item calls itself if it has children and repeats recursively.

I would love to get some insight on any improvements I can make or if this is a good or bad solution to the problem.

• user: Hi, could you please share the screen shot this?.. because i need to navigate the single page with different parameter from nested array in react-native. if you share the screenshot it more helpful for me.

Answer 1

TL;DR Reworked code : (I used a snippet so I could hide it)

class MobileNavigation extends React.Component {
 state = {
 currentOpenChildId: null
 };
 handleCurrentlyOpen = currentOpenChildId => {
 this.setState({ currentOpenChildId });
 };
 render() {
 const { classes, data: { navigation }, styles } = this.props;
 return (
 <List disablePadding className={classnames([styles, classes.root])}>
 {navigation.map(item => (
 <MobileNavigationMenuItem
 key={item.id}
 node={item}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={this.state.currentOpenChildId}>
 {item.children}
 </MobileNavigationMenuItem>
 ))}
 </List>
 );
 }
}
class MobileNavigationMenuItem extends React.Component {
 state = {
 open: false,
 id: this.props.node.id,
 currentOpenChildId: null
 };
 handleClick = () => {
 const { currentlyOpen, node, passToParent }
 if (currentlyOpen == node.id) {
 this.setState(state => ({ open: !state.open }));
 } else {
 this.setState({ open: true }, passToParent(node.id));
 }
 };
 handleCurrentlyOpen = currentOpenChildId => {
 this.setState({ currentOpenChildId });
 };
 getNestedBackgroundColor = depth => (
 {
 1: 'rgba(255, 255, 255, 0.1)',
 2: 'rgba(255, 255, 255, 0.15)'
 }[depth] || 'rgba(255, 255, 255, 0.05)'
 )
 getNestedPadding = depth => (
 {
 1: 15,
 2: 30
 }[depth] || 0
 )
 render() {
 const { classes, currentlyOpen, node, children } = this.props;
 const { currentOpenChildId, open } = this.state
 
 return (
 <React.Fragment>
 <ListItem
 onClick={this.handleClick}
 className={classes.item}
 style={this.getNestedBackgroundColor(node.depth)}>
 <div className={classes.wrapper}>
 <a
 href=""
 style={this.getNestedPadding(node.depth)}
 className={classnames([classes.link, !childnodes.length && classes.goFullWidth])}>
 {node.title}
 </a>
 {children && currentlyOpen == node.id && open ? 
 <ArrowDropUp />
 : 
 <ArrowDropDown />
 }
 </div>
 </ListItem>
 {children && (
 <Collapse
 in={currentlyOpen == node.id && open}
 timeout="auto"
 unmountOnExit>
 <List disablePadding>
 {children.map(childnode => (
 <MobileNavigationMenuItem
 key={childnode.id}
 node={childnode}
 classes={classes}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={currentOpenChildId}>
 {childnode.children}
 </MobileNavigationMenuItem>
 ))}
 </List>
 </Collapse>
 )}
 </React.Fragment>
 );
 }
}

---

Reducing your getXXX functions

3 of your functions share the same layout :

getNestedBackgroundColor(depth) {
 const styles = {
 backgroundColor: 'rgba(255, 255, 255, 0.05)'
 };
 if (depth === 1) {
 styles.backgroundColor = 'rgba(255, 255, 255, 0.1)';
 }
 if (depth === 2) {
 styles.backgroundColor = 'rgba(255, 255, 255, 0.15)';
 }
 return styles;
}

Using JSON objects, you could map each result to the desired depth number :

{
 1: 'rgba(255, 255, 255, 0.1)',
 2: 'rgba(255, 255, 255, 0.15)'
}

Now, just add brackets to extract the correct output, and return the default one if nothing was found using the || operator :

{
 1: 'rgba(255, 255, 255, 0.1)',
 2: 'rgba(255, 255, 255, 0.15)'
}[depth] || 'rgba(255, 255, 255, 0.05)'

The getNestedPadding function :

getNestedPadding = depth => (
 {
 1: 15,
 2: 30
 }[depth] || 0
)

Short syntax : getNestedPadding = depth => ({ 1: 15, 2: 30 }[depth] || 0)

Deconstructing

I added a lot of state and props deconstruction throughout your code :

const { classes, currentlyOpen, node, children } = this.props;
const { currentOpenChildId, open } = this.state

This allows you to stop repeating this.state.XXX later on and make your code more readable.

Conditional rendering

You are already using the && operator with some parameters but are not using it with the map function, your mapped arrays can also be conditionally rendered in your JSX :

return (
 <List disablePadding className={classnames([styles, classes.root])}>
 {navigation.map(item => ( //Short arrow function syntax
 <MobileNavigationMenuItem
 key={item.id}
 node={item}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={this.state.currentOpenChildId}>
 {item.children}
 </MobileNavigationMenuItem>
 ))}
 </List>
);

Also, putting single JSX component in a condition does not require using parenthesis :

{children && currentlyOpen == node.id && open ? 
 <ArrowDropUp />
 : 
 <ArrowDropDown />
}

And the variable children can be used instead of childnodes.length > 0 now that your children are conditionally rendered :

<List disablePadding>
 {children.map(childnode => (
 <MobileNavigationMenuItem
 key={childnode.id}
 node={childnode}
 classes={classes}
 passToParent={this.handleCurrentlyOpen}
 currentlyOpen={currentOpenChildId}>
 {childnode.children}
 </MobileNavigationMenuItem>
 ))}
</List>

• Taylor A. Leach: Thanks for the reply. Was more interested in feedback surrounding the higher level concept of passing the props and state around. The style functions can be written in numerous ways so wasn't really focusing on them. Wasn't looking to make the code more concise really either but the destructuring, syntax and conditional rendering are things I've noted.