Login page UI / code review
Published: 2021-11-18 14:59:40

I asked for code review for the first time and I'd like to write about what I heard back.
[Login UI](https://daaahailey.github.io/likelion/assignments/005/index.html)

#1) Don't write unnecessary code.
I didn't need <code><strong>div class="first-page-container"</strong></code> here. I thought I needed it for styling but I could've given the style to body instead since I already had flex property inside body. <strong><u>*I should try to write html efficiently.</u></strong>
This was my code below.
```html
<body>
    <div class="first-page-container">
        <section class="login-first-page">
            <h1 class="sr-only">위니브 로그인 페이지</h1>
            <img class="logo-img" src="src/img/weniv.png" alt="logo">
            <p>위니브를 더 즐겁고 편리하게 이용하세요.</p>
            <button id="goto-login-btn" type="button" onclick="window.location.href='login.html'">로그인</button>
        </section>
    </div>
</body>
```
This was my css
```css
body {
  position: relative;
  box-sizing: border-box;
  display: flex;
  justify-content: center;
  font-family: "Spoqa Han Sans Neo", "sans-serif";
  background-color: #f2f2f2;
}

.first-page-container {
  display: flex;
  text-align: center;
  align-items: center;
  height: 100vh;
}
```
I removed <code><strong>div class="first-page-container"</strong></code> from my html, removed <code><strong>.first-page-container</strong></code> and added this two lines to the body instead.
```css
text-align: center;
align-items: center;
```

#2) Give clear information when writing img's alt value.
```html
<img class="logo-img" src="src/img/weniv.png" alt="logo">
```
I only wrote it as logo but it might be helpful if it <em><strong>explains about the image clearly</strong></em> like "weniv company logo" for example.


#3) Write clean code!
```html
<body>
    <div class="first-page-container">
        <section class="login-first-page">
            <h1 class="sr-only">위니브 로그인 페이지</h1>
            <img class="logo-img" src="src/img/weniv.png" alt="company logo">
            <p>위니브를 더 즐겁고 편리하게 이용하세요.</p>
            <button id="goto-login-btn" type="button" onclick="window.location.href='login.html'">로그인</button>
        </section>
    </div>
</body>
```
I forgot to remove <code><strong>id="goto-login-btn"</strong></code> because I was going to make function using the id but instead I ended up putting onclick inline but forgot to remove id that I never used.

#4) If button doesn't have <u>width or height</u>, screen reader wouldn't recognise the button and wouldn't read it.
```html
 <button class="close-btn">
  <img class="close-page-icon" src="src/img/close.png" alt="로그인 창닫기 아이콘">
</button>
```
```css
main.login-container button.close-btn {
  position: absolute;
  border: none;
  background: transparent;
  height: 0;
  top: 18px;
  right: 24px;
}

main.login-container button.close-btn img.close-page-icon {
  top: 0;
  left: 0;
  width: 16px;
  height: 16px;
}
```
I wrote the <code>height:0</code> because I wanted to put replacement image instead of normal button being there and I thought I had to hide the button but my image is actually inside the button tag😅 So giving height to button doesn't make difference. It doesn't display the ugly default button style anyways since I gave these properties & values below.
```css
border: none;
background: transparent;
```

So, I gave height and width to button. Also, I forgot to add button type and had to add <strong><code>type="button"</code></strong> as well.


```html
<button type="button" class="close-btn">
  <img class="close-page-icon" src="src/img/close.png" alt="로그인 창닫기 아이콘">
</button>
```
```css
main.login-container button.close-btn {
  position: absolute;
  border: none;
  background: transparent;
  top: 11px;
  right: 17px;
  width: 30px;
  height: 30px;
}
main.login-container button.close-btn img.close-page-icon {
  width: 16px;
}
```

#5) Don't forget to write text inside <code>&lt;label&gt;</code> tag which explains the <code>&lt;input&gt;</code> tag that's linked with.
```html
<label for="user-id"></label>
<input class="login-input" type="text" name="id" id="user-id" placeholder="아이디" required>
<label for="user-pw"></label>
<input class="login-input" type="password" name="pw" id="user-pw" placeholder="비밀번호" required>
```
I thought it would be okay because I added placeholder but there are screen readers doesn't read placeholder. Therefore, label text must be written like below.
```html
<label for="user-id" class="sr-only">아이디</label>
<input class="login-input" type="text" name="id" id="user-id" placeholder="아이디" required>
```
However, the text was written as placeholder inside input and I didn't want the label text to be appear on the top of input field. So, I gave label elements class="sr-only"
```css
.sr-only {
  position: absolute;
  left: -9999px;
  width: 1px;
  height: 1px;
  overflow: hidden;
}
```
This will take the text out of screen but it's still in the html so the screen reader will still read it. (I've wrote about this a few days ago: [Web Accessibility](https://dev.to/daaahailey/web-accessibility-1kfj)