Unable to handle click on sub menu

[robertsLando]

Hi, I'm not able to handle clicks on items in menu that have a submenu. Is there a way to fix this?

I try to explain better: I have a structure with multiple levels:

zone 1 --->kitchen ------>fridge --------->food ------>toaster ------>table --->bath ------>shower --->bedroom ------>bed zone2 ... zone 3 .....

I have set a click listener on the <a> tag of the <li> item but everytime I click it opens the sublevel or it closes the menu without trigger the click. How can I fix this? I'm using overlapping levels style for the nav

[somewebmedia]

Can you give me example code that you are using so I can replicate it?

[robertsLando]

I'm using Knockout js:

<nav id="main-nav">
      <ul>
        <li>
          <a>Dashboards</a>
          <ul data-bind="template: { name: 'page-template', data: {pages: pages}}"></ul>
        </li>
      </ul>
</nav>

<script type="text/html" id="page-template">
            <!-- ko foreach: pages -->
              <li data-bind="css: { 'page-hidden': hidden(), 'tab-page-selected': $root.activePageIndex() == $index()}">
                <a class="page-item" data-bind="click: $root.switchPage, attr: { 'data-index': $index }">
                  <span data-bind="text: title" class="page-title"></span>
                  <!-- ko if: $root.isEditing -->
                  <i data-bind="click: $root.editPage" class="icon-edit icon-white page-action"></i>
                  <i data-bind="click: $root.deletePage" class="icon-trash icon-white page-action" style="padding-left: 7px;"></i>
                  <!-- /ko -->
                </a>
                <!-- ko if: (pages.length > 0 || $root.isEditing) -->
                <ul data-bind="template: { name: 'page-template', data: {pages: pages, parent: parent}}"></ul>
                <!-- /ko -->
              </li>
            <!-- /ko -->
          <li data-bind="if: $root.isEditing, click: $root.createPage"><a style="text-align:center"><i class="icon-plus icon-white icon-nav"></i>New page</a></li>
       </script>

The menu is correctly created but I'm just not able to handle clicks on the items inside the <a>. I have checked your code and you stop propagation and prevent default when href is not present or is "#" but I still can't make this work

[robertsLando]

Ok I have found where is the problem. The problem is that I use your function initNav() to init the nav on init and when editing is enabled to display the icons to edit the page and the 'new page' item. That function totally removes the content of the nav and recreates it, now the problem is that Knockout doesn't reapply binding on the new created view so the click events doesn't work. Is there a way to update content without removing and recreating the entire nav?

[robertsLando]

Done. I have edited the source code like this:

  // get levels for submenus
        $ul.each(function(){
          createLevel($(this))
        });

        self.updateMenu = function(){
          $nav.find('ul').each(function(){
            var $menu = $(this);
            //this level is not wrapped yet?
            if($menu.parent().is('li')){
              createLevel($menu)
            }
          });
        }

  function createLevel($menu) {
          const level = $menu.parents('li').length;

          if (level !== 0) {
            const $li = $menu.parent().addClass('nav-parent');
            const $a = $li.children('a');

            // create new level
            if (!Levels[level]) {
              Levels[level] = [];
            }

            // add elements to this level
            Levels[level].push({
              nav: $menu
            });

            // what's the submenu index
            const index = Levels[level].length - 1;

            // save parent wrapper
            Levels[level][index]['wrapper'] = $menu.closest('.nav-wrapper');

            // wrap submenus
            let $wrap = $menu.wrap(`<div class="nav-wrapper nav-wrapper-${level+1}">`).parent().on('click', stopPropagation);

            if (SETTINGS.levelSpacing && (SETTINGS.levelOpen === 'expand' || (SETTINGS.levelOpen === false || SETTINGS.levelOpen === 'none'))) {
              $menu.css('text-indent', `${SETTINGS.levelSpacing * level}px`);
            }

            if (SETTINGS.levelOpen === false || SETTINGS.levelOpen === 'none') {
              // stop here
              return;
            }

            // sublevel titles
            if (SETTINGS.levelTitles === true) {
              $wrap.prepend(`<h2>${$a.text()}</h2>`);
            }

            const $next_span = $('<span class="nav-next">').appendTo($a);
            const $next_label = $(`<label for="${uniqClass}-${level}-${index}">`).on('click', stopPropagation);

            const $checkbox = $(`<input type="checkbox" id="${uniqClass}-${level}-${index}">`)
              .attr('data-level', level)
              .attr('data-index', index)
              .on('click', stopPropagation)
              .on('change', checkboxChange);

            // add checkboxes to our levels list
            Levels[level][index]['checkbox'] = $checkbox;

            $li.prepend($checkbox);

            if (!$a.attr('href') || $a.attr('href') === '#') {
              $a.on('click', preventClick(true, false)).prepend($next_label);
            }
            else {
              $next_span.append($next_label);
            }

            // insert back links
            if (SETTINGS.insertBack !== false && SETTINGS.levelOpen === 'overlap') {
              let $back = $(`<li class="nav-back"><a href="#">${SETTINGS.labelBack || ''}<span></span></a></li>`);

              $back.children('a').on('click', preventClick(true, true, () => closeLevel(level, index)));

              if (SETTINGS.insertBack === true) {
                $menu.prepend($back);
              }
              else if (isNumeric(SETTINGS.insertBack)) {
                insertAt($back, SETTINGS.insertBack, $menu);
              }
            }
          }
        }

