2011-07-08 6 views
1

私はこのコードは動作しますが、肥大化した感じ、私はそれを行うための迅速な方法を確認してくださいtheresのだ、jQueryのを使用して、この非常に単純なタスクを実行しています:リファクタリングこのjQueryの(それが肥大化したようだ)

メニューと近いですデフォルトでは隠されています。

$(document).ready(function(){ 
var menu = $('#menu'); 
var open = $('.btnOpen'); 
var close = $('.btnClose'); 

open.click(function(){ 
    menu.show(); 
    open.hide(); 
    close.show(); 
}); 

close.click(function(){ 
    menu.hide(); 
    close.hide(); 
    open.show(); 
}); 

});

+1

は私には大丈夫そうです。私は3つの変数を1つの 'var'でグループ化します。 – alex

答えて

5

試してみてください。

$(document).ready(function(){ 
    $('.btnOpen, .btnClose').click(function(){ 
     $('#menu, .btnOpen, .btnClose').toggle(); 
    }); 
}); 
+0

元のコードよりも膨らみは少なくなりましたが、遅くなりました。私は変数として '$( '#menu、.btnOpen、.btnClose')'を宣言して、あなたが 'click()'ごとにそれを探す必要がないようにします。 – Andrew

+0

遅い?それは間違っています...オリジナルは3つの関数を呼び出しました - これは1つを呼び出す、また元の明示的に2つのイベントハンドラを宣言しました。 jQueryは一度に複数の要素を処理するために最適化されています。関数として宣言すれば、1〜2ミリ秒で節約できますが、膨大なコードは必要ありません。 –

+0

@Michael:関数を呼び出すよりもはるかに時間がかかるDOMを再度検索する必要があるため、処理が遅くなります。 –

2

あなたはグループ要素次のことができます。あなたもmenu.add(close)前もってへの参照を格納することができ

open.add(close).click(function(){ 
    var show = $(this).hasClass('btnOpen'); 
    menu.add(close).toggle(show); 
    open.toggle(!show); 
}); 

open.click(function(){ 
    menu.add(close).show(); 
    open.hide(); 
}); 

close.click(function(){ 
    menu.add(this).hide(); 
    open.show(); 
}); 

それともも可能(と短いが、それでも信頼性を)。

同期外れしないように、ちょうどtoggleを使用しないでください。可視性がこれらのボタン以外のものでは変更できないことが非常に確かな場合のみです。そして、もし速く/繰り返しクリックしても、これはうまくいきません。

+0

いいえ、前にadd()を見たことがありません... – benhowdle89

3
$('.btnOpen, .btnClose').click(function(){ 
    $('.btnOpen, .btnClose, #menu').toggle(); 
}); 

はCSSを通じて適切に初期状態を設定してください。

2

...のは、ここでいくつかのより多くのアイデアを投げてみましょう:)

$(document).ready(function(){ 
    var $buttons = $('.btnOpen,.btnClose'); 

    $buttons.click(function(){ 
     $buttons.add('#menu').toggle(); 
    }); 
}); 
関連する問題