factor1 / nifty-nav Goto Github PK

Would be a good idea to move this to a Bower Package.

The example in the repo is extremely out of date and should be removed (or updated but dont see that happening, really...)

Create an npm package for easy installations.

By default, subMenuParentLink is false, but why is this the case? A parent link in a menu is still a valid link, and the user expects this to be followed if they click on it. I understand if this was implemented to make it easier for the sub-menu functionality to work, for example

niftyNav({
  subMenus: true,
  subMenuParentLink: true,
});

leads to behavior where the link is followed and the sub menu will toggle open.

This functionality may need to be updated to allow for both following parent links and toggling sub menus independently.

Should restructure plugin to have /src/ and /dist/ with a proper gulp.js file.

Currently Nifty Nav uses this code

header{
  z-index: $header-index;
}

but this should probably be namespaced in some way. In its current state, it is applying this z-index to any header in the site, and this can potentially cause unexpected issues in other areas, for example any header that may be placed inside on an article, main, etc.

Look into the possibility of locking the page body from scrolling but allowing the navigation to scroll.

To make it more flexible, would be good to add a $panel-height option in the settings. Default value could be 100%, but easy access to change when sites need a full screen menu of 100vh or something.

Add a feature that allows any list item that has another UL inside it to be toggled and open in the panel itself.

Currently, the toggle button assumes that it will always be a hamburger icon, and adding text alongside of the hamburger icon requires basically a custom implementation in order to keep the icon and text in the same element.

This should probably be updated to easily allow text to be shown alongside the hamburger icon.

This option is on by default, but is preventing the parent nav items from being followed in a standard dropdown implementation. It is targeting all parent links on the page instead of being isolated to just those inside of nifty nav itself.

if( subMenuParentLink === false ){
      $('.menu-item-has-children > a').click(function(event){
        event.preventDefault();
      });
    }

should be more like

if( subMenuParentLink === false ){
      $('.nifty-panel .menu-item-has-children > a').click(function(event){
        event.preventDefault();
      });
    }

Currently, if a menu has sub-menus, and you click on a sub-menu item to navigate to a page you get a weird behavior of the sub-menu closing and then the page navigates to where ever you clicked. This is not a bug, but it could be better.

This code is likely the culprit of this behavior:

$nifty_parent.click(function(){
  $nifty_parent_active = $(this);
  $nifty_parent_active.find('.sub-menu').slideToggle(300);
  $nifty_parent_active.toggleClass('nifty-menu-opened');
});

When clicking on a nav item the user is really clicking on the a element. $nifty_parent is set here $nifty_parent = $('.nifty-panel ul li');. So since the click listener is on the li the code $nifty_parent_active.find('.sub-menu').slideToggle(300); gets triggered by way of bubbling.

I think that if either the bubbling is stopped when clicking an anchor or if this piece of code is extracted if would be a better experience.

When subMenus is true Nifty Nav uses preventDefault to disable the parent nav item from clicking through to the link (if there is one). This may cause issues for some users on desktop who wish to allow parent items to be clickable.

Should think about adding a preventDefault: false option if users want to turn off this feature.

Current work around for allowing link clicks on desktop and submenus on mobile:

  $window = window.innerWidth;
  if( $window <= 688 ){
    niftyNav({
      subMenus: true
    });
  } else{
    niftyNav();
  }

Need to revert the width/height to 40px on the nifty-nav-toggle.

Currently the <a> anchor is controlling the position but it would be better to allow that to be 100%/100% to have a larger click area and then adjust the span, span:before, span:after classes to align better and use those for positioning.

The last nifty nav update was an unintended breaking change and needs to be rolled back ASAP. Rolling back, issuing a patch, and then bumping the version to a new major version should fix the install issues that have been cropping up here and there when installing projects via npm install

Noticed a bug with Tonik, when subMenus are set to true. Clicking on any link applies the class nifty-menu-opened to that links parent list item, but I think that this should only happen if that link is also in a sub-menu. This code would need to be adjusted, as it ends up looking for any anchor in the entire menu instead of just anchors in a sub-menu:

The issue this causes is evident in this image:

I had styled the caret down to appear on list items that have the class nifty-menu-opened assuming that class would only ever be applied to list items that contained sub-menus, but since every list item is given that class when clicking on the anchor it causes the offset seen in News & Events. My CSS has been adjusted for this, but this will still need to be corrected in the code.

In its current state Nifty Nav is a function that relies on jQuery. It works so no issue there, but why not just make it a true jQuery function so you can choose what element is will act upon? Example $(item).niftyNav();

For our sites there is usually one sliding menu, but if there is ever a design that needs more than one the current implementation no longer works, as all menus will have the exact same properties.

A true jQuery plugin set up will allow for more than one menu/panel, and will also allow for situations when someone wants to chain additional jQuery methods to the Nifty Nav. Example niftyNav().css('color', 'red'); would cause an error while $(item).niftyNav().css('color', 'red'); will work fine.

Something along the lines of:

niftyNav({
  headerIndex: 9999,
  panelIndex: 9998
});

Currently there is no default styling for submenus. Currently with this code

niftyNav({
  subMenus: true,
});

the submenus still display on the page, and clicking the parent hides them.

CSS should be added to hide them with display: none by default.

Also, if this option is set to true it would be good to add a default icon or something to visualize that there is a sub menu available.

The panel reveal currently uses the jQuery slideToggle() method, but why not use CSS animations/transitions which are faster and more performant than JS for this? Also, to my knowledge, this only allows something to slide up or down, not left or right. Using CSS would open up the possibility to the panel to be revealed in any number of ways.