僕がCSSコードレビューのときに気をつけている4つのこと
2019年12月16日
これはFOLIO Advent Calendar 2019 5日目の記事です。
今回は僕がCSSのレビューで気をつけていることをまとめたいと思います。
皆さんはCSSのレビューしていますか?
CSSって表示が崩れてなければOKで特にレビューしないという人も多いんじゃないでしょうか。
特にWebアプリケーション開発ではレビューが疎かになりがちかなと思います。
レビューに時間をかけすぎることは良いことではないので、僕もすごく細かいところまでレビューすることはありませんが、最低限ここは確認しておこうというのを決めてレビューするようにしています。(好みはあると思うので、それはチーム内とかレビューの中とかでコミュニケーションとって解決してください)
CSS得意じゃないエンジニアの方もいると思うので参考になれば嬉しいです。
変更に耐えうるかどうか
要素の大きさや個数などが変更されても表示崩れがないかを確認しています。これはCSSのレビューと言うより表示確認ですが、考慮漏れていることが多いので、なるべく確認するようにしています。よく確認するのは次の項目です。
- テキストの長さ
- リストの個数
- 画像のサイズ
まれにCSS以外が問題の場合もありますが、ほとんどの場合はCSSを修正すれば解消されるはずです。
スペースのとり方が適切かどうか
明確な正解はないのですが、スペースのとり方(どの要素に margin
や padding
を設定するか)は一応確認しています。この設計が悪いと、デザインを変更したり新しい要素追加するときにけっこう苦労する気がします。
個人的に見てるポイントはこんな感じです。
- 共通コンポーネントに スペースが設定されていないこと
- 基本的に要素間のスペースは1つの要素に依存していること
- 表示/非表示が切り替わる要素がある場合、状態が切り替わっても各要素間のスペースが適切に保たれているか
- marginの相殺が起きていないか
一般的な実装方法になっているか
ここで言う「一般的な実装方法」の定義は、「フロントエンジニア10人に中8人の同意を得られる実装方法」とします。要するに、突飛な実装をしてないかレビューするということです。
極端な例をあげるとすれば、要素を横並びにする際に position: absolute;
を使う、などです。
これは好き嫌いの話も入ってくるので程々にレビューする&コミュニケーションを取ることを心がけていますが、無理のある実装は後々変更がつらくなるので、なるべく避けたいなという気持ちがあります。
ルールに則っているか(則ってないならその理由)
CSSのルール(色やスペースなど)を決めている場合、そのルールに則っているかどうかを確認します。当たり前かもしれないけどとても重要なことです。
またそのルールからはみ出ている場合、なぜそうなっているかを確認するようにしています。デザイナーの人が新しく入った人でそのルールを把握していないとかどうしてもそこだけルールから外れたデザインにしたいと言われたとか、実装以外の部分が絡んでいる場合、その判断が本当に適切なのかもレビューします。
まとめ
コンポーネントベースで開発しているときは、上記のことに気をつければ最低限のクオリティを担保できるんじゃないかなと思います。
これ以外にも気をつけてることあるよ!って人は是非コメントください。