.slideToggle()が勝手に何度も実行される状態を解消したい

Question

初めましてこんにちは。
javascriptの.slideToggle()が何度も実行されて、開閉が何度も起こる事態について質問させていただきます。
これは、何度もクリックしたときにその回数分開くのをstop()で止めたいという種類のものではありません。

したいことは、サイドナビゲーションを、userのスマホやタブレットとそれ以外によって条件分岐をし、更に画面サイズ（800px）で条件分岐をします。800px未満の場合は、サイドナビゲーションのSIDENAVI以外の部分を閉じた状態にし、SIDENAVI の部分をクリックしたとき、それ以下のリスト部分を開閉させ、800p以上の場合は、全部開いた状態にします。
閉じた状態が初期状態の場合（800px未満）は、.oc-markの文字を、リストが閉じているときは"開"、開いているときは"閉"にします。開いた状態が初期状態の場合（800px以上）は、クリックで開閉させず、"開"も"閉"も表示させません。（開、閉の非表示は、javascriptを使わず、cssで実装しています。）また、スマホが回転されたときやPCでブラウザの幅をリサイズされたときに、画面サイズを再度取得し、それに合わせてコンテンツを表示しています。

問題は現在の状態だと、
スマホの場合、回転させていない場合は意図通りクリックで開閉してくれるのですが、一度画面を回転させると、開いて閉じる、という動作になり、**二度目回転させると、開いて閉じて開く、**という状態になります。さらに何度も回転させていると、一度のクリックで、その回転させた回数分開閉を繰り返します。
PCの場合は、どこかの時点で画面をリサイズし、800px未満に一度なると、そのあと画面を広げて800px以上にして、サイドメニューが常に表示される状態になってほしくても、開いた状態にはなるのですが、クリックで開閉されてしまいます。さらにこちらも一度のクリックで何度も開閉を繰り返すようになります。

javascriptのコメントアウトの部分は、この何度も開閉する状態を回避するために、回転やリサイズされた際に強制的にリロードさせていたのですが、今回、ビデオやオーディオといったコンテンツを配信することにしましたので、スマホの回転や画面サイズ変更のたびにリロードしていると、再生していたコンテンツも一緒にリロードされてしまい、継続的に再生できない事態になりますので、リロードさせることができなくなりました。

初心者の域を出ておらず、効率的なコードの組み方がわからないため、大変恥ずかしい状態ではありますが、解決策がありましたら、ご教授いただきたいと思い、投稿いたしました。
下記が使用しているHTMLとjavascriptです。

どうぞよろしくお願いいたします。

<nav class="side-navigation">
  <h2>
    <span class="toggle-text">SIDENAVI
      <span class="oc-mark">開</span>
    </span>
  </h2>
  <ul class="sidenavi-contents">
    <li>
      <h3>見出し</h3>
    </li>
    <li class="sidenavi-list">
      <a>りすと１</a>
    </li>
    <li class="sidenavi-list">
      <a>りすと２</a>
    </li>
    <li class="sidenavi-list">
      <a>りすと３</a>
    </li>
    <li class="sidenavi-list">
      <a>りすと４</a>
    </li>
    <li class="sidenavi-list">
      <a>りすと５</a>
    </li>
  </ul>
</nav>

$(document).ready(function() {
    "use strict";

    var $ua = navigator.userAgent;
    $(function() {

        if ($ua.indexOf('iPhone') > 0 || $ua.indexOf('iPad') > 0 || $ua.indexOf('iPod') > 0 || $ua.indexOf('android') > 0 || $ua.indexOf('BlackBerry') > 0 || $ua.indexOf('windows Phone') > 0 || $ua.indexOf('NOKIA') > 0 || /Mobile.*Firefox/.test($ua)) {

            $(window).on("load orientationchange", Fitsizem);

            function Fitsizem() {
                var _width = $(window).width();

                if (_width <= 800) {

                    $(".side-navigation .sidenavi-contents").hide();
                    var flg = "close";

                    $(".side-navigation h2").click(function() {
                        var snavtglpart = $(".side-navigation .sidenavi-contents");
                        var sntexchange = $(".side-navigation h2 .toggle-text .oc-mark");

                        snavtglpart.slideToggle();

                        if (flg === "close") {
                            sntexchange.text("閉");
                            flg = "open";
                        } else {
                            sntexchange.text("開");
                            flg = "close";
                        }
                    });

                } else {
                    $(".side-navigation .sidenavi-contents").show();
                }
            }
            /*
            var timer;
            $(window).on("orientationchange", function() {
                clearTimeout(timer);
                timer = setTimeout(function() {
                    location.reload(true); 
                }, 100);
            });
            */

        } else {

            $(window).on("load resize", Fitsizepc);

            function Fitsizepc() {
                var _width = $(window).width();

                if (_width <= 800) {

                    $(".side-navigation .sidenavi-contents").hide();
                    var flg = "close";

                    $(".side-navigation h2").click(function() {
                        var snavtglpart = $(".side-navigation .sidenavi-contents");
                        var sntexchange = $(".side-navigation h2 .toggle-text .oc-mark");

                        snavtglpart.slideToggle();

                        if (flg === "close") {
                            sntexchange.text("閉");
                            flg = "open";
                        } else {
                            sntexchange.text("開");
                            flg = "close";
                        }
                    });

                } else {
                    $(".side-navigation .sidenavi-contents").show();
                }
                /*
                var timer = false;
                $(window).resize(function() {
                	if (timer !== false) {
                		clearTimeout(timer);
                	}
                	timer = setTimeout(function() {
                              location.reload(true); 
                	}, 200);
                });
                */

            }
        }
    });
});

