フロントエンドエンジニアのフリーランスhoshi(@funclur)です。
先日下記ツイートがバズっていました。
メイプルシステムズのフロントエンドエンジニアとして @youg_uraban がフロントを目指す後輩に助言した内容が秀逸なので、フロントを目指している人は読んで見たほうが良いよ。 pic.twitter.com/zD42HOQGwy
— Motcii (もっち〜) ＠ 逆張り経営のエンジニア社長 ＆ 「iU」客員教授 (@maple_no_motcii) January 26, 2019
メイプルシステムズのフロントエンドエンジニアとして @youg_uraban がフロントを目指す後輩に助言した内容が秀逸なので、フロントを目指している人は読んで見たほうが良いよ。
このツイートに対して賛否両論がありました。
つまり、要約すると後輩へのコーディングレビューの内容に対して賛否が集まっていたのですが、ではどうすれば賛だけになったかな？という疑問がわきました。
そのためこの記事は自分自身がやってるコーディングレビューでの心構えを提案した記事になります。
この記事では以下のことがわかります。
- この件のコードレビューに対して自分自身だったらどうレビューするか１つずつ解説
- コーディングレビューする時の心構え
コードレビューする側（レビュアー）も、コーディングレビューされる側（レビュイー）も見てほしい内容になってますのでよかったら読み進めください。
誠に勝手ではありますが、メイプルシステムズのレビューのレビューしてみました！
ツイッターの反応は賛否両論
まずこの投稿に対しての賛否両論について見ていきます。
その時は凹むかもしれませんが、こういう人がいるから人は成長する気がしますね。
この後輩さんがより高みにいけるように応援したいと思いました！！
自分のことはとりあえず置いときますが。笑
これ凄くリツイートといいねが
増えているんだけどねもっと若手の気持ち考えて
優しくしてやれって意見もチラホラいやいや
これくらい出来ないなら
辞めとけって優しくないかな傷口舐めて欲しいだけなら
上には来れないだけよね誰でも成れるほど
甘くはないんだよ
職人の世界はさ https://t.co/F8hhpmqV6X— Motcii (もっち〜) ＠ 逆張り経営のエンジニア社長 ＆ 「iU」客員教授 (@maple_no_motcii) February 3, 2019
これ凄くリツイートといいねが
増えているんだけどねもっと若手の気持ち考えて
優しくしてやれって意見もチラホラいやいや
これくらい出来ないなら
辞めとけって優しくないかな傷口舐めて欲しいだけなら
上には来れないだけよね誰でも成れるほど
甘くはないんだよ
職人の世界はさ
レビューをレビューしてみた
状況：ポートフォリオのレビューらしい？
前提としてなんで後輩のポートフォリオのレビューのようです。
恐らくtwitterを追ったところバックエンド→フロントエンドのジョブチェンジした社員のポートフォリオということでした。（たぶん）
つまり実務案件ではないというところは頭に入れておいてもよいかもです。
では早速勝手にレビューしてみます！
レビュー内容
書いてある内容は概ね正しいと思っております。
誤クリックの可能性
・適切なマージンが空いてないため誤クリック
→スマートフォンは特に注意した方がいいですね。ちなみにスマートフォンサイトでのボタンのサイズは高さ44pxが推奨されてます。
モーダルウィンドウ
・モーダルの背景のオーバーレイclose
これは基本の操作として覚えておくといいです。閉じるボタンを押す人もいれば背景を押して閉じる人もいます。
プラグインの初期設定では背景を押しても閉じないこともあります（Vueのモーダルなど）ので確認しましょう。
また、TABで回ることも考えモーダル以外の要素はタブ操作不可にする必要があります。
・なぜスクロールの必要のないモーダルにスクロールバーが表示されてるか？
これはわかります、こうなっちゃった気持ちも、これを指摘する気持ちも。
自分だったら「コンテンツ量がモーダルの高さに収まってる場合スクロールバーは不要です。」のように感情殺して機械的な文言吐きますね。
フォーム内の幅
・フォームと文言がガタガタで整理されてない
見ないとわからないですが左端がそろってなかったのでしょうか。
恐らく幅の問題かと思います。こういった場合は一つの例とtable要素を使うこともありです。
下記MDNではformを綺麗に実装する実例があります。
要素の位置
・ボタンが離れてる
UI設計ですね。
2つの位置の距離は関係性を表すと思うので、常に「なぜそうするのか」の意味を考えるといいです。
要素の位置
・人によって感じ方は違いますがアニメーションがうっとおしくないか？
これは見ないとわかりませんが、ポートフォリオなので所謂試したい、勉強したいというところがあったと察します。
ただアニメーションもアクセシビリティ的には5秒で止めなきゃいけなかったり停止ボタンをつけなきゃいけなかったりなんで、その辺は実務では気をつけなければいけないところではあります。
ただ作り手としてはアニメーションはさせたいですね。とは言いつつ見る側のことを第一優先に考えるというところでしょうか。
幅を変えた時の横スクロール
・横スクロール出てしまうのは論外です。
論外！ｗ
確かにそうなのですが、自分なら「○○（タブレット幅）で横スクロールがでてます」として、ここも感情は押し殺したいところ。
要素のサイズ
・ボタンが小さすぎて押せません。
・ボタン以外にも小さすぎる
実機で確認するとわかりやすいかもですね。サイトを見る人は指が大きい人もいればお年寄りもいます。
つまり、ユーザー視点でUIを考えましょうということですね。
入力フォームで拡大するやつ
・フォームをアクティブにすると拡大します。
inputやtextareaは16px以上にしないとiphoneでズームしてしまうというなんともよくわからない仕様があります。
なので16px以上は割とデフォルトにしておくといいです。
セマンティクス
・なぜほとんどdivなの？タグには意味があります。
これはその通りですね。「ただなぜほとんどdivなの？」とか書いちゃうと相手は嫌な気しかしないですよね。
ただ「タグには意味があります」、となぜdivだけじゃだめなのかを言ってくれてるのでこういうところはとてもいいですね。
・インラインスタイルはソースの見通しを悪くする
これもその通りです。運用性も悪くなりますので、注意です。
という訳で、レビューは以上になります。
初学者の方にとっては非常にためになるTipsが盛り込まれていて、指摘も的確なためTwitterもバズったというのがよくわかりますね。
レビューアーの心がけ
レビューアーの心がけとして気を付けたいのは、以下です。
- 感情を出さない
- アドバイスを書くと尚良い
感情を出さない
怒ってしまっては負けです。淡々と機械のように指摘するのが相手に不快に思われないレビューです。
相手に不快に思われないようにしなければいけないのか？という反論があるかもしれませんが、会社という組織で働く以上、相手に不快に思われてない方が何かと得です。
アドバイスを書くと尚良い
指摘されただけだと、初学者の方はそこから行動することができなかったり、「じゃどうするんだ？」ということにもなりえます。
そのためレビューに一言、参考の記事を入れたりすることもいいと思います。
そうでないと、直接聞きに来たりしてしまうのでレビュアーとしても手間が省けますよね。
感情を出したレビューをするとどうなるか
感情を出したレビューをしてしまうとこういうことが起きます。
感情を出したレビューをする
↓
レビュイーは嫌な気持ちになる
↓
萎えてしまいレビューを受けるのを怖がって出さなくなる
↓
公開後にミス発覚
↓
案件炎上
↓
売り上げ低下
極端な例ですが、そうならないとも限りません。
またコミュニケーションもうまくいかなくなることや精神的にまいってしまうことも考えられますよね。
会社でしたらプラスな空気を入れることが、売上の上昇にもつながるのではないでしょうか。
まとめ：レビューに優しさを加えよう
考えたらこのレビュー書いた方も公開されると思ってないわけで、それをレビューされるのもきつい話ですよね。。すみません。
ただ要するにレビューにちょっと優しさを加えるといいんじゃないかなという結論です。
自分の場合、語尾に～した方がいいかもですとか柔らかクッション敷きます。
もちろんすごい言われ方はいっぱいしてきましたけどねｗ
とはいえ同じ思いをさせる必要はないと思います。
一方でそれもあって頑張れたというのもあるかもしれないので一概には言えませんが。
レビュアー、レビュイーの方に参考になれば幸いです。
コメント
[…] […]