3
\$\begingroup\$

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
\$\endgroup\$
0
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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