5
\$\begingroup\$

Last night I had to build dropdown for the menu on the website that I'm working on. As I'm still React newbie I first looked if there is some package for what I'm trying to achieve but there wasn't, so I had to try to build it on my own.

So the menu has its menu items, whichever is hovered it should show its content and so on. The one thing I'm concered about is that I used too much code for the dropdown and I'm sure this can be built with less code. So I only want you to tell me what should be changed in a code and which approach isn't good from my side.

Object:

const options = [
 {
 index: '1',
 label: 'Title 1',
 links: [
 'demo link',
 'demo link',
 'demo link',
 'demo link',
 ],
 image: 'imageURL',
 },
 {
 index: '2',
 label: 'Title 2',
 links: [
 'demo link',
 'demo link',
 'demo link',
 'demo link',
 ],
 image: 'imageURL',
 },
 {
 index: '3',
 label: 'Title 3',
 image: 'imageURL',
 },
];

Code:

import React, { Component } from 'react';
import classNames from 'classnames';
import css from './demo.css';
const list = options => {
 return options.map(option => ({
 index: option.index,
 label: option.label,
 links: option.links,
 }));
};
class SecondaryNav extends Component {
 constructor(props) {
 super(props);
 this.onMouseEnter = this.onMouseEnter.bind(this);
 this.onMouseLeave = this.onMouseLeave.bind(this);
 this.state = {
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null,
 };
 }
 onMouseEnter(index, label, links) {
 if (links != null) {
 this.setState({
 hideDropdown: false,
 dropdownIndex: index,
 activeLabel: label,
 columns: links.length,
 });
 } else {
 this.setState({
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null,
 });
 }
 }
 onMouseLeave() {
 this.setState({
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null,
 });
 }
 render() {
 // Form object array and return navigation items
 const items = list(options);
 const navItems = items.map(item => {
 const index = item.index;
 const label = item.label;
 const links = item.links;
 const isActive = item.index === this.state.dropdownIndex ? css.active : null;
 return (
 <li
 className={isActive}
 onMouseOver={() => this.onMouseEnter(index, label, links)}
 key={item.index}
 >
 {item.label}
 </li>
 );
 });
 // Form array of navigation items that have links
 const selected = options
 .filter(option => option.index === this.state.dropdownIndex)
 .map(selected => selected.links);
 const dropdownLinks = selected[0];
 // Define active content in dropdown
 const activeContent =
 this.state.dropdownIndex != null &&
 dropdownLinks.map((label, key) => <li key={key}>{label}</li>);
 // Define list label for the first dropdown
 const listLabel =
 this.state.dropdownIndex === '1' ? classNames(css.dropdown, css.listLabel) : css.dropdown;
 // Put content in two rows
 const columns = this.state.columns >= 8 ? css.twoColumns : null;
 // Define dropdown image
 const dropdownImage = options
 .filter(image => image.index === this.state.dropdownIndex)
 .map((single, key) => {
 return <img src={single.image} key={key} alt={this.state.activeLabel} />;
 });
 // Form dropdown container
 const dropdown = !this.state.hideDropdown ? (
 <div className={css.dropdownContainer}>
 <h2 className={css.dropdownLabel}>{this.state.activeLabel}</h2>
 <div className={css.dropdownContent}>
 <ul className={classNames(listLabel, columns)}>{activeContent}</ul>
 <div className={css.dropdownImage}>{dropdownImage}</div>
 </div>
 </div>
 ) : null;
 return (
 <div className={css.secondaryNav} onMouseLeave={this.onMouseLeave}>
 <div className={css.navContainer}>
 <ul className={css.secondNav}>{navItems}</ul>
 </div>
 {dropdown}
 </div>
 );
 }
}
export default SecondaryNav;

So everything works as I wanted and as I said I'm just concered that I haven't used good aproaches as I'm still a newbie. So any suggestion will mean a lot.

πάντα ῥεῖ
5,1524 gold badges23 silver badges32 bronze badges
asked May 25, 2020 at 18:01
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Your code is not too bad for one claiming to be a react newbie. I see only a few minor things I'd suggest cleaning or tightening up.

  1. Don't Repeat Yourself (DRY Principle) with regards to object property accesses
  2. Consistent use of ===

Using object destructing and object shorthand property definitions, and directly return the map result.

const list = (options) =>
 options.map(({ index, label, links }) => ({
 index,
 label,
 links
 }));

Convert onMouseEnter and onMouseLeave to arrow functions. This allows you to drop the constructor as they will have this of the react class bound auto-magically. State can be declared a class property.

Explicitly check links !== null

class SecondaryNav extends Component {
 state = {
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null,
 };
 onMouseEnter = (index, label, links) => {
 this.setState(
 links !== null
 ? {
 hideDropdown: false,
 dropdownIndex: index,
 activeLabel: label,
 columns: links.length
 }
 : {
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null
 }
 );
 };
 onMouseLeave = () => {
 this.setState({
 hideDropdown: true,
 dropdownIndex: null,
 activeLabel: null,
 columns: null,
 });
 };

Destructure state values and object properties. Consistent use of ===/!==. Move the conditional render of the dropdown container and use logical and (&&) as react will ignore the false value if !hideDropdown evaluates to false.

render() {
 const { activeLabel, columns, dropdownIndex, hideDropdown } = this.state;
 // Form object array and return navigation items
 const items = list(options);
 const navItems = items.map(({ index, label, links }) => {
 const isActive = index === dropdownIndex ? css.active : null;
 return (
 <li
 className={isActive}
 onMouseOver={() => this.onMouseEnter(index, label, links)}
 key={index}
 >
 {label}
 </li>
 );
 });
 // Form array of navigation items that have links
 const selected = options
 .filter((option) => option.index === dropdownIndex)
 .map((selected) => selected.links);
 const dropdownLinks = selected[0];
 // Define active content in dropdown
 const activeContent =
 dropdownIndex !== null &&
 dropdownLinks.map((label, key) => <li key={key}>{label}</li>);
 // Define list label for the first dropdown
 const listLabel =
 dropdownIndex === "1"
 ? classNames(css.dropdown, css.listLabel)
 : css.dropdown;
 // Put content in two rows
 const columnsStyle = columns >= 8 ? css.twoColumns : null;
 // Define dropdown image
 const dropdownImage = options
 .filter(({ index }) => index === dropdownIndex)
 .map((single, key) => (
 <img src={single.image} key={key} alt={activeLabel} />
 ));
 return (
 <div className={css.secondaryNav} onMouseLeave={this.onMouseLeave}>
 <div className={css.navContainer}>
 <ul className={css.secondNav}>{navItems}</ul>
 </div>
 {!hideDropdown && (
 <div className={css.dropdownContainer}>
 <h2 className={css.dropdownLabel}>{activeLabel}</h2>
 <div className={css.dropdownContent}>
 <ul className={classNames(listLabel, columnsStyle)}>
 {activeContent}
 </ul>
 <div className={css.dropdownImage}>{dropdownImage}</div>
 </div>
 </div>
 )}
 </div>
 );
}
answered Aug 8, 2020 at 7:05
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.