ch03.md フロントエンド面レビュー

[yoshiko-pg]

ch03.md

ファイル構成

index.htmlがないなと思いました。 あと、ライブラリのディレクトリがcomponentsって初めて見たので新鮮でした。

グローバルナビゲーションのマークアップ

activeに関しては、クラス名に接頭辞を付けなくて大丈夫です。

これはbootstrapのクラスだからですよね？上の書き方だと、オリジナルのクラスだけど例外的にプレフィックスをつけないように読み取れたので・・・

と思ったら、下の方でactiveにスタイルあててますね。 ここではll-使わないのはなぜでしょうか？

HTMLのまとめ

HTMLは問題ないように見えました。 念の為にW3Cバリデータに通してみたんですが、header要素とfooter要素にはrole属性が不要のようです。 https://validator.w3.org/#validate_by_input

あとこれは単純に質問なんですが、navってheaderの外にだすのがベターなんですかね？私いつもheaderの中に書いてました。。

ヘッダーの見た目をよくする

idとクラスの使い分けの意図が知りたいなーと思いました。 大きめの枠がidなのだとは思うのですが。

pointer: default; → cursor: default; ですかね。

あと、

#ll-nav .active > a:after,
#ll-nav .active > a:focus:after,
#ll-nav .active > a:hover:after {

これって一番上のa:afterだけじゃだめなんですかね？ あと:afterはIE8対象外なら::afterのほうが推奨になっているのでベターかなと思います！

CSSのまとめ

/* 横幅が768px以上のときの設定 */　の上だけ空行がないのが気になりました！

[kubosho]

@yoshiko-pg レビューありがとうございます！

ファイル構成

index.htmlがないなと思いました。

失念してました。付け足します…

あと、ライブラリのディレクトリがcomponentsって初めて見たので新鮮でした。

元々bowerでライブラリをインストールしていたのですが、bowerのデフォルトがbower_componentsという名前で長いため少しでも短くしたいと思ってcomponentsと名付けました。 とはいえ今はbower関係ないので、externalにしようと思います。

グローバルナビゲーション

と思ったら、下の方でactiveにスタイルあててますね。 ここではll-使わないのはなぜでしょうか？

なにか考えがあったような気もするのですが、思い出せないのでll-追加します。

HTMLのまとめ

バリデータ

ありがとうございます！header要素とfooter要素には確かにroleはいらないですね。 http://momdo.github.io/html-aria/#rules-wd によると、articleまたはsection要素の子孫でない場合にrole=bannerやrole=contentinfoは使用すべきでないと読み取れます。

あとこれは単純に質問なんですが、navってheaderの外にだすのがベターなんですかね？私いつもheaderの中に書いてました。。

自分でも考えたのですが、もし今後デザイン変更があって今みたいなheaderとnavがはっきり分かれているようなデザインで無くなったらマークアップの変更が必要になりそうと思いました。 なので、headerの中に書くというのがベターだと思います。

ヘッダーの見た目

idとクラスの使い分けの意図が知りたいなーと思いました。 大きめの枠がidなのだとは思うのですが。

idはFLOCSSのlayoutに値するところで使っています。 一応、FLOCSSの該当箇所を引用しておきます。

ページを構成するヘッダーやメインのコンテンツエリア、サイドバーやフッターといったプロジェクト共通のコンテナーブロックのスタイルを定義します。 基本的には、ページ単位で唯一の存在である要素となるため、Layoutレイヤーの要素ではIDセレクタを使うことを推奨します。 IDセレクタはセレクタの詳細度を高めてしまうため、他のレイヤーやモジュールでは推奨しません。

cursorプロパティは間違えました…

これって一番上のa:afterだけじゃだめなんですかね？ あと:afterはIE8対象外なら::afterのほうが推奨になっているのでベターかなと思います！

その通りです。

CSSのまとめ

/* 横幅が768px以上のときの設定 */　の上だけ空行がないのが気になりました！

ですね（白目）

[kubosho]

/* 横幅が768px以上のときの設定 */　の上だけ空行がないのが気になりました！

これは基本となるスタイル定義とメディアクエリ部分を1つの塊として見せたくて、空行をつけないようにしてました（というのを思い出しました）

[kubosho]

@yoshiko-pg レビューありがとうございます。いろいろ直してみました。