CSSを10万行書いたフロントエンドエンジニアがコードレビューをする時に気をつけていること

Retty Advent Calendar 2019 9日目の記事です。

qiita.com

この記事を読んで欲しい人

  • 社内・チーム内でCSSを書く機会が多いエンジニア
  • HTML・CSSのコーディングに強くなりたいフロントエンド・バックエンドエンジニア
  • コーディングまで行なっているデザイナー

はじめに

今年の10月に入社した松田です。Rettyではフロントエンドエンジニアとして仕事をしています。

Rettyに入社したことでランチの平均予算と口コミの更新頻度が上がりました。体重も比例して増加してしまっているので、そこはなんとかキープしていきたいと思っています。

現在はお店一覧のUI改善などをメインで行なっているため、Vue.jsのコードを書いていることが多いですが、個人的にはCSSが好きです。

途中で数えるのはやめてしまいましたが、数千〜3万行ぐらいのCSSをいくつかのプロジェクトで書いたので、ここ5年ぐらいで仕事・趣味含めおそらく10万行はCSSを書いていると思います。

CSSコードレビューで気をつけていること

RettyではバックエンドメインのエンジニアでもVue.jsやCSSのコードを書く機会があります。

私自身はこれまで、自分がほとんどのCSSを書いていたためコードレビューの機会が少なかったのですが、Rettyに入社したことでその機会が増えました。

入社して2ヶ月と少しですが、プルリクエストのレビューをしていく中で、今までは無意識に行なっていた部分がかなりあることに気づきました。

レビューするときの観点として特に気をつけていることを、3つにまとめて言語化してみましたので、順番にご紹介していきます。

1. 実際の画面で動作確認をする

当然といえば当然やることなのですが、慣れてきたり時間がなかったりするとついつい省略しがちです。

しかし、CSSは非常に壊れやすい上に、壊れているという判断のほとんどが人間の主観によるものであるため、目視で確認しておいた方が良いです。

Chrome DevToolsのレスポンシブモードやXcodeのシミュレータなどでも実機に近い環境で確認することはできますが、可能であればiPhoneAndroidなどの実機を使って確認しています。

実機で確認している理由としては、パソコンのディスプレイで見ている時と実際にスマートフォンを手にとって見ている時では画面周囲の環境が違うので、異なる見え方になってしまう場合があるからです。

私が実機確認でよく気になるポイントとしては、色と文字サイズです。色についてはディスプレイの輝度の設定や、シミュレータの外側におかれているウィンドウの背景色などによって見え方に影響を与える場合があるため、実際に確認すると思ったより暗く感じてしまう、などが出てきます。

また、文字サイズについては、ブラウザのレスポンシブモードで見ている時は10pxでもそこまで小さく感じなかったのに対し、実機で確認した場合は小さく感じるといったことが起こります。

逆に、実機では画面幅に対して文字サイズが大きく感じるなども起こり得るため、文字コンテンツが多い場合は特に気をつけて見ています。

2. HTML構造が正しく、クラス名が既存の命名規則にしたがっていることを確認する

CSSのコードレビューをする前に、まずはHTMLからレビューしています。セクショニングコンテンツや各HTML要素の使い方が間違っていないかどうかをチェックしています。

HTMLは非推奨な書き方や、やってはいけないことなどはある程度決まっていますが、これが正解と言えるものはないので、基本的には既存のコンテンツと同じようにHTML要素が使われていれば大丈夫でしょう。

またクラス名については、プロジェクト全体を通して使っている命名規則に従っていれば、基本的には良いと思っています。

より良い命名規則をプルリクエスト単位で利用することは可能ではありますが、全体から外れた命名規則を決めてしまうと、複数種類の命名規則が混在する状態になってしまいます。

そうなると、運用でカバーしないといけない範囲が広がりCSSの破綻に繋がっていくため、命名規則自体を変更するのはプロジェクト全体のリファクタリングとしてやるべきだと思います。

3. 詳細度を低く保つ

CSSをレビューする時に特に気をつけているのは詳細度です。詳細度が高いコードが存在すると予期していない上書きが発生し、スタイル崩れに繋がりやすくなるので要注意です。

Vue.jsではScoped CSSが、React.jsではstyled-componentsが使えるため、予期していない影響を受けてスタイルが崩れることはほとんどなくなってきました。しかし、詳細度を低く保つように意識しておくと保守性の高いCSSにできます。

基本的には単一のクラス名でセレクタを書いているのが望ましいです。マルチクラスの場合でも.classA.classB { ... } とはせずに、.classA { ... } .classB { ... } とします。 stlylelintのselector-max-specificityルールで制御することができ、0, 2, 0としておくことで上記のようなパターンを強制することができます。

実例

それでは、Rettyでよく目にする店舗情報を実例として見てみましょう。

Rettyのお店一覧ページで表示されるお店1件分の要素

実際にRettyの店舗一覧ページでChrome DevToolsを開いて見ていただけるとわかりますが、店舗情報全体はrestaurant-cassetteというクラス名になっており、店舗名はrestaurant-cassette__titleとなっています。命名規則BEMに従っていて、単一のクラス名で指定していることがわかります。

同じようにスタイルを適用する上では.restaurant-cassette .titleとすることも可能ではありますが、詳細度が上がってしまうので可能な限り避けています。

店舗の空席情報が表示されているカレンダー

次に、空席情報が表示されているカレンダー部分に注目してみましょう。曜日の文字色が、平日は#666、土曜は#2d88d9、日曜は#d70025となっています。

この部分のクラス名を見ると、平日はreservation-item、土曜はreservation-item reservation-item--saturday、日曜はreservation-item reservation-item--sundayとなっています。

BEMのModifierを使って土日だけ文字色を変えていますね。ここではマルチクラスにして--saturday--sundayが付いているクラスにだけcolorを指定することで、詳細度を低く保つことができています。

まとめ

他にもいくつか気をつけていることはありますが、代表的なものを3つまとめました。機会があれば、他の観点も含めてこのブログやイベントなどでお話するかもしれません。

CSSは書くこと自体は簡単ですが、良いコードを書くのは難しいです。実際にブラウザで確認しないと合っているかわからない、特定のブラウザでは意図した通りにならない、本来は変えたくない場所に影響が出てしまう、など様々な問題が起きる可能性があります。

将来的にはデザインがあればCSSを自動で生成してくれるようになるのを願っていますが、今はまだ人類が頑張らないといけないフェーズなので、破綻しないCSSを維持できるようにしていきたいところです。