You need to get rid of the display: none
and let css take care of the transitions more.
I set the .np-collapsible to this in order to let the element always exist:
.np-collapsible:not(.np-expanded) {
height: 0;
overflow: hidden;
}
I set the transition class to not make any changes that would start up a transition (it only includes the transition property).
Then in the JS, the transition is done similarly to what you had originally, but the main difference is that I use menu.scrollHeight
to get the height of the menu in order to avoid having extra transitions to get the height normally.
I also added the ability to contract the menu to the function. In case of contracting the menu, you have to remove np-expanded
class before the transition due to the :not(.np-expanded)
selector earlier stopping the overflow: none.
// Retrieve the height of the menu
var targetHeight = menu.scrollHeight + 'px';
if (menu.classList.contains('np-expanded')) {
// It's already expanded, time to contract.
targetHeight = 0;
menu.classList.remove('np-expanded');
button.setAttribute('aria-expanded', false);
}
// Enable transition
menu.classList.add('np-transitioning');
menu.addEventListener('transitionend', function(event) {
// Disable transition
menu.classList.remove('np-transitioning');
// Indicate that the menu is now expanded
if (targetHeight) {
menu.classList.add('np-expanded');
button.setAttribute('aria-expanded', true);
}
}, {
once: true
});
// Set the height to execute the transition
menu.style.height = targetHeight;
Here's a working example:
var button = document.querySelector('.np-trigger');
var menu = document.querySelector(button.dataset.target);
button.addEventListener('click', function(event) {
expand();
}, false);
function expand() {
if (isTransitioning()) {
// Don't do anything during a transition
return;
}
// Retrieve the height of the menu
var targetHeight = menu.scrollHeight + 'px';
if (menu.classList.contains('np-expanded')) {
// It's already expanded, time to contract.
targetHeight = 0;
menu.classList.remove('np-expanded');
button.setAttribute('aria-expanded', false);
}
// Enable transition
menu.classList.add('np-transitioning');
menu.addEventListener('transitionend', function(event) {
// Disable transition
menu.classList.remove('np-transitioning');
// Indicate that the menu is now expanded
if (targetHeight) {
menu.classList.add('np-expanded');
button.setAttribute('aria-expanded', true);
}
}, {
once: true
});
// Set the height to execute the transition
menu.style.height = targetHeight;
}
function isTransitioning() {
if (menu.classList.contains('np-transitioning')) {
return true;
}
return false;
}
.np-collapsible:not(.np-expanded) {
height: 0;
overflow: hidden;
}
.np-transitioning {
transition: height 0.25s ease;
}
.navigation-menu {
display: flex;
flex-direction: column;
position: fixed;
top: 4rem;
left: 1rem;
width: 270px;
}
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
<button class="btn btn-dark np-trigger" data-target="#menu">Menu</button>
<nav id="menu" class="bg-dark navigation-menu np-collapsible">
<a class="nav-link" href="#">Link</a>
<a class="nav-link" href="#">Link</a>
<a class="nav-link" href="#">Link</a>
</nav>
As was mentioned, there was a race condition in the original, because there were multiple transitions being started by setting the classes as they were originally. This method avoids issues with display: none
and other race conditions by keeping it simple.
Interestingly enough, moving the menu.classList.add('np-transitioning
) line up to the same spot as removing np-collapsible
allows it to work. I feel like it could just be that the changes were essentially batched together at that point. The reason the original jquery code worked, is likely because it had the classes being added/removed without any other DOM work in-between.
Here's the original updated to work in the method mentioned above
https://jsfiddle.net/5w12rcbh/
Here's that same update, but expanded and cleaned up a little using methods like classList.replace
to perform more work at once. This adds the same toggle ability as my original snippet.
https://jsfiddle.net/bzc7uy2s/