フロントエンド面レビュー

yoshiko-pg commented 8 years ago

第3章で作ったHTMLを元に、この章ではトップページを作っていきます。 → この章では、第3章で作ったHTMLを元にしてトップページを作っていきます。

あと次の節からなんですが、カルーセルとページネーションが交互に入ってるので、先にカルーセルのHTML/CSSやって終わらせてからページネーションのHTML/CSSやったほうが混乱しない気がしました！

4.1　カルーセル部分のマークアップ

今回カルーセルの実装に使うslick用に次のHTMLを用意します。

slickっていきなりでてきてわからなかったので、↓の感じがいいかと。

今回はカルーセルの実装にslickというライブラリを使用します。slick用に次のHTMLを用意します。

あとリンクになってますが、本にするときは脚注でURL入れる感じですかね？

js_slides ll-slides

ハイフン繋ぎとアンダースコア繋ぎが＞＜ js_slidesってslick固定のクラスなんですかね？もしくはjsから使うクラスは差別化でjs__にしてるんですかね・・・個人的にはjs-_のほうがll-*にも対応していて綺麗だなと思いました。

4.2　ページネーション部分のマークアップ

ページネーションは表示される順番が決まっているため、意図的に順序づけられた項目リストを示すol要素を使います。

なるほど、たしかに！考えたことなかったです。 1 2 3 ... みたいなページングだとたしかにそうですね。ただ、カルーセルの場合は（画像に前後の文脈がない単発バナーとかなら）順番が入れ替わっても影響ないような気が・・・（でもこまかいのでスルーして大丈夫です）

<ol class="js_slides-pagination ll-slides-pagination">
  <li>●</li>
  <li>●</li>
</ol>

かなりめんどくさくなっちゃいますが、li要素の中は●より画像のキャプション（タイトル？）にしておいてCSSで隠して●を描画するのが適切ではないかなーとおもいました。。めんどいですが。。。同じキャプションがimgのaltにも入っているとよりいいかと！

4.3　カルーセルの見た目を整える

4.6　JavaScriptの準備

はじめに、JavaScriptのソースコード全体を(function() { ... });で囲みます。

カッコが抜けてる＞＜(function() { ... })();ですね。あとここの文、説明してない箇所は各々で調べてねって感じだと思うのですが、その場合は検索キーワードを与えてあげたほうが良いと思います。ここでいうと「即時関数」「スコープ」とかですかね。

はじめに、JavaScriptのソースコード全体を(function() { ... });で囲みます。理由は、定義した変数や関数が他のJavaScriptと競合しないようにするためです。

「はじめに、JavaScriptのソースコード全体をひとつの関数として囲みます。JavaScriptでは宣言した変数や関数の有効な範囲（スコープ）が関数単位で閉じられるため、(function() { ... })のように名前の無い関数で全体を囲み、最後に()をつけて即実行することで、使用した変数や関数が他のJavaScriptに干渉することを防ぐことができます。このように囲って即実行する関数は即時関数と呼ばれます。」

長い〜〜〜〜＞＜しJS文法知らない人にはきついですね・・・ただここで()まで説明しないとたぶん次のjQuery化でつまづくとおもいます。

またjQueryを使う場合は(function() { ... })();の前に$を書きます。つまり$(function() { ... })();と書きます。

jQuery使うときって$(function() { ... })();じゃなくて$(function() { ... });でしたよね？（$に引数として関数を渡す） $(function() { ... })();だと$の返り値が関数になってないとエラーになっちゃうので・・・今全体的に生JSとjQueryでインラインコードのカッコ有無がごっちゃになってるので、そこを直すのと、そのあとなんでjQueryだと()がなくなるのかを説明したほうがいいですね。関数が引数になってるんだよって（そのために↑のように生JSの段階ででちょっと掘り下げて説明したほうがいいかと思いました）

んーでもここむずかしいですね。。対象読者層によってはおまじないで済ませてしまってもいいのかな。。。。。

将来に備えていくつかの単語を使えなくする（たとえばletやyieldなど）効果もあるので、"use strict";は書いても損はありません。

strictモードだとlet使えないんだっけ！？と思ったんですが、予約語になるので変数とかで使えなくなるってことですかね。「将来に備えていくつかの単語を予約語として自由に使えなくする」のほうが意味が通るかもです。

とりあえずここまで！続きはまた！

kubosho commented 8 years ago

あとリンクになってますが、本にするときは脚注でURL入れる感じですかね？

そうです！

kubosho commented 8 years ago

カルーセルの場合は（画像に前後の文脈がない単発バナーとかなら）順番が入れ替わっても影響ないような気が

カルーセルで表示する場合も、どの要素を先に表示するか後に表示するかという意図があると思います。

つまり意図的に順序付けられた表示順なので、連携するページネーションも順序付けする必要があるかなと思いました。

kubosho commented 8 years ago

li要素の中は●より画像のキャプション（タイトル？）にしておいてCSSで隠して●を描画するのが適切ではないかなーとおもいました

@yoshiko-pg これはなぜでしょうか？自分はalt属性に適切な値を入れておくだけで事足りる気がします…

kubosho commented 8 years ago

@yoshiko-pg 指摘された点は直したので見ていただけると助かります。

yoshiko-pg commented 8 years ago

続き書きますね！

4.7　カルーセルのためにHTMLの要素を取得する

var carouselE = $(".js-slides");
var paginationE = $(".js-slides-pagination");
var paginationItemElms = $(".js-slides-pagination li");

複数がpaginationItemElmsなら単数はcarouselかcarouselElmが混乱しないかなって思いました。（エンジニアはわかるけど読んでる人はEって何だ？って思いそう・・・）

4.8　カルーセルの実装をする

どのページがいま表示されているか示すための関数 ↓ いまどのページが表示されているのかを示すための関数

4.11　ページネーションが動くようにする

現在アクティブであることを示すクラス名が最初のドットへ付くようにします。 ↓ ページネーション内の最初のli要素には現在アクティブであることを示すクラス名が付くようにします。

最初ドットが何のことかわからなかったので・・・

yoshiko-pg commented 8 years ago

（命名らへんは好みが出るとこなので対応しなくてもだいじょぶです）

yoshiko-pg commented 8 years ago

5章です。

5.1　動画部分のマークアップ

今回動画はYouTubeにある物を使います。

ここ以降もですが、「もの」「こと」「とき」らへんは漢字をひらいたほうがいいっぽいです。

5.6　アーティスト名と曲名部分の見た目を整える

こちらは背景色として白色を示す#ffffffを指定します

コードでは#fffになっています＞＜

5.11　CSSのまとめ

また図のようにタブレットで見た時は動画が1番目立つ位置にあります。

PCといっしょっぽいです・・・ http://startdash.o2p.jp/book/ch05.html

yoshiko-pg commented 8 years ago

@kubosho お待たせしました！以上ですー！

yoshiko-pg commented 8 years ago

あとこれだけ確認お願いします！ https://github.com/o2project/start-dash-of-site-making/commit/55696c0567a69428acab503416b81632ce7d1e92#commitcomment-15079127

kubosho commented 8 years ago

@yoshiko-pg ありがとうございます！

kubosho commented 8 years ago

@yoshiko-pg もろもろレビューを反映させました。レビューありがとうございました！

o2project / start-dash-of-site-making