---
# System prepended metadata

title: C_T6 code review
tags: [CodeReview]

---

[TOC]
## Code review #1 by Pam

### header & main
* 我設定了3個斷點等於有4個版，但實際上接案時可能只會有1個斷點2個版，所以不會要求在每一個尺寸都看起來很順眼
* `<div>`沒有包東西只用來設定background有點怪
    * 可以用background: linear-gradient設定上下的黑色區塊
    * 可以使用偽元素固定background
```!
* 測試後發現因為background在body的特例，無法做到用偽元素固定background加上在body設定黑色區塊
* 所以黑色區塊還是要用額外的<div>設定
```
* 可以參考Bootstrap使用不同class的組合去控制某一個樣式
* 不要有元素流浪在`<header>`、`<main>`或`<footer>`外面
```!
* 測試後發現因為nav後面的區塊背景的問題，沒辦法把nav和背景區塊放入main
```
* 用不到的class要清乾淨
    * 例如在`_reset.scss`已經設定`h1 { margin: 0 }`，就不用在`<h1>`設定`margin:0`
* 如果只有一個元素要排列，就不要用`flex`，可以用`text-align`等其他方式排列
* 小版時banner要有一定的高度
```
min-height: 50vh;
```
* 小版時banner圖片看不到人物，可以調整圖片位置
```
object-position: right;
```
* 同樣的東西要有統一的排版方式，例如`<h2>`統一使用`margin-bottom`，而不是有的用`flex`有的用`margin`
* 要試著能夠講出自己設計的邏輯，例如圖片是固定寬，而文字的寬度則隨尺寸大小變化，才不會不知道自己在幹麻

