回答編集履歴

4

脱字修正

2016/03/10 21:28

投稿

unau
unau

スコア2468

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

3

コードの解説を追加

2016/03/10 21:28

投稿

unau
unau

スコア2468

test CHANGED
@@ -187,3 +187,31 @@
187
187
  });
188
188
 
189
189
  ```
190
+
191
+ ---
192
+
193
+ 再度追記
194
+
195
+
196
+
197
+ まず、質問のタグに「jQuery」もつけるとよかったかな、と思いました。
198
+
199
+
200
+
201
+ 他の方も書かれていますし、繰り返しになりますが、「最初に一回だけ実行すること」と「イベントが発生したときに毎回実行すること」とを意識的にわけて考えてコードを書くとよいかな、と思いました。onClick に対する処理(イベントハンドラ)は毎回実行されるわけですが、その登録は一回だけすればいいわけです。
202
+
203
+
204
+
205
+ それと、`.slideToggle()` は、「ある要素が閉じていたら開く、開いていたら閉じる」というときに限定して使い、「ある要素が閉じていたら開きつつ関連する他の要素の状態を変え、開いていたら閉じつつ関連するほかの要素の状態も変える」というケースでは使わないほうがよいと思います。というのは、`.slideToggle()` で表示状態が反転するわけですが、関連する要素の状態が別のところで変更されている場合に整合性が取れなくなる可能性があるからです。それよりは `is(':visible')` もしくは `is('hidden')` で表示状態をチェックし、`.slideUp()`、`.slideDown()` で明示的に非表示、表示を切り替えるといいと思います。
206
+
207
+
208
+
209
+ それと、`$.fn.mynavi = function() { ... }` というのは jQuery のプラグインの定義です。必ずしも プラグインにしなくてもよいのですが、そのほうが見通しがよくなるのと、他で再利用しやすいので私は使うことが多いです。
210
+
211
+
212
+
213
+ 初期化の部分はもっとすっきり書けそうですが、あまり追求しませんでした。
214
+
215
+
216
+
217
+ もし、望んでいる動作と違うような場合はご指摘ください。

2

コードの修正

2016/03/10 09:20

投稿

unau
unau

スコア2468

test CHANGED
@@ -73,3 +73,117 @@
73
73
  ```
74
74
 
75
75
  なんか整理して書いてみたら `isMobile` 変数を使わない気がしてきました ...。
76
+
77
+
78
+
79
+ ---
80
+
81
+ 追記:修正版
82
+
83
+
84
+
85
+ またもや書きっぱなし状態で時間切れになってしまったので、解説はまたあとで ...。
86
+
87
+ ```javascript
88
+
89
+ $.fn.mynavi = function() {
90
+
91
+ var isMobile = (function($ua) {
92
+
93
+ return ($ua.indexOf('iPhone') > 0 || $ua.indexOf('iPad') > 0 ||
94
+
95
+ $ua.indexOf('iPod') > 0 || $ua.indexOf('android') > 0 ||
96
+
97
+ $ua.indexOf('BlackBerry') > 0 ||
98
+
99
+ $ua.indexOf('windows Phone') > 0 || $ua.indexOf('NOKIA') > 0 ||
100
+
101
+ /Mobile.*Firefox/.test($ua));
102
+
103
+ })(navigator.userAgent);
104
+
105
+
106
+
107
+ var $navi = $(this);
108
+
109
+ var $content = $navi.children('.sidenavi-contents');
110
+
111
+ var $button = $navi.children('h2');
112
+
113
+ var $toggleText = $button.children('.toggle-text');
114
+
115
+ var $sntexchange = $toggleText.children('.oc-mark');
116
+
117
+ $button.click(function() {
118
+
119
+ // 800px 以上の場合、h2 を隠しているのでクリックできないので呼ばれないはずだけど
120
+
121
+ // 万が一呼ばれたら何もせずに return
122
+
123
+ if ($navi.data('wide')) return false;
124
+
125
+ if ($content.is(':visible')) {
126
+
127
+ $content.slideUp();
128
+
129
+ $sntexchange.text("開");
130
+
131
+ } else {
132
+
133
+ $content.slideDown();
134
+
135
+ $sntexchange.text("閉");
136
+
137
+ }
138
+
139
+ return false;
140
+
141
+ });
142
+
143
+ function fitsize() {
144
+
145
+ if ($(window).width() <= 800) {
146
+
147
+ // $content.hide(); // 800px 未満になったときに急にリストが消えるのが変かなと思い
148
+
149
+ $toggleText.show();
150
+
151
+ $navi.data('wide', false); // 800px 未満だったことを記憶
152
+
153
+ } else {
154
+
155
+ $content.show();
156
+
157
+ $toggleText.hide(); // h2 を隠し、クリックさせないようにする
158
+
159
+ $navi.data('wide', true); // 800px 以上だったことを記憶
160
+
161
+ }
162
+
163
+ }
164
+
165
+ $(window)
166
+
167
+ .on("load orientationchange", fitsize)
168
+
169
+ .on("load resize", fitsize)
170
+
171
+ ;
172
+
173
+ fitsize(); // 初期化
174
+
175
+ if ($content.is(':visible')) { // もし最初開いたときに開いていたら「閉」に直す
176
+
177
+ $sntexchange.text("閉");
178
+
179
+ }
180
+
181
+ };
182
+
183
+ $(function() {
184
+
185
+ $('.side-navigation').mynavi();
186
+
187
+ });
188
+
189
+ ```

1

修正

2016/03/10 08:30

投稿

unau
unau

スコア2468

test CHANGED
@@ -3,8 +3,6 @@
3
3
  元々のコードだと、ShunsukeIzumi さんが指摘されているように、向きが変わったりリサイズしたときに起動されるイベントハンドラの中で onClick イベントのハンドラを追加しているので、同じイベントハンドラが何度も追加されていってしまっています。document の ready イベントで一回だけ起動されるイベントハンドラの中で一度だけ onClick イベントのハンドラを設定するようにするといいのではないでしょうか。
4
4
 
5
5
  ```javascript
6
-
7
- ipt>
8
6
 
9
7
  $(document).ready(function() {
10
8