他人のCSSを覗いてみよう!CSSレビュー会開催報告 #sa_study

こんにちは、マークアップエンジニアのCandyです。
4月27日にARCANA Meetup #24と題して弊社でCSSのレビュー会を行いましたので、レポートしたいと思います!

レビュイーの3名にはレビュー用のソースコードを参加者全員に共有してもらい、レビューしてもらう前に5分程度で、コーディングについての考え方、悩んでいること、レビューをもらいたいことなどについて発表してもらいました。
レビュータイムは白熱的な議論が続き、「時間が全然足りない」と遅くまで語っていましたよ。
それでは内容について、順を追って紹介したいと思います。

自己紹介

まずは参加者全員の自己紹介!

今回の参加者は以下の通りでした。

ディレクター:1名
(マークアップもする)デザイナー:2名
マークアップエンジニア:6名
(マークアップを専門としない)エンジニア:5名


1人目:Candyのソースコード

レビュイーが気になっていること

  • CSS preprocesser: Sass(SCSS)
  • CSS設計: FLOCSS

1番目は私、Candyのソースレビューです。
当日は省略しましたが、スライドの『気をつけていること』で紹介した「ネストしすぎない(3階層まで)」「プロパティーの順序(smacss)」に関しては.scss-lint.ymlを用いて行っています。

参考:propaty sort orders:smacss
https://github.com/brigade/scss-lint/blob/master/data/property-sort-orders/smacss.txt

CSS設計手法をCSSファイルに明記しよう

CSS設計にFLOCSSを使っていて、FLOCSSのCSSディレクトリ構成であるfoundationを
ぱっと見ではフレームワークのFoundationを使っていると思われてしまいました。

readme.mdにはCSS設計について書いてあったのですが、CSS設計手法をCSSファイルには明記していなかったので、partialをincludeするファイルとしていたcommon.scssにも明記した方がわかりやすいかなと思いました。

font-sizeの変数化について

理想はfont-sizeも変数化したいけど、実案件でデザインカンプ通りにCSS実装すると例えば「1.1rem〜2.4remまで0.1remごとにすべて変数化する」ようなことが多く、逆に効率が落ちてしまって変数化するメリットを感じられないというお悩み相談。
私自身、社外のデザイナーからカンプをいただくことが多く、クライアントと既にFIXさせたデザインでもあるので、なかなかデザイナーと頻繁に相談しながらコーディングというわけにもいかずという状況です。

「デザインが悪い」、「同じくfont-sizeに関してはメリットを感じていないけど、数値は勝手にまるめちゃう」というマークアップエンジニアもいて、私はびっくりしましたが、そんなことされたら「意図があるから自分で修正しちゃう」なんてデザイナーの反論もありました。
また「line-heightpaddingfont-sizeの変数と連動するようにしているから変数化している」というご意見もありました。

この問題に関しては結論は出ていないけど、変数化を諦めないでデザインによっては取り組んでいきたいと思っています。

冗長なCSS?

.p-navi {
  // ul style

  &__item {
    // li style
  }

  &__link {
    // a style

    &:hover,
    &:focus {
      // optional style
    }
  }
}

グローバルナビゲーションのスタイルでは上記のように記述していました。
そこで「<ul>の子要素の<li>.p-navi__itemとするのは、冗長なのでは?<ul>の子要素は必ず<li>だから子セレクタ.p-navi > liで充分(この場合詳細度が上がることは気にしなくて良い)」というご意見をいただきました。
こちらに関してはどちらでも良いと思っていますが、私個人としては、classをあてた方が要素の変更もできるし、CSSの見通しもよいと思ってclassを振っておりました。

Source Mapは出すべき?

「Source Mapがないからわからない」というご意見をいただきました。
弊社では社内で統一してFLOCSSを採用しており、Blockごとにpartialするルールになっているため、Source Mapがなくてもclass名を見るとどのファイルに記述されたソースなのかわかると思い込み、昔は出力していたSouce Mapですが出力しなくなっていました。今回のご指摘により、Source Mapを出力しないのはエゴだということに気付かされました。

その他

コメントの書き方は『リーダブルコード』を読もう

写真:説明するCandy


2人目:Nomaさんのソースコード

レビュイーが気になっていること

  • CSS preprocesser: PostCSS
  • CSS設計: FLOCSS

続いて弊社のマークアップエンジニア、Nomaさんのソースレビュー!!
初めてPostCSSを使ってみたということですが、主に悩んでいることはSassのときと変わらないようです。

!important多用しちゃうんですよね…

良くないとは思っているんですけど例外的な箇所で!importantを結構使ってしまうというお悩みでした。
「例外的な箇所ではモディファイアとして追加するべきでは?」
!importantをふだんから使ってると本当に使いたいときに使えない」
.mb25.mb45があるのに.mb35はないなど、使うものだけ追加するのは良くない」
!important!dangerousimportantぐらいの気持ちで書くべき」
などなどの意見が出ました。

Nomaさんは「!important断ちしてみます」と宣言していましたよ☆

どんな風に変数を命名してるの?

お次は変数の命名についてのお悩み。
「どうしても色の名前とか使っちゃう」ということです。
あ〜確かに私も、隣接する微妙な違いのグレーとか、悩みの種なんですよね…

