HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Simple menu with sub-levels

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplewithsubmenulevels

Problem

The following works as intended, however is there a more concise way to write this script without changing it too much? It's a simple menu with sub-levels which have a plus/minus icon when each li opens/closes.

jQuery:

$(document).ready(function() {
    $li = $('#main > li.expand');
    $li.click(function(){
        $ul = $(this).find('ul.sub-level').delay(200).slideDown(400);
        $(this).removeClass('expand').addClass('minus');
        $('#main > li > ul').not($ul).slideUp('slow');
        $('#main > li.minus').not($(this)).removeClass('minus').addClass('expand');

    })

    $li2 = $('.sub-level > li.expand');
    $li2.click(function(){
        $(this).find('ul.sub-level2').delay(200).slideToggle(400);
        $li2.toggleClass('minus').toggleClass('expand');
    })      
});


HTML:


test-1

    sub-test-1
        
            sfasfasf
            sfafasf
        
    
  
  
  test-2
  test-3
  
    sub-test-2
    
   
   test-4
   

Solution

Some suggestions:

  • Use var when declaring variables, unless you really want to make global variables (which you probably don't)



-
Combine var statements using ,

var a = 1, b = 2, c = 3;


-
Combine classes in toggleClass

$(this).toggleClass('minus expand')


  • Cache selectors that are used often (like $('#main'))



  • You don't need to use $li2 inside the click handler, you can just use $(this)



  • Turn repeated code into a function



$(function(){
    var $main = ('#main'),
    slide = function(ele, find){
        return $(ele).toggleClass('minus expand').find(find).delay(200).slideToggle(400);
    },
    $li = $('> li.expand', $main).click(function(){
        var $ul = slide(this, 'ul.sub-level');
        $('> li > ul', $main).not($ul).slideUp('slow');
        $('> li.minus', $main).not(this).toggleClass('minus expand')
    }),
    $li2 = $('.sub-level > li.expand').click(function(){
       slide(this, 'ul.sub-level2');
    });
});

Code Snippets

var a = 1, b = 2, c = 3;
$(this).toggleClass('minus expand')
$(function(){
    var $main = ('#main'),
    slide = function(ele, find){
        return $(ele).toggleClass('minus expand').find(find).delay(200).slideToggle(400);
    },
    $li = $('> li.expand', $main).click(function(){
        var $ul = slide(this, 'ul.sub-level');
        $('> li > ul', $main).not($ul).slideUp('slow');
        $('> li.minus', $main).not(this).toggleClass('minus expand')
    }),
    $li2 = $('.sub-level > li.expand').click(function(){
       slide(this, 'ul.sub-level2');
    });
});

Context

StackExchange Code Review Q#13468, answer score: 6

Revisions (0)

No revisions yet.