1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62
$(document).ready(function(){ $('#nav_menu_1').before('<a id="open_all" class="" href="#">Screen reader users can click here to open all menus</a>'); $("#menu_column ul li").not("#menu_column ul li ul li").each(function(){ checkCookie(this); }); function checkCookie(li) { var menu_index = $(li).index(); var menu_id = $(li).parent().parent().attr('id'); var cookie_state = $.cookie(menu_id + '_' + menu_index); if(cookie_state === null || cookie_state === '0') { $('ul', li).slideUp('fast'); $("a", li).not("#menu_column ul li ul li a").addClass('closed'); // Add class of closed to top level headings to add rounded corners $("span", li).addClass('closed'); // Add class of closed to top level headings to add rounded corners if ($(li).children('ul').length) { $(li).children('a, span').before('<div class="toggle nav_arrow_light_down"></div>'); } else { $(li).not("#menu_column ul li ul li").addClass('no_toggle'); // If no children add no_toggle class } }; if(cookie_state === '1') { $(li).children('a, span').before('<div class="toggle nav_arrow_light_up"></div>'); }; } $('#menu_column #open_all').click(function(){ // For screen reader users - Opens all menus $('#menu_column .toggle').each(function(){ if($(this).hasClass('nav_arrow_light_down')){ $(this).siblings('ul').slideDown('fast'); $(this).removeClass('nav_arrow_light_down'); $(this).addClass('nav_arrow_light_up'); $(this).next().removeClass("closed"); changeCookie(this) } }); }); function changeCookie(toggle) { if($(toggle).hasClass('nav_arrow_light_down')){ var menu_index = $(toggle).parent().index(); var menu_id = $(toggle).parent().parent().parent().attr('id'); $.cookie(menu_id + '_' + menu_index, '0', { path: '/' }) }else{ var menu_index = $(toggle).parent().index(); var menu_id = $(toggle).parent().parent().parent().attr('id'); $.cookie(menu_id + '_' + menu_index, '1', { path: '/' }); } } $('.toggle').click(function(){ $(this).siblings('ul').slideToggle(300); $(this).next().toggleClass('closed'); $(this).toggleClass('nav_arrow_light_up'); $(this).toggleClass('nav_arrow_light_down'); changeCookie(this); return false }); });
Refactorings
No refactoring yet !
naugtur
August 4, 2010, August 04, 2010 11:56, permalink
I didn't break it down to see how it works, but there's a tip I can give You to optimize performance.
Repeating $(this) or $("anything") line by line is not what jQuery was intended for.
I've fixed up the bit from the end of Your code
[1] saves a lot of time on recreating the $(this) object
[2] this is called chaining and is the best feature of jQuery - You can call multiple methods on an object and if they are not supposed to return a value - they return the object so You can call some more.
1 2 3 4 5 6 7 8 9 10
$('.toggle').click(function(){ var $this=$(this); // [1] $this.siblings('ul').slideToggle(300); $this.next().toggleClass('closed'); $this.toggleClass('nav_arrow_light_up').toggleClass('nav_arrow_light_down'); // [2] changeCookie(this); return false });
arkilus
August 24, 2010, August 24, 2010 23:31, permalink
Regarding chaining, this should work:
1 2 3 4
$(this).siblings('ul').slideToggle(300).end() .next().toggleClass('closed').end() .toggleClass('nav_arrow_light_up') .toggleClass('nav_arrow_light_down');
Hi, I am pretty new to JS, I have written this simple script for a menu and saving to a cookie using the jquery.Cookie plugin. It seems to work well but I'm sure it could be optimised (it seems pretty long). It would be great if someone could look over it for glaring mistakes and if anyone has any tips or tricks to optimise or write my code better, that would be great.
Thanks