React.js Nested Nav Bar

Question

This was written for a coding challenge for a company I recently starting working for. I'm looking for any suggestions on how to clean up the code, as well as any issues anyone thinks may occur as it is written now. Fairly new to React.js, always looking to improve.

Currently the data is passed through the app component where I render the Nav Container (functional stateless component). The Nav container creates the classNames and passes data + classNames to the NestedNav that I've put below.

It started to feel a little sloppy while mapping twice below the render, not sure if that's a problem?

The data object example shows how I modeled my data. This example has a parent with children that also have children.

• The parentClass is for the outer parent.
• The nestedParentClass is for the children of the outer parent.
• The childClass is reserved for any children of the nestedParent.

export const data = [ { id: 1, icon: <FontAwesomeIcon className='i' icon={faHeartbeat} />, title: 'SAFETY', children: [ {title: 'Reporting', children: [ {id: 11, title: 'I-21 Injury Reporting', url: '/safety/report'}, {id: 12, title: 'ASAP Reporting', url: '/safety/asap-report'}, {id: 13, title: 'General ASAP Information', url: '/safety/general'}, {id: 14, title: 'Flight Attendant Incident Report', url: '/safety/flight-attendant-incident'}, ]}, {title: 'Agriculture and Customs', children: [ {id: 15, title: 'Product Safety Data Search', url: '/safety/data-search'}]}, {id: 16, title: 'Known Crewmember', url: '/safety/known-cremember'}, {id: 17, title: 'Product Safety Data Search', url: '/safety/data-search'}, ], }, ] 

class NestedNav extends Component { constructor(props) { super(props) this.state = { selected: '', nestSelect: '', children: [], active: '', } } handleClick = (title) => { this.setState({ selected: '', nestSelect: '', children: [], active: '', }) } render() { const {data, parentClass, nestedParentClass} = this.props const renderedChildElements = this.state.children const active = this.state.active === 'true' ? 'true' : '' const mappedChildren = (child, title) => { let childElement if(child) { childElement = child.map((child, i) => <li key={i} id={child.id} className={nestedParentClass + (this.state.nestSelect === child.title ? 'nest-selected' : '')} url={child.url}><Link to={'/'}>{child.title}</Link> {child.children ? <FontAwesomeIcon onClick={() => mappedChildren(child.children, this.state.select)} className='i button' icon={faArrowCircleDown} /> : null} </li>) this.setState({ selected: title, children: childElement, active: 'true', }) } return '' } const navListItems = data.map((collection, i) => <li key={i} id={collection.id} className={parentClass + (this.state.selected === collection.title ? 'selected' : '')} url={collection.url}><i onClick={this.handleClick}>{collection.icon}</i><Link to={'/'}><span>{collection.title}</span></Link> {collection.children ? <FontAwesomeIcon onClick={() => mappedChildren(collection.children, collection.title)} className='i button' icon={faArrowCircleRight} /> : null} </li>) return ( <div className={'nested-nav'}> <div className={'container-two-' + active}> <h2>{this.state.selected}</h2> {this.state.children} </div> <div className='container-one'>{navListItems}</div> </div> ) } } export default NestedNav 

Answer 1

it's clear and readable but i would like to point out some things :

Avoid using functions inside Render()

A function in the render method will be created each render which is a slight performance hit. t's also messy if you put them in the render, which is a much > bigger reason

Use the child.id instead of key as index when you map through an array in render() because Index as a key is an anti-pattern.

Keep your code as DRY as possible, the initialState can be an object outside the class instead of writing it twice or more, especially if it's a big object.

Optional : use Destructuring assignment to indicate which keys you will use in the function.

const initialState = { selected: '', nestSelect: '', children: [], active: '', } class NestedNav extends Component { constructor(props) { super(props) this.state = initialState } handleClick = title => { this.setState({ ...initialState }) } mappedChildren = (child, selectedTitle) => { const { nestedParentClass } = this.props let childElement if (child) { childElement = child.map(({ id, title, children, url }) => <li key={id} id={id} className={nestedParentClass + (this.state.nestSelect === title ? 'nest-selected' : '')} url={url}><Link to={'/'}>{title}</Link> {children ? <FontAwesomeIcon onClick={() => this.mappedChildren(children, this.state.select)} className='i button' icon={faArrowCircleDown} /> : null} </li>) this.setState({ selected: selectedTitle, children: childElement, active: 'true', }) } return '' } navListItems = data => data.map(({ id, url, children, title, icon }) => { const { parentClass } = this.props return (<li key={id} id={id} className={parentClass + (this.state.selected === title ? 'selected' : '')} url={url}><i onClick={this.handleClick}>{icon}</i><Link to={'/'}><span>{title}</span></Link> {children ? <FontAwesomeIcon onClick={() => this.mappedChildren(children, title)} className='i button' icon={faArrowCircleRight} /> : null} </li>) }); render() { const { data } = this.props const active = this.state.active === 'true' ? 'true' : '' return ( <div className={'nested-nav'}> <div className={'container-two-' + active}> <h2>{this.state.selected}</h2> {this.state.children} </div> <div className='container-one'>{this.navListItems(data)}</div> </div> ) } } export default NestedNav