-
Notifications
You must be signed in to change notification settings - Fork 26
[정상인] sprint4 #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "Basic-\uC815\uC0C1\uC778-sprint4"
[정상인] sprint4 #90
Changes from all commits
1ff068b
2bef22b
ce75b2a
96dceda
c2f7650
7e26d87
660a433
4bea80a
6becdbe
014d9e9
9b5ebb3
8391df1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||
| .login-btn { | ||||||
| .go-login-btn { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (심화/생각해보기/제안)
|
||||||
| .go-login-btn { | |
| <a href="/login.html" class="button button-primary">로그인 </a> |
만약, 현재 button-login과 같은 스타일의 "장바구니에 담기" 라는 버튼이 존재한다면 클래스 이름이 어떻게 될까요?
button-login 클래스로 작성한다면 login이라는 "기능"을 목적으로 하는 클래스 이름을 넣게 될거예요. 혹은 중복된 스타일과 다른 클래스 이름인 button-add-wish와 같은 클래스 이름을 가지게 돼요. 이는 재사용성을 저하시킬거예요.
차라리 해당 클래스가 "어떤 스타일을 포함하는가?"에 목적을 두어 네이밍을 한다면 더욱 재사용하기 용이할거라고 생각해서 제안드립니다 😊
priamry: 주로 브랜드의 주요 컬러를 의미합니다 ! 😊 판다마켓의 경우 파란색이죠 !
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 재사용성을 고려하신 설계가 정~말 좋습니다 👍 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,33 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const isValidValue = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (권장)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const isValidValue = { | |
| export const formValidState = { |
하지만 값은 객체로 작성되어있군요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(심화/생각해보기) 구조가 정말 좋아요. 스키마로 거듭날 수도 있겠군요 !
문제 해결 방법에 대한 접근이 정말 좋아요 !
다음과 같이 스키마 형태로 발전시킬 수도 있겠네요 !:
| export const isValidValue = { | |
| login: { | |
| loginEmail: false, | |
| loginPassword: false, | |
| }, | |
| signup: { | |
| signupEmail: false, | |
| signupNickname: false, | |
| signupPassword: false, | |
| signupCheckPassword: false, | |
| }, | |
| }; | |
| export const signupSchema = { | |
| email: { | |
| isValid: false, | |
| validate: (value) => /\S+@\S+\.\S+/.test(value), | |
| message: { | |
| void: "이메일을 입력해주세요.", | |
| invalid: "잘못된 이메일 형식입니다.", | |
| }, | |
| }, | |
| nickname: { | |
| isValid: false, | |
| validate: (value) => value.trim().length > 0, | |
| message: { | |
| void: "닉네임을 입력해주세요.", | |
| }, | |
| }, | |
| password: { | |
| isValid: false, | |
| validate: (value) => value.length >= 8, | |
| message: { | |
| void: "비밀번호를 입력해주세요.", | |
| invalid: "8자 이상 입력해주세요.", | |
| }, | |
| }, | |
| checkPassword: { | |
| isValid: false, | |
| validate: (value, original) => value === original, | |
| message: { | |
| invalid: "비밀번호가 일치하지 않습니다.", | |
| }, | |
| }, | |
| }; |
상인님께서는 제가 알기로 리액트도 어느 정도 해보셨다고 알고 있어서 조금은 심화된 피드백을 드려봅니다. 😉
나중에는 zod나 joi같은 유효성 검사 라이브러리도 사용해보실 수도 있습니다 ! 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
크으 ~ 함수들이 하나같이 외부 환경에 영향을 주지 않는군요 !
'인자로 받은 것들만을 처리' 하며, 외부에 무언가 영향을 주는 함수가 없네요 👍
상인님께서 작성하신 이러한 함수를 “순수 함수(Pure Function)” 라고 부릅니다.
이러한 구조는 테스트하기도 쉽고, 예측 가능한 코드 작성에 도움이 되는 좋은 패턴입니다.
지금처럼 유틸리티 함수를 순수하게 분리해 작성하는 습관, 정말 잘 하고 계세요! 🔥
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,26 +19,37 @@ | |
| <img class="form-logo" src="../../../images/logo.png" alt="logo" /> | ||
| </a> | ||
| </div> | ||
| <form class="login-signup-form"> | ||
| <form class="login-signup-form" id="login-form"> | ||
| <label class="form-label">이메일</label> | ||
| <input | ||
| class="form-input" | ||
| type="email" | ||
| placeholder="이메일을 입력해주세요" | ||
| /> | ||
| <div class="form-input-area"> | ||
| <input | ||
| id="login-email" | ||
| class="form-input" | ||
| type="email" | ||
| placeholder="이메일을 입력해주세요" | ||
| /> | ||
| </div> | ||
| <label class="form-label">비밀번호</label> | ||
| <div class="password-input-area"> | ||
| <div class="form-input-area"> | ||
| <input | ||
| id="login-password" | ||
| class="form-input" | ||
| type="password" | ||
| style="width: 100%" | ||
| placeholder="비밀번호를 입력해주세요" | ||
| minlength="8" | ||
| /> | ||
| <button type="button" class="visibility-img"> | ||
| <img src="../../../images/visibility-icon.svg" alt="visibility" /> | ||
| <button type="button" class="button visibility-img"> | ||
| <img | ||
| src="../../../images/visibility-icon.svg" | ||
| alt="visibility" | ||
| id="password-visibility-img" | ||
| /> | ||
| </button> | ||
| </div> | ||
| <button class="form-submit-btn disabled" disabled>로그인</button> | ||
| <button type="submit" class="button form-btn" id="login-btn" disabled> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굿굿 ~!
|
||
| 로그인 | ||
| </button> | ||
| </form> | ||
|
|
||
| <div class="easy-login-area"> | ||
|
|
@@ -63,5 +74,7 @@ <h3 class="easy-login-title">간편 로그인하기</h3> | |
| </div> | ||
| </div> | ||
| </main> | ||
|
|
||
| <script type="module" src="./login.js"></script> | ||
| </body> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| import { | ||
| clearErrorTag, | ||
| createErrorTag, | ||
| isValidEmail, | ||
| isValidValue, | ||
| } from "../formUtils.js"; | ||
|
|
||
| const $loginForm = document.querySelector("#login-form"); | ||
| const $loginEmail = document.querySelector("#login-email"); | ||
| const $loginPassword = document.querySelector("#login-password"); | ||
| const $loginBtn = document.querySelector("#login-btn"); | ||
| const $visibilityBtn = document.querySelector(".visibility-img"); | ||
|
|
||
| $loginEmail.addEventListener("focusout", (e) => { | ||
| e.preventDefault(); | ||
| clearErrorTag(e.target); | ||
| const { value } = e.target; | ||
| if (!value) { | ||
| const errorTag = createErrorTag(e.target, "이메일을 입력해주세요"); | ||
| $loginBtn.disabled = true; | ||
| return errorTag; | ||
| } | ||
|
|
||
| if (!isValidEmail(value)) { | ||
| const errorTag = createErrorTag(e.target, "잘못된 이메일 형식입니다"); | ||
| $loginBtn.disabled = true; | ||
| return errorTag; | ||
| } | ||
| isValidValue.login.loginEmail = true; | ||
| const checkAll = Object.values(isValidValue.login).every( | ||
| (bool) => bool === true | ||
| ); | ||
| if (checkAll) { | ||
| $loginBtn.disabled = false; | ||
| } | ||
| }); | ||
|
|
||
| $loginPassword.addEventListener("focusout", (e) => { | ||
| e.preventDefault(); | ||
| clearErrorTag(e.target); | ||
| const { value } = e.target; | ||
| if (!value) { | ||
| const errorTag = createErrorTag(e.target, "비밀번호를 입력해주세요"); | ||
| $loginBtn.disabled = true; | ||
| return errorTag; | ||
| } | ||
|
|
||
| if (value.length < 8) { | ||
| const errorTag = createErrorTag( | ||
| e.target, | ||
| "비밀번호를 8자 이상 입력해주세요." | ||
| ); | ||
| $loginBtn.disabled = true; | ||
| return errorTag; | ||
| } | ||
|
Comment on lines
+42
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굿굿 ! Guard Clause 패턴으로 가독성을 향상시켰군요 ! 👍👍Guard Clause 패턴을 사용하고 계시군요 ! 멋집니다 상인님. 또한, 미리 조건에 따라서 데이터의 범위를 좁힐 수 있기에 핵심 로직에서는 보장받은 데이터를 편리하게 사용할 수 있다는 장점도 있어요 😊 Guard Clause ?다음과 같이 조기에 함수를 끝내는 기법을 의미합니다: function calculateDiscount(price, discountRate) {
// Guard Clause: 가격이나 할인율이 유효하지 않으면 함수 종료
if (price <= 0 || discountRate <= 0 || discountRate > 1) {
console.error("Invalid price or discount rate");
return;
}
// 할인된 가격 계산
const discountedPrice = price * (1 - discountRate);
return discountedPrice;
} |
||
| isValidValue.login.loginPassword = true; | ||
| const checkAll = Object.values(isValidValue.login).every( | ||
| (bool) => bool === true | ||
| ); | ||
|
|
||
| if (checkAll) { | ||
| $loginBtn.disabled = false; | ||
| } | ||
| }); | ||
|
|
||
| $loginForm.addEventListener("submit", (e) => { | ||
| e.preventDefault(); | ||
| window.location.href = "/items"; | ||
| }); | ||
|
|
||
| $visibilityBtn.addEventListener("click", () => { | ||
| const $passwordVisibilityImg = document.querySelector( | ||
| "#password-visibility-img" | ||
| ); | ||
| if ($loginPassword.type === "password") { | ||
| $loginPassword.type = "text"; | ||
| $passwordVisibilityImg.src = "../../../images/visibility-on.svg"; | ||
| } else { | ||
| $loginPassword.type = "password"; | ||
| $passwordVisibilityImg.src = "../../../images/visibility-icon.svg"; | ||
| } | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<h1>태그는 문서에 하나만 넣는게 어떨까요?이하 MDN 발췌
페이지 당 하나의
<h1>만 사용하세요. 여러 개를 써도 오류는 나지 않겠지만, 단일<h1>이 모범 사례로 꼽힙니다. 논리적으로 생각했을 때도,<h1>은 가장 중요한 제목이므로 전체 페이지의 목적을 설명해야 할 것입니다. 두 개의 제목을 가진 책이나, 여러 개의 이름을 가진 영화는 볼 수 없죠! 또한 스크린 리더 사용자와 SEO에도 더 적합합니다.