0

I bought this theme a while back and have been chopping it up and modifying it to add/remove functionality. There is this slidein menu that appears on the page once a link is clicked from the navbar menu. The issue is that there is an X button to close the menu that is supposed to toggle/remove a class from the #btn-offcanvas-menu element when it is clicked (#btn-close-canvasmenu is clickable element); it works on the desktop but not the responsive mobile site. The only way it is toggled/removed on mobile is if the original link is clicked again. I have looked at the below questions here, and several others, including trying to use the closest/find options but nothing seems to work. What am I missing?

Desired behavior: the isLeft class should toggle/be removed when the #btn-close-canvasmenu link is clicked. It is on desktop but not mobile.

This is relevant portion of the index page...

    <li id="recent-mobile"><!-- this is hidden by CSS on desktop -->
            <a id="btn-offcanvas-menu" href="#">Recent Results</a>
    </li>
    <li id="recent-results"><!-- this is hidden by CSS on mobile -->
        <div class="header-buttons pull-right hidden-xs hidden-sm">
            <div class="navright-button">
                <a href="#" id="btn-offcanvas-menu"><i class="fa fa-bars">  Recent Results</i></a>
            </div>
        </div>
    </li>

    <!-- Menu OffCanvas right begin -->
    <div class="navright-button">
        <div class="compact-menu-canvas" id="offcanvas-menu">
            <h3>menu</h3><a id="btn-close-canvasmenu"><i class="fa fa-close"></i></a>
            <nav>
                <ul class="clearfix">
                    <li><a href="index.html">Home</a></li>
                    <li><a href="features.html">Features</a></li>
                    <li><a href="pages.html">Pages</a></li>
                    <li><a href="portfolio.html">Portfolios</a></li>
                    <li><a href="shop.html">Shop</a></li>
                    <li><a href="blog.html">Blog</a></li>
                    <li><a href="contact.html">Contact Us</a></li>
                </ul>
            </nav>
        </div>
    </div>
    <!-- Menu OffCanvas right close -->

This is the relevant portion of the javascript compact.js file...

$("#btn-close-canvasmenu").on("click", function() {
    $("#offcanvas-menu").stop().animate({
        right: "-260px"
    }, 300), $("a#btn-offcanvas-menu").removeClass("isLeft")
});

$(document).on("click", "#btn-offcanvas-menu, #btn-offcanvas-menu-mobile", {} , function(a){
    var id = $(this).attr('id');
    if (id === "btn-offcanvas-menu-mobile") {
        $("#btn-close-canvasmenu").toggleClass("mobile");
    }

    return a.preventDefault(), $(this).hasClass("isLeft") ? $("#offcanvas-menu").stop().animate({
        right: "-260px"
    }, 300) : $("#offcanvas-menu").stop().animate({
        right: "0px"
    }, 300), $(this).toggleClass("isLeft"), false
});

