Collapse Nav (Broken)

Here's my current code: https://codepen.io/Matt-CopOffMatt/pen/eYXJBpN Currently, I'm having issues with my functionality for collapsing by clicking outside of the menu. The following check:
menuChildren.forEach((child) => {
console.log(e.target, child)
if (e.target === child) {
console.log("Click outside of the menu")
} else {
checkbox.checked = false
menu.classList.remove('visible')
document.removeEventListener("click", checkMenu);
}
});
menuChildren.forEach((child) => {
console.log(e.target, child)
if (e.target === child) {
console.log("Click outside of the menu")
} else {
checkbox.checked = false
menu.classList.remove('visible')
document.removeEventListener("click", checkMenu);
}
});
Should loop through children and determine if the click is a child. If child is clicked, log (not execute the else). However, it seems as though the code is executing directly after calling the click event listener on document (which I assumed shouldn't work like this). What's the most optimal way of fixing this? Also, there's some weird glitch where it stutters when closing. What could be causing that? Thank you
33 Replies
Matt
Mattβ€’9mo ago
I slowed down the animation for visual reference. I also added overflow: hidden; to the Menu. Still running into an issue where the content isn't fully collapsing -Fixed was caused because of default padding. -Might make more sense to transition with position rather than size
clevermissfox
clevermissfoxβ€’9mo ago
This is a bit of a mess. I've forked it and will rework it in a bit if no one else has gotten to it yet.
Matt
Mattβ€’9mo ago
I'm still working on trying to get better with JS. I apologize. I just uploaded my updated version (still can't get the click outside box to work properly). I would typically do this with just regular CSS but it needs to be done with JS
Matt
Mattβ€’9mo ago
Odd, checkbox.checked = false doesn't work, but checkbox.click() does. @hart❀πŸ”₯ Would you possibly give me your thoughts on my updated version? Took me a bit to get it to work
clevermissfox
clevermissfoxβ€’9mo ago
you will need to make it responsive for mobile but looks less messy. Im curious though, if youre toggling classes and using JS anyway, why still use the checkbox? thats mostly used to use css to try to hack this issue without JS. at this point your checkbox is making your JS more verbose than it needs to be.
Matt
Mattβ€’9mo ago
I wasn't sure how to handle button clicks
clevermissfox
clevermissfoxβ€’9mo ago
this is doing the same thing with about 10 lines of JS. and the clickable elements are buttons so they can be interacted with for keyboard users as well. https://codepen.io/Miss-Fox/pen/XWGXREN?editors=0110
Matt
Mattβ€’9mo ago
Ah actually, maybe I could just check if class is present This is missing clicking outside of menu to close
clevermissfox
clevermissfoxβ€’9mo ago
you didnt have that implemented in your code so i didnt include it. This was purely an example to show you that you have all the code you needed, you dont need to use a checkbox as its making it more verbose.
Matt
Mattβ€’9mo ago
Yeah I do. And thank you for the feedback. I'll rework to use a button instead, using input isn't ideal
clevermissfox
clevermissfoxβ€’9mo ago
well then refactor your code to accomplish the same thing but more DRYly.
Matt
Mattβ€’9mo ago
I honestly think I overthank this πŸ˜…
clevermissfox
clevermissfoxβ€’9mo ago
it happens
Want results from more Discord servers?
Add your server