Than just init nav using something like this:

function initNav(){

        if(!self.nav){
            var $nav = $('#main-nav');
            var $toggle = $('#nav-toggle');

            // call the plugin
            self.nav = $nav.hcOffcanvasNav(
                {
                    customToggle: $toggle,
                    maxWidth: false,
                    levelTitles: true
                }
            );
        }else{
            self.nav.updateMenu();
        }
    }

This allows to update submenu views without removing and recreating the menu. Do you want that I create a pull request?

[somewebmedia]

Even if I made the update method, it would still need to clone the new items from the original menu, which events I assume would not work with the Knockout.

I'm not sure you can use my plugin with the MVW libs, cause my plugin needs to clone the menu and insert new elements in order to work properly, so updating it with the model is hardly possible.

[somewebmedia]

@robertsLando You wrote the new message while I was writing the response, so I didn't see it :) Let me examine your code.

[robertsLando]

I can confirm that with my update the method works even with knockout ;) Just be sure to init the menu before the bindings are applied.

[somewebmedia]

I'm a little bit confused, how can it work when my plugin clones the original menu, meaning this update method would not get the newly inserted items from the original menu? Or am I missing something?

[robertsLando]

With knockout you use the method ko.applyBindings(viewModel) to apply the bindings to the dom. If you call this method after the menu is rendered the updateMenu just checks if in the cloned nav there are new ul to render and creates the required sublevels

[somewebmedia]

So when my plugin cloned the original nav, it also brought with it the data attributes, which then knockout uses to apply the bindings, so it can update the DOM afterwards. Nice, but that means the update method you created is specific only for this case, and would not have any effect for the normal plugin (update) use. :)

[robertsLando]

Yep but (at least) it allows to update the nav structure without the need to recreate the entire nav. Anyway I'm looking for some improvments I will keep you updated.

[robertsLando]

How could I "fix" the Levels object by removing levels that haven't an ul tag? I have succesfully make it possible to update levels submenu but I'm not able to do the reverse thing, remove the submenu when the ul in the li is no more present. I was thinking of parsing the entire Levels object and fix levels that doesn't exist anymore and remove them from the Object. Could you help me?

for(var l in Levels){
            for(var i=0; i<Levels[l].length-1; i++){
              var item = Levels[l][i];
             //if item nav is not present in dom remove span and everything else from the parent li
            }
    }

[somewebmedia]

I'm not keen on using the update method you proposed, because if it were to update the plugin it would be from the original DOM, not from the cloned.

I think I figured out why your events are not triggering. Let me try something first.

[robertsLando]