これについては下記のような意見でまとまりました。
「色の名前を変数にすること自体が悪いわけではない、二重変数にしたら良いのでは?」
Sassで指定するとこんな感じでしょうか。
別の箇所で同じ色が出てきても新しく定義をするとスムーズにいきそうですよね。

$color-blue: #1d62f0;
$color-orange: #ff9500;
$color-white: #fff;
$color-gray: #ccc;

$color-corporate: $color-blue;
$color-accent: $color-orange;
$color-bg: $color-white;
$color-border: $color-gray;
$color-title-bg: $color-gray;

Post CSSはSassよりどういいの??

あまりPost CSSユーザーがいなかったこともあり、あまり意見は出ませんでしたが、
いいところと言っても、コンパイルスピードくらいしか聞きませんよね…
「どっちでも好きな方使えばいいんじゃないの?どのエディタがいいのって聞いてるようなもの」
こんな感じにまとまりました。

また個人的に参考になった記事も紹介しておきますね。

PostCSSとcssnextで最新CSS仕様を先取り!- Sassから乗り換えるべきか?
https://html5experts.jp/t32k/17235/

写真:説明する野間さん


3人目:WEBCRE8さんのソースコード

レビュイーが気になっていること

モジュールがモジュールの中にある場合限定のスタイルを親モジュールと子モジュールのどちらに書くか

  • 親モジュールに子モジュールの情報を書くと子モジュールの情報を見るだけではモジュールのスタイル全てを把握しにくいが、子モジュールを使いまわしやすくなる
  • 子モジュールに親モジュールの情報を書くと子モジュールを見るだけでモジュールの情報を把握できるが親モジュールが増えるほど子モジュールに記述が増えていってしまう

属性セレクターを多用するようにしているが、どう思うか(使いやすそうかどうか)

  • 基本構造からマルチクラスを排除しようとしているため、書き方が特殊
  • CSS preprocesser: Sass(SCSS)
  • CSS設計: WEBCRE8 Original(YouCSS?!)

ラストはフリーランスでマークアップエンジニアをしているWEBCRE8 酒井優さんにソースコードを提供していただきました!
独自のCSS設計ということで、その説明から。

CSS設計の特徴

独自の命名規則

  • キャメルケースはコード上意味はない連結された熟語の区切りを表す(例:.latestNews)
  • ハイフンはモジュールの子を表す(例:.latestNews-item)
  • アンダーバーはモジュールのバージョン違いを表す(例:.button_secondary)

汎用モジュールやレイアウトモジュールを定義(本人曰く、SMACSSに似ているとのことです)

  • utillitiesは変数、ミックスイン
  • commonはタイプセレクター、属性セレクターなど
  • layoutはページ共通のレイアウト
  • modulesは汎用的に用いられるモジュール

また上記以外になるべくシングルクラスで統一しているという特徴がありました。

今回は優さんによる口頭説明があったので、みんなルールを飲み込めたのですが、ソースコードだけ渡された場合、独自の設計手法なので「コメントがないとわかりにくいかな」という意見が出ました。
あとはまとまった感じの意見ではありませんがご容赦ください💦

子モジュールの派生パターンはどのファイルに記述すべき?

例えば、header内の検索ボックスとコンテンツ内の検索ボックスでスタイルに違いがある場合、header内検索ボックスのスタイルは_header.scssに記述するべきか、_searchBox.scssに記述するべきかというお悩みについてです。

「別のモジュールとして独立させる」
「子に書く。子のスタイルなので」
「親に書きたい。そこにあるからそのスタイルなんだぞってしたい。派生パターンが無限に増えるのを危惧しないなら親に書くのが良いのでは?」
上記の通り、いろんな意見が出ました…

BEMなどの設計にしないのはなぜ?

今度はレビュアーからの質問です。おそらく参加者全員が感じていたであろう独自の設計についての質問でした。

回答は「BEMを知る前に自分で作ったから。後にBEMを知った上でも寄せなくてもいいかなと思ったので引き続き使っている。
BEMに寄せる場合、現在使用している[class*=button_]のような属性セレクタでの指定がしにくくなるため。」とのこと。

モジュールの子セレクタとしてdivと記述している箇所があるが、意図しない挙動にならないのか

引き続き、参加者からの質問。下記のようなコードに対しての指摘。

.module > div {
	// style
}

回答「slickを使用した際にdivを追加されるので、例外的に記述しているが、これは説明しないとわからないですよね」

優さん、コメントお願いします〜〜!!

結論

優さんのコードに関してはやはり、独自の設計ということもあり、説明されれば納得ということが多かったので「他人にわかりやすくコメントを書こう!」というのが結論となりました。
ソースコードにコメントを書くことは基本ですが、なかなか最適なコメントの書くことは難しいのではないでしょうか。

写真:説明する優さん


おわりに

今回はプロジェクト全体のファイルを共有して議論したこともあり、細かな部分は指摘できなかったこともたくさんあったと思います。
次回やるとしたら、モジュールをいくつか共有するだけでもかなり個人差が出ることがわかったので、もっと細かく見ていければ良いなと思いました。

とはいえ、とてもとても楽しい議論の時間でした。
ご参加いただいた皆さんからも「とても有意義な時間でした」などとコメントをいただき、都合がつかず今回は参加できなかった皆さんからも「参加したかった」「次回は参加したい」などのお声をいただきまして感謝の思いでいっぱいです。

次回のテーマは決まっていませんがARCANA Meetupを楽しみにしていただけたら幸いです!