回答編集履歴
3
箇条書きがそもそもしすぎてたので修正
answer
CHANGED
|
@@ -1,21 +1,21 @@
|
|
|
1
1
|
レビューしました。
|
|
2
2
|
|
|
3
|
-
-
|
|
3
|
+
- jQueryの`$(fn)`自体が`window.onload = fn`の完全上位互換みたいなもの
|
|
4
|
-
`window.onload = fn`は後勝ちで上書きされ、前の関数が動作しなくなる害悪設定です。二度と使う事はないので忘れ
|
|
4
|
+
`window.onload = fn`は後勝ちで上書きされ、前の関数が動作しなくなる害悪設定です。二度と使う事はないので忘れよう
|
|
5
|
-
- 単なる関数は[ホイスト（巻き上げ）](https://qiita.com/tomcky/items/988fc5f56d019e9dc097)に気をつけ
|
|
5
|
+
- 単なる関数は[ホイスト（巻き上げ）](https://qiita.com/tomcky/items/988fc5f56d019e9dc097)に気をつけよう
|
|
6
|
-
おすすめは普通に変数宣言してそれに突っ込む方法
|
|
6
|
+
おすすめは普通に変数宣言してそれに突っ込む方法
|
|
7
|
+
- fixは外に出そう。
|
|
7
|
-
|
|
8
|
+
内部で宣言されている変数は全てfix外で管理し、fixは値を書き換えるのに集中
|
|
8
|
-
- ところで`fixBox`ってなんですか？未定義でエラーが出そうです。
|
|
9
|
-
- `fixBoxL`
|
|
9
|
+
- `fixBox`や`fixBoxL`が未定義だね。fix関数の中身だから頭にfix付けてみただけ？
|
|
10
|
-
- `$(window).on('resize', fn)`の第二引数に無名関数を作って渡して
|
|
10
|
+
- `$(window).on('resize', fn)`の第二引数に無名関数を作って渡してるけど、thisを使わない設計なのでそのままfixを渡してしまったほうが簡潔
|
|
11
|
-
- jQueryオブジェクトは慣習的に変数名の頭に$を付ける人が多い
|
|
11
|
+
- jQueryオブジェクトは慣習的に変数名の頭に$を付ける人が多い
|
|
12
|
-
- オブジェクトのケツカンマはChromeでは動
|
|
12
|
+
- オブジェクトのケツカンマはChromeでは動くけど、IEでは動作しなかったような…まぁ優先度低
|
|
13
|
-
-
|
|
13
|
+
- オフセット値を書き換えたら画面更新しないと駄目でしょ
|
|
14
14
|
|
|
15
15
|
---
|
|
16
16
|
|
|
17
17
|
上記を踏まえてコードをリファクタリングしました。
|
|
18
|
-
※`fixBox`、`fixBoxL`は$BoxとBoxLなんだろうなと解釈したので変数名を変えま
|
|
18
|
+
※`fixBox`、`fixBoxL`は$BoxとBoxLなんだろうなと解釈したので変数名を変えてます。
|
|
19
19
|
|
|
20
20
|
```JavaScript
|
|
21
21
|
$(function() {
|
2
関数の動作修正
answer
CHANGED
|
@@ -10,6 +10,7 @@
|
|
|
10
10
|
- `$(window).on('resize', fn)`の第二引数に無名関数を作って渡していますが、thisを使わない設計なのでそのままfixを渡してしまったほうが簡潔です。
|
|
11
11
|
- jQueryオブジェクトは慣習的に変数名の頭に$を付ける人が多いです
|
|
12
12
|
- オブジェクトのケツカンマはChromeでは動きますが、IEでは動作しなかったような…まぁ優先度低ですね
|
|
13
|
+
- そもそもオフセット値を書き換えたら画面更新しないと駄目でしょ。
|
|
13
14
|
|
|
14
15
|
---
|
|
15
16
|
|
|
@@ -25,10 +26,7 @@
|
|
|
25
26
|
BoxL = offset.left;
|
|
26
27
|
BoxT = offset.top;
|
|
27
28
|
}
|
|
28
|
-
|
|
29
|
-
fix();
|
|
30
|
-
$(window).on('resize', fix);
|
|
31
|
-
|
|
29
|
+
var render = function() {
|
|
32
30
|
if ($(window).scrollTop() < offset.top - 20) {
|
|
33
31
|
$Box.css({
|
|
34
32
|
position: 'absolute',
|
|
@@ -42,6 +40,14 @@
|
|
|
42
40
|
left: BoxL,
|
|
43
41
|
});
|
|
44
42
|
}
|
|
43
|
+
}
|
|
44
|
+
var refresh = function() {
|
|
45
|
-
|
|
45
|
+
fix();
|
|
46
|
+
render();
|
|
47
|
+
}
|
|
48
|
+
|
|
49
|
+
refresh();
|
|
50
|
+
$(window).on('resize', refresh);
|
|
51
|
+
$(window).on('scroll', render);
|
|
46
52
|
});
|
|
47
53
|
```
|
1
文章校正
answer
CHANGED
|
@@ -1,12 +1,12 @@
|
|
|
1
1
|
レビューしました。
|
|
2
2
|
|
|
3
|
-
- そもそもjQueryの`$(fn)`自体が`window.onload = fn`の完全上位互換みたいなので
|
|
3
|
+
- そもそもjQueryの`$(fn)`自体が`window.onload = fn`の完全上位互換みたいなものです。
|
|
4
|
+
`window.onload = fn`は後勝ちで上書きされ、前の関数が動作しなくなる害悪設定です。二度と使う事はないので忘れましょう。
|
|
4
5
|
- 単なる関数は[ホイスト（巻き上げ）](https://qiita.com/tomcky/items/988fc5f56d019e9dc097)に気をつけましょう
|
|
5
6
|
おすすめは普通に変数宣言してそれに突っ込む方法です。
|
|
6
7
|
- fixは外だしします、内部で宣言されている変数は全てfix外で管理し、fixは値を書き換えるのに集中しましょう。
|
|
7
8
|
- ところで`fixBox`ってなんですか？未定義でエラーが出そうです。
|
|
8
|
-
- `fixBoxL`も未定義ですが
|
|
9
|
+
- `fixBoxL`も未定義です。`fix`で値が変わるのは`BoxL`なのでBoxLを一生懸命更新しても何も変わりませんよ。
|
|
9
|
-
(多分コードの外側で定義してあるjQueryオブジェクトなんだろうと思いますが)
|
|
10
10
|
- `$(window).on('resize', fn)`の第二引数に無名関数を作って渡していますが、thisを使わない設計なのでそのままfixを渡してしまったほうが簡潔です。
|
|
11
11
|
- jQueryオブジェクトは慣習的に変数名の頭に$を付ける人が多いです
|
|
12
12
|
- オブジェクトのケツカンマはChromeでは動きますが、IEでは動作しなかったような…まぁ優先度低ですね
|
|
@@ -14,8 +14,7 @@
|
|
|
14
14
|
---
|
|
15
15
|
|
|
16
16
|
上記を踏まえてコードをリファクタリングしました。
|
|
17
|
-
※`fixBox`、`fixBoxL`は
|
|
17
|
+
※`fixBox`、`fixBoxL`は$BoxとBoxLなんだろうなと解釈したので変数名を変えました。
|
|
18
|
-
修正して試してみてください。
|
|
19
18
|
|
|
20
19
|
```JavaScript
|
|
21
20
|
$(function() {
|
|
@@ -31,16 +30,16 @@
|
|
|
31
30
|
$(window).on('resize', fix);
|
|
32
31
|
$(window).on('scroll', function() {
|
|
33
32
|
if ($(window).scrollTop() < offset.top - 20) {
|
|
34
|
-
|
|
33
|
+
$Box.css({
|
|
35
34
|
position: 'absolute',
|
|
36
35
|
top: 'inherit',
|
|
37
36
|
left: 'inherit',
|
|
38
37
|
});
|
|
39
38
|
} else {
|
|
40
|
-
|
|
39
|
+
$Box.css({
|
|
41
40
|
position: 'fixed',
|
|
42
41
|
top: '20px',
|
|
43
|
-
left:
|
|
42
|
+
left: BoxL,
|
|
44
43
|
});
|
|
45
44
|
}
|
|
46
45
|
});
|