I have noticed that you do a check on the <a> tag and if href attr is not present or is # you set the label element to cover the entire item to handle the click, if not you set the clicl to the nav next span. I have fixed this in my case by check also for the property "data-bind". I think this could be an option that you could add to settings, maybe a shouldHandleClick function that get the <a> element as param and returns true or false if the a tag should handle click or not.

[somewebmedia]

Can you check with the new v3.1.3 version if the event's are copied with the cloned nav and triggered on click? But this time run the plugin after the knockout was initiated.

The logic here is that the cloned off-canvas nav will have the copied knockout's events with it, so they should work as on the original nav. If this is the case, I'll make an real update method that will again copy new items from the original nav along with their events.

Let me know if this works so I can implement the update method.

[robertsLando]

Hi @somewebmedia. Thanks for the update, I have just tried it but seems that is not working :cry: I think the problem here is that when I need to update the menu I have to remove the old one that was a copy of the first one, this step removes also the events linked from both the original and the cloned one as they are passed as reference.

Anyway I have been able to make everything working, I know mine is a specific case so I will just post my update here if someone will need it (or if you will want to implement something like this in future updates):

Updates to hc-offcanvas-nav.js

$.fn.extend({
    hcOffcanvasNav: function(options) {
      if (!this.length) return this;

      var self = this;

//....... same code

return this.each(function() {
        const $this = $(this);
        let $nav;

//.... same code
       // get levels for submenus
        $ul.each(function(){
          createLevel($(this))
        });

       self.updateMenu = function(){
          $nav.find('ul').each(function(){
            var $menu = $(this);
            var $parent = $menu.parent();
            //this level is not wrapped yet?
            if($parent.is('li')){
              createLevel($menu)
            }
          });

          for(var l in Levels){
            for(var i=0; i<Levels[l].length-1; i++){
              var item = Levels[l][i];

              if(item.nav.is(":hidden"))
                item.next_span.hide();
              else
                item.next_span.show();
            }
          }

 } //end of updateMenu

function createLevel($menu) {
          const level = $menu.parents('li').length;

          if (level !== 0) {
            const $li = $menu.parent().addClass('nav-parent');
            const $a = $li.children('a');

            // create new level
            if (!Levels[level]) {
              Levels[level] = [];
            }

            // add elements to this level
            Levels[level].push({
              nav: $menu
            });

            // what's the submenu index
            const index = Levels[level].length - 1;

            // save parent wrapper
            Levels[level][index]['wrapper'] = $menu.closest('.nav-wrapper');

            // wrap submenus
            let $wrap = $menu.wrap(`<div class="nav-wrapper nav-wrapper-${level+1}">`).parent().on('click', stopPropagation);

            if (SETTINGS.levelSpacing && (SETTINGS.levelOpen === 'expand' || (SETTINGS.levelOpen === false || SETTINGS.levelOpen === 'none'))) {
              $menu.css('text-indent', `${SETTINGS.levelSpacing * level}px`);
            }

            if (SETTINGS.levelOpen === false || SETTINGS.levelOpen === 'none') {
              // stop here
              return;
            }

            // sublevel titles
            if (SETTINGS.levelTitles === true) {
              $wrap.prepend(`<h2>${$a.text()}</h2>`);
            }

            const $next_span = $('<span class="nav-next">').appendTo($a);
            const $next_label = $(`<label for="${uniqClass}-${level}-${index}">`).on('click', stopPropagation);

            const $checkbox = $(`<input type="checkbox" id="${uniqClass}-${level}-${index}">`)
              .attr('data-level', level)
              .attr('data-index', index)
              .on('click', stopPropagation)
              .on('change', checkboxChange);

            // add checkboxes to our levels list
            Levels[level][index]['checkbox'] = $checkbox;
            Levels[level][index]['next_span'] = $next_span;

            $li.prepend($checkbox);

            if (!$a.attr('data-bind') && !$a.attr('href') || $a.attr('href') === '#') {
              $a.on('click', preventClick(true, false)).prepend($next_label);
            }
            else {
              $next_span.append($next_label);
            }

            // insert back links
            if (SETTINGS.insertBack !== false && SETTINGS.levelOpen === 'overlap') {
              let $back = $(`<li class="nav-back"><a href="#">${SETTINGS.labelBack || ''}<span></span></a></li>`);

              $back.children('a').on('click', preventClick(true, true, () => closeLevel(level, index)));

              if (SETTINGS.insertBack === true) {
                $menu.prepend($back);
              }
              else if (isNumeric(SETTINGS.insertBack)) {
                insertAt($back, SETTINGS.insertBack, $menu);
              }
            }
          }
        }//end of createLevel

The createLevel function is almost the same I have just added a reference on the level span to know when to show/hide it and !$a.attr('data-bind') to check if a has some events linked or no.

Than in the code just init the nav before to apply bindings and use updateMenu method to update it:

initNav = function(){

        if(!self.nav){
            var $nav = $('#main-nav');
            var $toggle = $('#nav-toggle');

            // call the plugin
            self.nav = $nav.hcOffcanvasNav(
                {
                    customToggle: $toggle,
                    maxWidth: false,
                    levelTitles: true
                }
            );
        }else{
            self.nav.updateMenu();
        }
    }

[robertsLando]

I have created this fiddle if you want to make some tests :smile:

https://jsfiddle.net/roberts_lando/ndrk2qa1/35/

[robertsLando]

Also pay attention here, why you have removed the stop propagation on label? https://github.com/somewebmedia/hc-offcanvas-nav/commit/5ffe131f2e63756c5a6ac0e74ead69543c13e7d5#diff-1aa718e0e0951d964f96a94af15a7771L339

[somewebmedia]

I'm not sure your fiddle example works without my plugin, so I'm not sure what to test.

I've commented the nav init, so the only script running is yours, and as you can see there aren't any click events working: https://jsfiddle.net/2pvx7y3t/

With this you just created a method clicked, but you are not calling it anywhere:

this.clicked = function() {
    alert("You clicked an item");
}

Also you have this code: <li data-bind="click: $root.createPage"> which I don't know what it's for.

Can you fix your example, let's say to alert every item click in the list, and also add a button that inserts some new items to the original menu. Then I'll check how my plugin interacts with it. :)

And on the question why I removed the stop propagation on label, it's because if you have a custom click event on the <a> (copied from the original menu), it wouldn't get triggered, because it is behind the label.

[somewebmedia]

I managed to get your test working. I'll see what I can do about the plugin update method now.

[somewebmedia]

I'm happy to inform you that I've managed to get my plugin working alongside knockout (or any other MVW lib), but in order to make a public version I'll need to completely re-write my code, and that will probably take some time. :)

So I can't promise you when that will be, since I've got other things on my plate. But I'll keep this ticket open, and let you know when the new version is ready.

[robertsLando]

Sorry for the late reply but I was not at my pc. I have created that fiddle very quickly by copy and paste some of my code and editing it, It is not really the same code as I have also a ViewModel for each page but that is enought for the test :smile: Knockout is really easy to understand and I really liked it even if it is not widely use like angular vue and react.

I'm happy to hear that you managed to make the example work, If you need me to edit it for example to add a new page creation method or something else just let me know.

Thanks also for your very quick replies and support with my problem I really appreciate it! Hope you will make it work someway.

Will you add a mutation observer to the original nav and update items everytime the MVW lib injects content in the DOM or what else? At the moment as I told you I have edited your code to work with my example successfully so maybe I could help you someway?

[somewebmedia]

I can't use observables here, cause my plugin is not meant for that kind of use. I'll have to traverse for the changes on the original menu and update my nav accordingly, which is a little tricky. But since I've hardcoded tested the code with knockout in your example I'm confident it will work as intended. :)

I already started decomposing my code.

[somewebmedia]

@robertsLando Tthe update is ready! It's in the branch new, there is also a knockout demo there (/demo/knockout.html). You'll see that after adding new items, the click events on the old and new items are working (they are showing console log). I made 2 examples how you can update the nav, one is immediate (in case you don't want to close your menu on item click), and the second is with the close event, so you update the nav when it's closed (that example is commented).

Also you can update the options of the plugin, just pass the object as first argument, and true as second.

Can you please check it and also test the plugin in your code, and give me the green light if everything is ok so I can merge it and push the new version :)

[robertsLando]

Thank you so much @somewebmedia! I will test it this afternoon 🎉

[robertsLando]

Testing now... Did you changed something on css too? Just to know because I had made some style changes on your css and I would like to know if I need to import it too or I can use the old css

[somewebmedia]

This was the only style change: https://github.com/somewebmedia/hc-offcanvas-nav/commit/b11f5f062a7ec08af41114bac20f9a25ff203e09 It fixes some double border bug on specific examples.

[robertsLando]

I'm having some issues when testing, seems. The problem is that the binding context changes when I'm in a foreach container. This means that inside the foreach the context is the page element (that in my case is a viewmodel like the navmodel in the example and there I access the viewModel vars. Example:

<!-- ko foreach: pages -->
       <span data-bind="text: title></span> <---throws error as when you clone the element I lose the container
    <!-- /ko -->

Now You have saved the binding but when using empty containers the example doesn't works, now I try to check if it works if I set the foreach in a container.

[somewebmedia]

Can you create for me again a jsfiddle example of what's not working?

[robertsLando]

Sorry, I was wrong, the problem was that I forgot to move the applybinding before nav init, this caused knockout to apply bindings to the cloned nav too (whitout the empty containers). Now it doesn't throw any error but doesn't renders pages, I try to do more tests

[robertsLando]

Ok everything works now, I just had to move the applyBinding call before the nav init and use update(true) instead of update(). Now the nav is rendered correctly.

One question: is it possible to make the label click to work just in the span?

[somewebmedia]

I'm not sure I understood the question :)

[robertsLando]

Sorry, I have explained it very bad :laughing: When I click on a page it always trigger both the click on the <a> tag and also opens the level. I would like to open the level just when I click on next span :)

[robertsLando]

Here two screenshots with some rendiring problems:

Black margin on left when opening level 2

The main level doesn't fill the page height

Also I think you should check for parent childs and remove the next span when the parent an ul with 0 childs or the ul is hidden

[somewebmedia]

Well that is the intention. The entire item should be clickable to open submenu. If the item has a valid link and a submenu, than only the span opens the new level.

If you take a look at the demo here: http://somewebmedia.com/hc-offcanvas-nav/ the Criptocurrency item is the only one that has a valid link, so it looks and acts different (notice the border that separates the next span).

[robertsLando]

What if I have something like a click event in <a> tag? should't it be treated as your 'Criptocurrency' item?

[somewebmedia]

That is an interesting question, didn't think about that case. Yeah it should be treated as an item with a link, but there is a problem, as I'm aware of there isn't any way of checking if event is attached to the element. So I can't know if the item has a click attached to it.

[somewebmedia]

In that case you should do a little custom styling. Add your list items some class, let's say .has-click

.hc-offcanvas-nav li.nav-parent.has-click label {
    left: auto;
    width: 45px;
}

.hc-offcanvas-nav li.nav-parent.has-click .nav-next {
    border-left: 1px solid #2c5d8f;
}

[somewebmedia]

Or you put a valid link on your original item and preventDefault on it. So, my plugin will see there is a link (eg. /charts), and it will do the work for you, but since you've prevented default click on it, it will not load the new url but trigger your attached click event instead. :)

[somewebmedia]

@robertsLando Check the latest code in the new branch and the readme: https://github.com/somewebmedia/hc-offcanvas-nav/tree/new

I've also updated the knockout demo in that branch, so take a look. Let me know if everything works, I'll be merging this and pushing the new version soon.

[robertsLando]

Sorry for the late reply @somewebmedia but I was so busy this days. I have successfully tested your new version and everything works fine, also new features now are well documented on docs so I think you can merge it :) Well done and thank you so much for the support, If I will notice something strange I will tell you.

[somewebmedia]

Great!

I've merged and pushed the new minor version v3.2.0

I'm closing this ticket, if you find any issues please open a new one.

Cheers and happy coding!