jQuery mobile menu toggle not closing-CodePudding

I'm having an issue trying to close my mobile menu with jQuery. It opens fine and changes the attribute of the element as intended but doesn't fire on the second click, so everything after the else doesn't seem to happen.

The only thing I found that may be of consideration is that jQuery only remembers the state of attributes on load? (Not sure if this is correct) My theory is that if it checks the visibility of a loaded object that is false and on click the data-visible is changed to true it only remembers the object as false & doesn't execute the else on second click?

jQuery(document).ready(function($) {
  var primaryNav = document.getElementsByClassName('header');
  var navToggle = document.getElementsByClassName('mobile-nav-toggle');

  $(navToggle).on('click', (function() {
    var visibility = $(primaryNav).data('visible');
    if (visibility = true) {
      $(primaryNav).attr("data-visible", true);
      $(navToggle).attr("aria-expanded", true);
    } else {
      $(primaryNav).attr("data-visible", false);
      $(navToggle).attr("aria-expanded", false);
    }
  }))
});

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>
<header >
  <button  aria-controls="header" aria-expanded="false"><span >Menu</span></button>
  <div >
    <a href="<?php echo site_url(); ?>">
      <img src="<?php echo get_theme_mod('inverted-custom-logo') ?>">
    </a>
  </div>
  <div>
    <nav >
      <?php 
        $args = array('theme_location' => 'header');                            
        wp_nav_menu('header');               
      ?>
    </nav>
  </div>
</header>

CodePudding user response：

To do what you require you can just use the toggle() method to alternate the display state of an element. In addition you don't need to use the data attribute to store the state of the element, you can just read it from the DOM using the :visible selector.

Also note that you're using an odd mix of native JS and jQuery. If you've loaded jQuery in to the page, you may as well use it and make your life easier.

Here's a working example:

jQuery($ => {
  let $primaryNav = $('.header');
  let $navToggle = $('.mobile-nav-toggle');
  
  $navToggle.on('click', () => {
    $primaryNav.toggle();
    $navToggle.attr("aria-expanded", $primaryNav.is(':visible'));
  });
});

.header { display: none; }

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>
<header >
  <button  aria-controls="header" aria-expanded="false" type="button">
    <span >Menu</span>
  </button>
  <div >
    <a href="<?php echo site_url(); ?>">
      <img src="<?php echo get_theme_mod('inverted-custom-logo') ?>">
    </a>
  </div>
  <div>
    <nav >
      Menu content here...
    </nav>
  </div>
</header>