Dropdown component

dropdown.jsx

/* 
Prompt:
  We have defined a basic dropdown via the Dropdown and DropdownItem components below, with example usage in the ExampleNav component.
  The Dropdown and DropdownItem components have some problems, and also have room for improvements.
  Please fix any obvious bugs you see with the dropdown, and explain your reasoning.
  Please then describe some improvements you would make to the dropdown, and how you would implement them.
  Consider the different contexts in which you might use this dropdown and what changes might be neccessary to make it more flexible.
  
  Follow up question: if we wanted to sync this dropdown selection to the server with app.sync('PATCH', 'user', { dropdown_1_state: {true,false} }) where would this be included
  
  PS: No need to worry about CSS.
 */

class Dropdown extends React.PureComponent {
  // Typo in constructor!
  constructor(props) {
    super(props);
    this.state = {
      isOpen: false
    };
  }

  componentDidMount() {
    // Async stuff here.
  }

  // Explicitly have open and close functions => easier to debug and hard to have logical errors here.
  handleOpen = () => {
    this.setState({ isOpen: true });
  };

  handleClose = () => {
    this.setState({ isOpen: false });
  };

  render() {
    const { isOpen } = this.state;
    const { label } = this.props;
    /* Fix2: Type in aria-expanded */
    return (
      <div className="dropdown">
        <button
          type="button"
          className="dropdown-button"
          id="dropdownButton"
          aria-haspopup="listbox"
          aria-expanded={isOpen ? "true" : "false"}
          onClick={() =>
            this.state.isOpen ? this.handleClose() : this.handleOpen()
          }
        >
          {label}
        </button>

        {isOpen ? (
          <ul
            className={`${isOpen ? "dropdown-open" : ""} dropdown-menu`}
            aria-labelledby={label}
            role="listbox"
          >
            {React.Children.map(this.props.children, (child) => {
              if (child.type === DropdownItem) {
                return React.cloneElement(child, {});
              }
              return child;
            })}
          </ul>
        ) : null}
      </div>
    );
  }
}

class DropdownItem extends React.PureComponent {
  render() {
    const { href } = this.props;
    return (
      <li>
        <a href={href}>{this.props.children}</a>
      </li>
    );
  }
}

class ExampleNav extends React.PureComponent {
  render() {
    return (
      <nav>
        <Dropdown label="More items">
          <DropdownItem href="/page2">Page 2</DropdownItem>
          <DropdownItem href="/page3">Page 3</DropdownItem>
          <DropdownItem href="/page4">Page 4</DropdownItem>
        </Dropdown>
        <Dropdown label="Even more items">
          <DropdownItem href="/page5">Page 5</DropdownItem>
          <DropdownItem href="/page6">Page 6</DropdownItem>
        </Dropdown>
      </nav>
    );
  }
}


/*
Personally, I would not use this compound components pattern to create react components.The context API with this pattern can create some powerful 
extensibility, but they have their drawbacks:
1. Here in the <DropdownItem /> I can easily forget to wrap the <a> inside an <li> => component is not HTML Semantic anymore. I have to be careful and watchful of a future refactor.
2. Especially troublesome with event handlers => down the line if we had to change the <a> to <button> with some event handlers, a change in the markup could lead to these listeners going to the wrong element.

The pattern i do like is hiding this implementation altogether:
<Dropdown items=[{id: 'id1', label: 'label 1'}] /> => This ensure the consumer just passes the list of options and is done => the rendering, updation etc is taken care of inside the component!
*/