EDIT (fixed 2/16/20): I edited the js file file to reflect this updated function: $("#btn-close-canvasmenu").on("click", function()

Essentially, what fixed it was changing the selector from #btn-offcanvas-menu to a#btn-offcanvas-menu. Not sure why that worked as opposed to using closest/find method but once I did, it toggled/removed the class on both desktop and mobile correctly. If anyone has anything thoughts on why the a made the difference with the selector, I'd appreciate the feedback as I am not an expert on js/jQuery. Otherwise, this can be considered closed.

Chris L.
  • 5
  • 3
  • 1
    @Bhargav Rao: I made the requested edits; please re-open my question you closed. – Chris L. Feb 16 '20 at 09:01
  • Seems your HTML is invalid. I see no closing `` though I see `li`, then `div`. `ul` cannot have other children than `li`. – connexo Feb 16 '20 at 09:29
  • @connexo: the only
      that exists above is closed right before the nav is closed. Every element is closed.
    – Chris L. Feb 16 '20 at 09:54
  • Not in the code you show. – connexo Feb 16 '20 at 11:31
  • @connexo There's only one
      tag in the code I attached and it's closed by the only
    tag in the code. I only provided the relevant portions of the index, not the whole index, because one of the mods originally closed the question saying only to provide what was necessary. So I edited it to the sections that were relevant to issue at hand.
    – Chris L. Feb 16 '20 at 16:03
  • Again, there is no ul included in that because the moderator said to only post what is relevant when I initially included the entire index file. The li's above this comment --- --- are partial code there purely to show the relevant elements that I am having the issue with the JavaScript. I can post the entire index if you want so you can see the entirety that has the
      but it's superfluous IMO.
      – Chris L. Feb 16 '20 at 21:56
    • Updated with small tweak that resolved the issue. – Chris L. Feb 17 '20 at 05:17
    • Then please answer your own question with what you edited into your question. People shouldn't have to read all the way through your post to see it is actually resolved. – connexo Feb 17 '20 at 07:54
    • *If anyone has anything thoughts on why the a made the difference with the selector* That usually indicates you have more than one element with `id="btn-offcanvas-menu"` (which would be illegal). Now that I'm saying this, your code indeed has that id more than once. You cannot ever have more than one element in a page with any given id. – connexo Feb 17 '20 at 07:56
    • @connexo: I originally had one for desktop (id="btn-offcanvas-menu") and one for mobile (id="btn-offcanvas-menu-mobile") but it wasn't working which is why I opened this question. Whichever way I did it, either using the same name or a different one for each, it wasn't working on mobile. One of them was hidden on desktop and the other hidden on mobile so I'm not sure why it would toggle the class for something that was hidden which it looked like it was doing. I will close this now with my answer. – Chris L. Feb 17 '20 at 17:53

    1 Answers1

    0

    Changing the selector in the compact.js file from #btn-offcanvas-menu to a#btn-offcanvas-menu fixed the issue and while this worked, it was the wrong way to correct the issue as connexo pointed out due to the duplicate ids.

    The correct way to resolve this was to change the mobile navbar itself so I could use an id specifically for the mobile that only appeared there.

    <nav id="mobile-menu" class="site-mobile-menu hidden-lg hidden-md">
        <ul>
            <li id="recent-mobile">
                <a href="#" id="btn-offcanvas-menu-mobile">Recent Results</a>
            </li>
        </ul>
    </nav>
    

    Next I changed the js function that populated the html for the mobile menu to use prepend instead of append so it populated everything above the Recent Results link...

    $("#desktop-menu > ul").children().clone().prependTo($("#mobile-menu > ul")), $("#show-mobile-menu").on("click", function() {
        $(this).toggleClass("clicked"), $("#mobile-menu > ul").stop(true, true).slideToggle()
    }); 
    

    Then I tweaked the js function to toggle the class to mobile for the #btn-close-canvasmenu if the selector was #btn-offcanvas-menu-mobile to be used later...

    $(document).on("click","#btn-offcanvas-menu,#btn-offcanvas-menu-mobile", {} , function(a){
        var id = $(this).attr('id');
    
        if (id === "btn-offcanvas-menu-mobile") {
            $("#btn-close-canvasmenu").toggleClass("mobile");
        }
    
        return a.preventDefault(), $(this).hasClass("isLeft") ? $("#offcanvas-menu").stop().animate({
            right: "-260px"
        }, 300) : $("#offcanvas-menu").stop().animate({
            right: "0px"
        }, 300), $(this).toggleClass("isLeft"), false
    });
    

    Finally, I tweaked the js function that closed the menu to check the class to determine which id to toggle the isLeft class for...

    $(document).on("click","#btn-close-canvasmenu", {} , function(a){
        var id = $(this).attr('id');
    
        if ($(this).hasClass("mobile")) {
            var container = $("#btn-offcanvas-menu-mobile");
            $("#btn-close-canvasmenu").toggleClass("mobile");
        } else {
            var container = $("#btn-offcanvas-menu");
        }
    
        $("#offcanvas-menu").stop().animate({
            right: "-260px"
        }, 300), container.removeClass("isLeft")
    });
    
    Chris L.
    • 5
    • 3
    • You still have **duplicate ids** which is an **absolute must-fix**. – connexo Feb 17 '20 at 20:41
    • @connexo: Here's the thing tho: there's one navbar for the desktop and the javascript uses it to populate the html for the mobile navbar. The css hides one of the navbars depending on screensize but there's always the exact same two sets of
    • 's. So that's how it gets duplicated but one set is always hidden. That's why I thought the toggle would only affect the visible id but I guess not. I just reworked the navbars and the js and will post my changes as the answer since I no longer need the a in the selector for the js. Thanks for your help.
    • – Chris L. Feb 17 '20 at 23:46
  • `#btn-offcanvas-menu` is dupe, which makes selecting by `id` difficult. Attribute `id` **must be unique** - everything else is `class` (which obviously does not have to be unique). – Martin Zeitler Feb 18 '20 at 00:31