回答編集履歴
4
脱字修正
answer
CHANGED
|
@@ -100,7 +100,7 @@
|
|
|
100
100
|
|
|
101
101
|
他の方も書かれていますし、繰り返しになりますが、「最初に一回だけ実行すること」と「イベントが発生したときに毎回実行すること」とを意識的にわけて考えてコードを書くとよいかな、と思いました。onClick に対する処理（イベントハンドラ）は毎回実行されるわけですが、その登録は一回だけすればいいわけです。
|
|
102
102
|
|
|
103
|
-
それと、`.slideToggle()` は、「ある要素が閉じていたら開く、開いていたら閉じる」というときに限定して使い、「ある要素が閉じていたら開きつつ関連する他の要素の状態を変え、開いていたら閉じつつ関連するほかの要素の状態も変える」というケースでは使わないほうがよいと思います。というのは、`.slideToggle()` で表示状態が反転するわけですが、関連する要素の状態が別のところで変更されている場合に整合性が取れなくなる可能性があるからです。それよりは `is(':visible')` もしくは `is('hidden')` で表示状態をチェックし、`.slideUp()`、`.slideDown()` で明示的に非表示、表示を切り替えるといいと思います。
|
|
103
|
+
それと、`.slideToggle()` は、「ある要素が閉じていたら開く、開いていたら閉じる」というときに限定して使い、「ある要素が閉じていたら開きつつ関連する他の要素の状態を変え、開いていたら閉じつつ関連するほかの要素の状態も変える」というケースでは使わないほうがよいと思います。というのは、`.slideToggle()` で表示状態が反転するわけですが、関連する要素の状態が別のところで変更されている場合に整合性が取れなくなる可能性があるからです。それよりは `is(':visible')` もしくは `is(':hidden')` で表示状態をチェックし、`.slideUp()`、`.slideDown()` で明示的に非表示、表示を切り替えるといいと思います。
|
|
104
104
|
|
|
105
105
|
それと、`$.fn.mynavi = function() { ... }` というのは jQuery のプラグインの定義です。必ずしも プラグインにしなくてもよいのですが、そのほうが見通しがよくなるのと、他で再利用しやすいので私は使うことが多いです。
|
|
106
106
|
|
3
コードの解説を追加
answer
CHANGED
|
@@ -92,4 +92,18 @@
|
|
|
92
92
|
$(function() {
|
|
93
93
|
$('.side-navigation').mynavi();
|
|
94
94
|
});
|
|
95
|
-
```
|
|
95
|
+
```
|
|
96
|
+
---
|
|
97
|
+
再度追記
|
|
98
|
+
|
|
99
|
+
まず、質問のタグに「jQuery」もつけるとよかったかな、と思いました。
|
|
100
|
+
|
|
101
|
+
他の方も書かれていますし、繰り返しになりますが、「最初に一回だけ実行すること」と「イベントが発生したときに毎回実行すること」とを意識的にわけて考えてコードを書くとよいかな、と思いました。onClick に対する処理（イベントハンドラ）は毎回実行されるわけですが、その登録は一回だけすればいいわけです。
|
|
102
|
+
|
|
103
|
+
それと、`.slideToggle()` は、「ある要素が閉じていたら開く、開いていたら閉じる」というときに限定して使い、「ある要素が閉じていたら開きつつ関連する他の要素の状態を変え、開いていたら閉じつつ関連するほかの要素の状態も変える」というケースでは使わないほうがよいと思います。というのは、`.slideToggle()` で表示状態が反転するわけですが、関連する要素の状態が別のところで変更されている場合に整合性が取れなくなる可能性があるからです。それよりは `is(':visible')` もしくは `is('hidden')` で表示状態をチェックし、`.slideUp()`、`.slideDown()` で明示的に非表示、表示を切り替えるといいと思います。
|
|
104
|
+
|
|
105
|
+
それと、`$.fn.mynavi = function() { ... }` というのは jQuery のプラグインの定義です。必ずしも プラグインにしなくてもよいのですが、そのほうが見通しがよくなるのと、他で再利用しやすいので私は使うことが多いです。
|
|
106
|
+
|
|
107
|
+
初期化の部分はもっとすっきり書けそうですが、あまり追求しませんでした。
|
|
108
|
+
|
|
109
|
+
もし、望んでいる動作と違うような場合はご指摘ください。
|
2
コードの修正
answer
CHANGED
|
@@ -35,4 +35,61 @@
|
|
|
35
35
|
;
|
|
36
36
|
});
|
|
37
37
|
```
|
|
38
|
-
なんか整理して書いてみたら `isMobile` 変数を使わない気がしてきました ...。
|
|
38
|
+
なんか整理して書いてみたら `isMobile` 変数を使わない気がしてきました ...。
|
|
39
|
+
|
|
40
|
+
---
|
|
41
|
+
追記：修正版
|
|
42
|
+
|
|
43
|
+
またもや書きっぱなし状態で時間切れになってしまったので、解説はまたあとで ...。
|
|
44
|
+
```javascript
|
|
45
|
+
$.fn.mynavi = function() {
|
|
46
|
+
var isMobile = (function($ua) {
|
|
47
|
+
return ($ua.indexOf('iPhone') > 0 || $ua.indexOf('iPad') > 0 ||
|
|
48
|
+
$ua.indexOf('iPod') > 0 || $ua.indexOf('android') > 0 ||
|
|
49
|
+
$ua.indexOf('BlackBerry') > 0 ||
|
|
50
|
+
$ua.indexOf('windows Phone') > 0 || $ua.indexOf('NOKIA') > 0 ||
|
|
51
|
+
/Mobile.*Firefox/.test($ua));
|
|
52
|
+
})(navigator.userAgent);
|
|
53
|
+
|
|
54
|
+
var $navi = $(this);
|
|
55
|
+
var $content = $navi.children('.sidenavi-contents');
|
|
56
|
+
var $button = $navi.children('h2');
|
|
57
|
+
var $toggleText = $button.children('.toggle-text');
|
|
58
|
+
var $sntexchange = $toggleText.children('.oc-mark');
|
|
59
|
+
$button.click(function() {
|
|
60
|
+
// 800px 以上の場合、h2 を隠しているのでクリックできないので呼ばれないはずだけど
|
|
61
|
+
// 万が一呼ばれたら何もせずに return
|
|
62
|
+
if ($navi.data('wide')) return false;
|
|
63
|
+
if ($content.is(':visible')) {
|
|
64
|
+
$content.slideUp();
|
|
65
|
+
$sntexchange.text("開");
|
|
66
|
+
} else {
|
|
67
|
+
$content.slideDown();
|
|
68
|
+
$sntexchange.text("閉");
|
|
69
|
+
}
|
|
70
|
+
return false;
|
|
71
|
+
});
|
|
72
|
+
function fitsize() {
|
|
73
|
+
if ($(window).width() <= 800) {
|
|
74
|
+
// $content.hide(); // 800px 未満になったときに急にリストが消えるのが変かなと思い
|
|
75
|
+
$toggleText.show();
|
|
76
|
+
$navi.data('wide', false); // 800px 未満だったことを記憶
|
|
77
|
+
} else {
|
|
78
|
+
$content.show();
|
|
79
|
+
$toggleText.hide(); // h2 を隠し、クリックさせないようにする
|
|
80
|
+
$navi.data('wide', true); // 800px 以上だったことを記憶
|
|
81
|
+
}
|
|
82
|
+
}
|
|
83
|
+
$(window)
|
|
84
|
+
.on("load orientationchange", fitsize)
|
|
85
|
+
.on("load resize", fitsize)
|
|
86
|
+
;
|
|
87
|
+
fitsize(); // 初期化
|
|
88
|
+
if ($content.is(':visible')) { // もし最初開いたときに開いていたら「閉」に直す
|
|
89
|
+
$sntexchange.text("閉");
|
|
90
|
+
}
|
|
91
|
+
};
|
|
92
|
+
$(function() {
|
|
93
|
+
$('.side-navigation').mynavi();
|
|
94
|
+
});
|
|
95
|
+
```
|
1
修正
answer
CHANGED
|
@@ -1,7 +1,6 @@
|
|
|
1
1
|
時間がなくて書き殴りの未検証コードですが、こういうことがやりたいのかな、というのを整理して書いてみました。
|
|
2
2
|
元々のコードだと、ShunsukeIzumi さんが指摘されているように、向きが変わったりリサイズしたときに起動されるイベントハンドラの中で onClick イベントのハンドラを追加しているので、同じイベントハンドラが何度も追加されていってしまっています。document の ready イベントで一回だけ起動されるイベントハンドラの中で一度だけ onClick イベントのハンドラを設定するようにするといいのではないでしょうか。
|
|
3
3
|
```javascript
|
|
4
|
-
ipt>
|
|
5
4
|
$(document).ready(function() {
|
|
6
5
|
"use strict";
|
|
7
6
|
var isMobile = (function($ua) {
|