昔自分が作ったサイトについてレビューしてみる

はじめに

東京理科大学 Advent Calendar 2017 20日目の記事です。
19日目は@motoakim さんの記事でした。21日目は@fujitora さんの記事です。

2016年8月、HTML/CSS初心者だった私は、あるHPの実装を担当することになりました。
その時に実装したページがこちらです。

HTML/CSS経験値が溜まってきたので今回、昔自分が書いたコードのレビューをしてみます。

一番大きな間違い

当時は「position relative/absoluteって要素を自由自在に配置できてすごいな！！！！」とpositionのすごさに感動していた時期でした。
これがあればなんでも実装できるな、と思うくらいpositionを信頼していたので、
だいたいの要素をpositionを使って配置をしました。
(さらに、最初レスポンシブ対応のことは全く考えずに実装していたのでレスポンシブなサイトにしようとした時に死にました。)

positionを使いすぎるのはダメ。何事も適度な量が大事。

レビュー1　単位とcssの指定の仕方が良くない

まずheaderの下の写真部分について。
ここはjqueryを用いてスライドショーにしています。
smokeクラスをつけた要素を用いて、白いもやをかけています

コードはこちら

about.html

<div class="photo">
    <div class="smoke"></div>
    <h1>大学に新しい文化を作る</h1>
</div>

about.css

# about .photo{
    position: relative;
    width: 100%;
    height: 60vh;//(1)
    background-color: white;
    background-image: url("../img/about/01.jpg");//(2)
    background-position: 50% -134px;//(3)
    background-size: cover;
    background-attachment: fixed;
    transition: background-image 1s linear;
  }

  #about .smoke{
    position: absolute;
    top: 0;
    width: 100%;
    height: 100%;
    background-color: white;
    opacity: 0.4;
  }

@media screen and (max-width: 999px) {
  #about .photo{
    padding-top: 45%;//(1)
  }

  #about .photo h1{
    position: absolute;
    top: 68%;
    right: 5%;
    font-size: 250%;
    color: #594e52;
    z-index: 1;
  }
}

(1) padding-topとheightについて指定しているのが、重複している。どちらかは消すべき。

(2) jQueryで

$(".photo").css("background-image","(次の画像のurl)")

の処理をしているのでHTMLに直書きする方が良さそう。

(3) 高さをpxでとっていない(height: 60vh;//(1))のでpxでない方がよい。

レビュー2　positionをここで使うのはよくない。

ここで言えることはただ一つ。
タイトルの「About Unitus」にあたっているCSSをみると...

# about .title{
    position: absolute;//※
    font-size: 25px;
    border-radius: 3%;
    top: 9%;//※
    width: 100%;
    text-align: center;
  }

位置調整に※を使っている。marginを使った方が良いです。
文章の位置調整も同じことをやっていたので省略。

レビュー3　中央ぞろえしよ...

ここはbootstrap3を用いてリファクタしてありました。良さそう。
ただ、縦方向に中央ぞろえできていないですね。当時だったら

display: table-cell;
vertical-align: middle;

を使えばできたんだと思うけどどうして断念してしまったのだろうか...
今ならFlexboxがあるし、bootstrap4も縦方向の位置揃えができるようになったのでより簡単に実装できますね！

レビュー4　colとpositionとpaddingが混ざりに混ざってデザインが崩れる

画面が大きい時はそんなに気にならないけれど...

画面を小さくするとコンテンツが右寄りになっている印象を感じます。(下の写真は横幅を縮小した時の様子)

コードを見てみましょう。

about.html

<div class="container" style="margin-bottom: 8%;"><!-- (1) -->
  <div style="font-size: 35px; margin: 10% 0 10%; text-align: center; overflow: hidden;"><span class="i">M</span>embers</div>
  <div class="col-xs-6 hidden-xs">
    <div class = "box">
      (六角形に関するコード)
    </div>
  </div>
  <div class="col-xs-3 sm-xs-content">
      <ul>
        <li class="lititle">学部生</li>
        <li>2年　　26人</li><!-- (2) --> 
        <li>3年　　3人</li>
        <li>4年　　2人</li>
        <li class="lititle">卒業生</li>
        <li>11人</li>
      </ul>
  </div>
  <div class="col-xs-3 sm-xs-content">
      <ul>
        <li class="lititle">所属学部</li>
        <li>理学部</li>
        <li>工学部</li>
        <li>理工学部</li>
        <li>経営学部</li>
        <li>経営学部(立命館大学)</li>
      </ul>
    </div>
</div>

まず悪さをしているのがこのスタイル。

# about .box{
    position: absolute;//※はみ出し
    overflow: hidden;
    width: 26vw;
    padding-top: 28.36vw;
    top: 24%;
    left: 28%;
}

せっかくBootstrap3のcolを用いて位置調整をしているのに、
さらにtop、leftを用いて位置調整をしています。
もうちょっと右下に寄って欲しい...という気持ちが伝わってきます。
この指定の仕方だと要素の位置が歪になるから

margin: 0 auto;

を使ったらよかったんじゃないかな。

悪さをしている2つめの原因は標準でulタグについているpadding。
中央寄せのデザインなのにulタグのpaddingを残しておくのはおかしいですね。

以上の情報をまとめると、だいたいこんな感じ。

(col自体に当たっているpaddingなどは省略)
containerから.boxがはみ出しているのはcssの(※はみ出し)のように、六角形を

position: absolute;

で位置調整しているのが原因です。
この影響で.col-sm-6の要素の高さがなくなっています。

今までの修正点を反映させるとこんな感じになります。

終わりに

positionを使いすぎるのはダメ、ということを最初に言ったので
positionはどんな時に使ったらいいのか書こうと思いましたが力尽きました。
いつか書こうかなーと思います。

昔書いたコードをレビューするのは、当時どう考えてコーディングしていたかを思い出せる点と、自分自身の成長を感じられる点で良いです。
皆さんも、昔書いたコードをレビューしてみるのはいかがでしょうか？