Coding a hamburger menu with JS, CSS & HTML

Question

I've created a menu that allows you to display the list when clicking on the menu icon. I wonder whether this can be improved somehow - can anyone help improve this?

Also, initially I created it with the event passed into the script to prevent it from scrolling to the top when clicking on the link. I then changed it to an ES6 function - does it not need the event passed in and calling e.preventDefault()?

Thank you. The code is as follows:

 const cont = document.querySelector('.cont');
 const hamburger = document.querySelector('#hamburger');
 
 // const toggleMenu = (e) => {
 // e.preventDefault();
 // cont.classList.toggle('open');
 // }
 const toggleDisplay = () => cont.classList.toggle('open');
 
 hamburger.addEventListener('click', toggleDisplay);

.cont {
 position: relative;
}
a {
 font-size: 3em;
}
a:hover {
 cursor: pointer;
}
a::after {
 content: '002円b';
}
.open a::after {
 content: '00円d7';
 color: red;
}
.open ul {
 display: flex;
}
ul {
 all: unset;
 display: none;
 position: absolute;
 background: #0ff;
 padding: .5em;
 min-width: 100px;
 top: 100%;
 left: 0;
 flex-direction: column;
 text-align: center;
}
li {
 all: unset;
 width: 100%;
}
li:not(:first-child) {
 margin-top: .5em;
 padding-top: .5em;
 border-top: 1px solid #ccc;
}

<div class='cont'>
 <a id='hamburger'></a>
 <ul>
 <li>First</li>
 <li>Second</li>
 <li>Third</li>
 </ul>
</div>

Answer 1

Here are a few suggestions:

• '.cont' is a bit vague - I suppose it means container but try to avoid meaningless abbreviations
• Be careful with all: unset. It might have unexpected consequences cross-browser.
• I would stick to rem for margin/paddings and only use em for fontsizes. There is a lot of articles explaining the difference if you perform a simple google search
• You could make it behave a little more smoothly if you had a CSS transition
• Instead of a wrapping div this could potentially be a nav element if it is part of some navigation. Always try to use semantically correct tags
• I don't think you want an a tag here as you are not linking to anything. Maybe a button would be better?

As for your question both of your functions are ES6 functions (fat-arrow functions).

In the example you commented out you are using explicit return while you are using implicit return in the latter.

• user8758206: Thanks for your feedback. A few questions: 1. What side-effects could all: unset have? 2. I read that the opposite is true, and that em is perfect for padding/margin as it's based on the inherited font size, and rem is ok for font sizes - research seemed subjective but generally agreeable on em for margins/padding 3. What's wrong with the a tag? It's good for accessibility as it's selectable when pressing tab (although so are buttons) 4. True, but does it actually need e.preventDefault()?
• J.Kirk.: 1. You can check compatibility on "caniuse.com".. It might also impact accessibility on some fields. In your case I guess you are really just trying to reset the list-style-type. 2. em is relative to the nearest parent while rem is to the root font-size. That's why padding/margins with em could lead to more inconsistent designs imo. Not sure where you found that consensus :). 3. It's semantically incorrect. You should use a tags for links. You can get the same accessibility with a button. 4. No there is no default action to prevent if you don't have any href value.
• radarbob: Use tags that best convey intent - ditto. I'd have upvoted even if that was the only nugget in this answer.