Answer 1

イベントの設定が実行されるタイミングをロード時の1回だけにしましょう。
何度も繰り返し実行される関数内に、クリックイベントの設定を書くとご質問されているような意図しない動作に当然なります。

それと、下記は同じ意味です。なので、入れ子で使われていますが内側の関数は不要です。
プログラム的には、ご自身が書いたとおりに動いています。いろいろと見直すといいと思います。

$document.ready(function() {
 // 
});

$(function() {
 // 
});

Comments

[nuttstock]

ご回答いただき、ありがとうございます。
アドバイスありがとうございます。
もう一度考えてみます。

Answer 2

あまり確りと見ていませんが、
$("").click(function() {}は呼ばれるたびにイベントリスナーが追加されていったと思います。（呼ばれるたびに同じイベントが追加されていきます）
この場合、resize orientation loadのたびに追加されているのかと考えられます。

Comments

[nuttstock]

ご回答いただき、ありがとうございます。
皆さんにアドバイスいただいても、あまりピンとこず、やはりきちんとしたjavascriptの素地がないことが大きな原因だと思い知りました。
もう一度考えてみます。

Answer 3

時間がなくて書き殴りの未検証コードですが、こういうことがやりたいのかな、というのを整理して書いてみました。
元々のコードだと、ShunsukeIzumi さんが指摘されているように、向きが変わったりリサイズしたときに起動されるイベントハンドラの中で onClick イベントのハンドラを追加しているので、同じイベントハンドラが何度も追加されていってしまっています。document の ready イベントで一回だけ起動されるイベントハンドラの中で一度だけ onClick イベントのハンドラを設定するようにするといいのではないでしょうか。

$(document).ready(function() {
    "use strict";
    var isMobile = (function($ua) {
      return ($ua.indexOf('iPhone') > 0 || $ua.indexOf('iPad') > 0 ||
              $ua.indexOf('iPod') > 0 || $ua.indexOf('android') > 0 ||
              $ua.indexOf('BlackBerry') > 0 ||
              $ua.indexOf('windows Phone') > 0 || $ua.indexOf('NOKIA') > 0 ||
              /Mobile.*Firefox/.test($ua));
    })(navigator.userAgent);

    var $content = $(".side-navigation .sidenavi-contents");]
    var $sntexchange = $(".side-navigation h2 .toggle-text .oc-mark");
    $(".side-navigation h2").click(function() {
        if ($content.is(':visible')) {
          $content.slideUp();
          sntexchange.text("開");
        } else {
          $content.slideUp();
          $sntexchange.text("閉");          
        }
    });
    function fitsize() {
      if ($(window).width() <= 800) {
        $content.hide();
      } else {
        $content.show();
      }
    }
    $(window)
    .on("load orientationchange", fitsize)
    .on("load resize", fitsize)
    ;
});

なんか整理して書いてみたら isMobile 変数を使わない気がしてきました ...。

---

追記：修正版

またもや書きっぱなし状態で時間切れになってしまったので、解説はまたあとで ...。