### footer
* z-index, stacking context理解
> 可看[MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context#see_also)的三個example
* 建立自己使用flex的原則，什麼時候用，什麼時候自己用原生推
* 命名要做到看到名字就能在腦海中知道是指畫面上的哪個東西，才不會需要一直切換頁面

### code
* 變數的命名要直接命名內容，要做到看到名字就知道是什麼
```css!
/* 我現在的寫法 */
$feature_icon_1: url("../images/p1.png");
$feature_icon_2: url("../images/p2.png");
$feature_icon_3: url("../images/p3.png");
$feature_icon_4: url("../images/p4.png");

/* 改成不需要還要點開原圖才知道是什麼的命名 */
$feature_icon_growth: url("../images/p1.png");
$feature_icon_support: url("../images/p2.png");
$feature_icon_programs: url("../images/p3.png");
$feature_icon_services: url("../images/p4.png");
```
* 由小切到大時會設定這樣的media queries`@media screen and (min-width: 576px){}`，意思是如果我要列印的話會變成印出手機版的樣子，但通常用A4列印應該會想要印成桌機版，所以可以把screen這個條件去除`@media (min-width: 576px){}`

## Code review #2 by 阿傑

### 畫面
* 搜尋框框在邊界上，可以往左移一點
* navbar後面的背景像素很低，檢查一下
```
好像沒有問題XD
```

### code
* component直接放在layout裡面引用，目前是放在`main.scss`才引用，這樣的作法比較不直覺

#### _base.scss
* @use 先不要用 as [簡寫]，取用變數時直接用那個檔案的名稱，以免檔案多的時候會混淆

#### _typography.scss
* 不會針對tag直接寫樣式，可以寫成utility用class控制(像Bootstrap那樣)，或是寫成mixin
* 不一定要按照7-1 pattern的分類法

```css!
/* 目前是這樣寫 */
h1,
h2,
h3,
h4 {
  text-transform: uppercase;
}

/* 推薦以下的寫法 */
.text-upper {
  text-transform: uppercase;
}

@mixin display-xl() {
    font-size: 25px;
    font-weight: 500;
    ...
}
.card__title {
  @include display-xl()
  text-transform: uppercase;
}

.box__title {
  @include display-xl()
}
```

#### _variables.scss
* 顏色可以分成另一個檔案，不然$本身就是variable的意思，感覺很多餘

```css    
/* 我現在的寫法 */
background-color: variables.$layout_white;

/* 可以改成 */
background-color: colors.$layout_white;
```

#### _mixins.scss
* 命名不要把單字連在一起，例如respondto應改為respond-to或respond_to (可以用vscode的套件code spell check檢查)
* respond_to不同斷點單獨寫出來比較符合實務上的用法，或者把變數寫出來也可以

```scss!
/* 我現在的寫法 */
@mixin respondto($breakpoints) {
  @media screen and (min-width: $breakpoints) {
    @content;
  }
}

/* 可以改成 */
$sm: 576px;

@mixin respondto($breakpoints) {
  @media screen and (min-width: $breakpoints) {
    @content;
  }
}

@include respondto($sm);

/* 或改成 */
@mixin respond-to-sm {
  @media screen and (min-width: 576px) {
    @content;
  }
}

@include respond-to-sm;
```
* `@mixin`不一定要都放在`_mixins.css`裡面，以複用性和好閱讀為準，例如我的navhover就可以放在header就好
 
#### _slick.scss
* 目前寫得有點散亂在各處，可以多利用&來集中
```scss!
/* 改成 */
.slick {
    ...
    &-prev {
        @extend .slick;
        ...
    }
    
    &-next {
        @extend .slick;
        ...
    }
}
```

#### layout/
* 不要直接在tag上做樣式，可以給他class，例如body改成.body，也利於結合BEM
* 使用後代選擇器例如:first-child會使結構巢狀化，會依賴父層，非必要盡量不要使用
* 都一樣的樣式可以寫成變數，例如box-shadow，不過確實很難一開始就知道哪些樣式會複用，也是會有寫到一半再把樣式獨立出來的情況
* section太大塊，變成底下所有元素都要在section下面
```css!
/* 目前 */
.section__features

/* 改為 */
.features
```
```css!
/* 目前用後代選擇器控制後代裡的樣式 */
> :nth-child(1) .section__icon {
    background-image: image.$feature_icon_1;
}

> :nth-child(2) .section__icon {
    background-image: image.$feature_icon_2;
}

> :nth-child(3) .section__icon {
    background-image: image.$feature_icon_3;
}

> :nth-child(4) .section__icon {
    background-image: image.$feature_icon_4;
}

/* 改為 */
icon {
    &--growth {
        background-image: image.$feature_icon_1;
    }
    &--support {
        background-image: image.$feature_icon_2;
    }
}
```

* %通常會放在檔案最前面
* sass的color function例如lignten，設計稿沒有定義顏色的時候才可以用，不然沒辦法保證使用這些function就可以調整到特定的顏色

#### 額外補充(等進js再查)
* defer

## Code review #final by Chris

### 畫面
* hover effect: 離開hover時，文字要先離開，黑色區塊後離開，沒有實作到

### 問題
1. 改動Bootstrap的樣式要怎麼寫?
> 不會額外用vendors_extensions/放修改後的樣式，

2. typography, tag統一的樣式要怎麼寫? 寫在base可以嗎?
> 業主會說，h1就要長這樣！那直接修改tag的樣式來追求整體設計感就是無可避免的事情 (Bootstrap則是tag和class都有做)，而且如果要微調的話，可以另外寫成class，用class來壓過tag的權重也比較方便。

### Sass
* 不要用語法分檔，例如_mixins.scss, _placeholder.scss，而是要把每段code放在會用到的情境內，例如respond_to和斷點變數放一起，typography和font變數放一起
```sass
// 原本的拆檔
_breakpoints.scss
$sm: 576px;
$md: 768px;
$lg: 992px;    

_mixins.scss
@mixin respond_to($breakpoints) {
  @media (min-width: $breakpoints) {
    @content;
  }
}

// 建議的拆檔
_breakpoints.scss
$sm: 576px;
$md: 768px;
$lg: 992px;    

@mixin respond_to($breakpoints) {
  @media (min-width: $breakpoints) {
    @content;
  }
}
```


* 在會變化樣式 (例如hover、media queries) 的前後放預設值和更改後的值

```sass
// 原本的寫法，改到什麼才寫什麼進去
.navbar__container {
  background-color: colors.$layout_white;
  width: 100%;

  @include mixins.respond_to(breakpoints.$md) {
    box-shadow: 1px -2px 4px -3px colors.$layout_black;
  }

  @include mixins.respond_to(breakpoints.$lg) {
    margin-right: 7px;
  }
}

// 建議的寫法，有改到的東西在改前改後都要寫一份
.navbar__container {
  background-color: colors.$layout_white;
  width: 100%;

  box-shadow: none;
  margin-left: 0;

  @include mixins.respond_to(breakpoints.$md) {
    box-shadow: 1px -2px 4px -3px colors.$layout_black;
    margin-left: 0;
  }

  @include mixins.respond_to(breakpoints.$lg) {
    box-shadow: 1px -2px 4px -3px colors.$layout_black;
    margin-right: 7px;
  }
}
```
* 樣式中可以用空行去隔開不同功能的語法，甚至放在兩個分開的class

```sass
// 原本的寫法，單純按照字母順序排列
.card-of-feature {
  align-items: center;
  background-color: colors.$primary_gray;
  display: flex;
  flex-direction: column;
  gap: 24px;
  height: 100%;
  padding: 35px 30px 24px;
  text-align: center;
  transition: all 0.2s;
}  

// 建議的寫法，類似功能的放一起，中間用分行隔開，甚至寫在兩個class
.card-of-feature {
  transition: all 0.2s;
}

.card-of-feature {
  display: flex;
  flex-direction: column;
  align-items: center;
  gap: 24px;

  height: 100%;
  padding: 35px 30px 24px;

  text-align: center;

  background-color: colors.$primary_gray;
}  
```

### 其他
* preprocessor: macro (中國翻成宏替換)，利用有特徵的關鍵字做字串取代
> [wiki](https://zh.wikipedia.org/zh-tw/%E9%A2%84%E5%A4%84%E7%90%86%E5%99%A8)
* compiler: 即程式語言，設計出同一套規則讓所有人的電腦都可以看得懂。在瀏覽器則叫render "渲染"。
> 一個現代編譯器的主要工作流程如下：
>
> 原始碼（source code）→ 預處理器（preprocessor）→ 編譯器（compiler）→ 組譯程式（assembler）→ 目的碼（object code）→ 連結器（linker）→ 執行檔（executables）
> [wiki](https://zh.wikipedia.org/wiki/%E7%B7%A8%E8%AD%AF%E5%99%A8)

> 其他補充：abstract syntax tree (AST)
* 要理解專有名詞，目的是之後遇到很多套件、工具時才能理解他到底是在處理什麼事情、服務對象是誰
* 由於資安問題，利用除錯工具、開發者工具能看到的額外資訊，在公開部署時都需要刪除，例如.map檔案可以看見你是用Sass寫css，公開部署時就要刪掉
* Sass的function等其他功能等到學會JS後再自然而然地去學習需要的功能就好