/* 

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.

 */
import React, {PureComponent} from 'react';

/*
    **** Possible Improvements ****

    I alwasy try to balance between complexity and flexability 
    
    // Simple version with less flexability 

    Adding a menuItems prop and a onItemClick prop would 
    cover most use cases and give a simple component api.
    I would keep the ability to render children as well
  
    // example 
    <Dropdown
      menuItems =  {[{value : 'value', title :'title'}, ...] }
      onItemClick = { value => setSomeState(value) }
    />  
    
    // More Complex version with better flexability 

    Adding props to control the classes and the container element. 
    would give more styling versality. Having a prop that lets 
    you control when the dropdown is open would give you greater control

    // example 
    <Dropdown
      menuItems =  {[{value : 'value', title :'title'}, ...] }
      onItemClick = { value => setSomeState(value) }
      isOpen = {this.state.isDDOpen}
      onClick = { e => this.openDD(e)}
      openClassNmae = 'dd__open'
      containerEl = <MyCustomDDContainer />
    />  



*/


/*
    **** Server Sync ****
  
    The logic to call the server should be encapsulated
    in a seperate piece of code, such as a thunk or a API service.
    The dropdown component should then call that code when its toggled  
*/
class Dropdown extends PureComponent {

  /* 
      Using Class properties fixes the issues with THIS, and 
      looks cleaner to me 
  */ 
  state = { isOpen: false }

  toggle= () => {
    // I feel that when accessing previous state, using the
    // setState function is more idiomatic
    this.setState(prevState => ({ isOpen: !prevState.isOpen }));
  }

  render() {
    const {isOpen} = this.state;
    const {label} = this.props;

    return (
      <div className="dropdown">
        <button type="button" className="dropdown-button" id="dropdownButton" aria-haspopup="true" aria-expended={isOpen} onClick={this.toggle}>{label}</button>

        <ul className={`${isOpen ? 'dropdown-open' : ''} dropdown-menu`} aria-labelledby="dropdownButton" role="menu">
          { 
            // simple solution to only showing items when isOpen is true
            isOpen && this.props.children  
          }
        </ul>
      </div>
    );
  }
}

class DropdownItem extends PureComponent {
  // Returning children here is a simple solution 
  // and reflects how the component is being used 
  render() {
    return this.props.children
  }
}

class ExampleNav extends PureComponent {
  render() {
    return (
      <nav>
        <a href="/page1">Page 1</a>
        <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>
    );
  }
}