$.fn.mynavi = function() {
  var isMobile = (function($ua) {
    return ($ua.indexOf('iPhone') > 0 || $ua.indexOf('iPad') > 0 ||
            $ua.indexOf('iPod') > 0 || $ua.indexOf('android') > 0 ||
            $ua.indexOf('BlackBerry') > 0 ||
            $ua.indexOf('windows Phone') > 0 || $ua.indexOf('NOKIA') > 0 ||
            /Mobile.*Firefox/.test($ua));
  })(navigator.userAgent);

  var $navi = $(this);
  var $content = $navi.children('.sidenavi-contents');
  var $button = $navi.children('h2');
  var $toggleText = $button.children('.toggle-text');
  var $sntexchange = $toggleText.children('.oc-mark');
  $button.click(function() {
    // 800px 以上の場合、h2 を隠しているのでクリックできないので呼ばれないはずだけど
    // 万が一呼ばれたら何もせずに return
    if ($navi.data('wide')) return false;
    if ($content.is(':visible')) {
      $content.slideUp();
      $sntexchange.text("開");
    } else {
      $content.slideDown();
      $sntexchange.text("閉");
    }
    return false;
  });
  function fitsize() {
    if ($(window).width() <= 800) {
      // $content.hide(); // 800px 未満になったときに急にリストが消えるのが変かなと思い
      $toggleText.show();
      $navi.data('wide', false); // 800px 未満だったことを記憶
    } else {
      $content.show();
      $toggleText.hide(); // h2 を隠し、クリックさせないようにする
      $navi.data('wide', true); // 800px 以上だったことを記憶
    }
  }
  $(window)
  .on("load orientationchange", fitsize)
  .on("load resize", fitsize)
  ;
  fitsize(); // 初期化
  if ($content.is(':visible')) { // もし最初開いたときに開いていたら「閉」に直す
    $sntexchange.text("閉");
  }
};
$(function() {
  $('.side-navigation').mynavi();
});

---

再度追記

まず、質問のタグに「jQuery」もつけるとよかったかな、と思いました。

他の方も書かれていますし、繰り返しになりますが、「最初に一回だけ実行すること」と「イベントが発生したときに毎回実行すること」とを意識的にわけて考えてコードを書くとよいかな、と思いました。onClick に対する処理（イベントハンドラ）は毎回実行されるわけですが、その登録は一回だけすればいいわけです。

それと、.slideToggle() は、「ある要素が閉じていたら開く、開いていたら閉じる」というときに限定して使い、「ある要素が閉じていたら開きつつ関連する他の要素の状態を変え、開いていたら閉じつつ関連するほかの要素の状態も変える」というケースでは使わないほうがよいと思います。というのは、.slideToggle() で表示状態が反転するわけですが、関連する要素の状態が別のところで変更されている場合に整合性が取れなくなる可能性があるからです。それよりは is(':visible') もしくは is(':hidden') で表示状態をチェックし、.slideUp()、.slideDown() で明示的に非表示、表示を切り替えるといいと思います。

それと、$.fn.mynavi = function() { ... } というのは jQuery のプラグインの定義です。必ずしも プラグインにしなくてもよいのですが、そのほうが見通しがよくなるのと、他で再利用しやすいので私は使うことが多いです。

初期化の部分はもっとすっきり書けそうですが、あまり追求しませんでした。

もし、望んでいる動作と違うような場合はご指摘ください。

Comments

[nuttstock]

ご回答いただき、ありがとうございます。
僭越ではありますが、このコードの場合、800px以上の時にクリックでリスト部分が閉じてしまい、その後開きません。800px以下の場合、リストは閉じていますが、クリックで開きません。.oc-markの部分だけ、クリックで一度だけ変更されるだけで、その後はクリックも無効です。

11行目の　]　を削除し、
16行目の　sntexchange.text("開");　の頭に$を加えて実行した結果、
上記の結果になりました。

また何か解決方法が浮かびましたら、ご回答いただけると幸いです。
よろしくお願いいたします。

[unau]

外仕事から戻ってきたんでこれから修正版を作ってみます。

[nuttstock]

ありがとうございました。
作っていただいたスクリプトでおかしな挙動が見当たらなくなりました。
貴重な時間を割いて問題に付き合っていただき、誠にありがとうございます。
アドバイスもたくさんいただき、今後に役立てたいと思います。

>>質問のタグに「jQuery」もつけるとよかったかな、と思いました。
すっかり抜けてしまっていました。今後、そのようにいたします。

ありがとうございました。

Answer 4

リサイズする度にイベントが設定されているためではないでしょうか。

Comments

[nuttstock]

ご回答いただき、ありがとうございます。
継続的によく考えてみます。
ありがとうございました。