Building Dynamic Menu With Click Handler

I'm having lots of trouble finding the issue in this code... I am trying to build a horizontal menu that expands when clicked, the user can then click the option they want. To clos

Solution 1:

Update: Added sample menu implementation

I realize that stylistically this isn't what you're going for, but conceptually this is the direction I think you'd want to go:

const languages = [
  "English",
  "Spanish",
  "French",
  "Wookie",
  "Klingon"
]

function Menu () {
  // keep track of whether the menu is open
  const [isOpen, setOpen] = React.useState(false);
  
  // in a real app this would notify interested
  // parties (redux dispatch or whatever)
  const [selectedLanguage, setLanguage] = React.useState(languages[0]);	
  
  // convenience function for setting the language
  // and closing the menu
  const onLangSelect = language => {
setLanguage(language);
setOpen(false);
  };
  
  return (
<div>
  <button onClick={() => setOpen(!isOpen)}>{selectedLanguage} v</button>
  <ul className="language-menu">
    {
      // if the menu is open, render the items…
      isOpen && (
        // by iterating over the available languages and emitting an item for each.
        // item.onClick invokes onLangSelect, passing in the selected language
        // we're also flagging the current item with a css class here
        languages.map(lang => (
          <li className={selectedLanguage === lang ? 'selected' : ''} onClick={() => onLangSelect(lang)} key={lang}>
            {lang}
          </li>
        ))			
      )
    }
  </ul>	
</div>
  )
}
  
ReactDOM.render(<Menu />, document.querySelector("#app"))

body {
  background: #20262E;
  padding: 20px;
  font-family: Helvetica;
}

#app {
  background: #fff;
  border-radius: 4px;
  padding: 20px;
  transition: all 0.2s;
}

.language-menu {
  position: absolute;
  background: white;
  font-size: 0.875rem;
  min-width: 150px;
  padding: 0;
  margin: 0;
  list-style: none;
  
}

.language-menu li {
  padding: 1em;
}

.language-menu li:hover {
  background: lightblue;
}

li.selected {
  background: bisque;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.10.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.10.0/umd/react-dom.production.min.js"></script>

<div id="app"></div>

---

You shouldn't be manipulating the DOM directly. Doing so undercuts the entire point of React. Just emit the nodes you need. So instead of this:

// don't do this. there's no need to manually create dom elements in react
[...props.ui.languages].forEach(function (language, i) {
  const el = document.createElement('li')
  el.value = language.languageCode
  el.innerHTML = language.name
  el.className = "item"
  el.addEventListener("click", function(e) {
    e.stopPropagation()
  })
  listContainer.appendChild(el)
})

Just emit the markup with jsx:

// within render method
[...props.ui.languages].map((language) => (
  <li key={language} onclick={...}>{language}</li>
))

---

When you do this:

// this sets onClick to undefined because openMenu doesn't return anything
onClick={openMenu(this)}

You're setting the onclick handler to the return value of openMenu(this), which is undefined since openMenu doesn't return anything.

And again, don't manipulate the DOM. Instead of:

e.parentElement.classList.add('open')

Use setState to track whether the menu is open or not:

this.setState({open: true});

And then render accordingly:

const {open} = this.state;
<div className={open ? 'menu open' : 'menu'}>
 ...
</div>

(There are packages like clsx that can assist with composing classnames; I'm emitting menu open manually here to keep the example simple.)