Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions login.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
</a>
<form>
<label for="email">이메일</label>
<input id="email" name="email" type="email">
<input id="email" name="email" type="email" placeholder="이메일을 입력해주세요">
<label for="password">비밀번호</label>
<div class="password-container">
<input id="password" name="password" type="password">
<input id="password" name="password" type="password" placeholder="비밀번호를 입력해주세요">
<button class="eye-btn" type="button"><img src="images/ic_eye_invisible.svg"></button>
</div>
<button class="complete-btn" type="submit">로그인</button>
<button class="complete-btn" type="button" onclick="location.href='/items.html'" disabled>로그인</button>
</form>
<div class="simple-login">
간편 로그인하기
Expand All @@ -38,5 +38,6 @@
판다마켓이 처음이신가요? <a href="/signup.html">회원가입</a>
</div>
</main>
<script src="scripts/auth.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안

아마 script 태그를 HTML의 하단에 배치하신 이유가 script가 문서의 렌더링을 막지 않도록 하기 위해서이실 것 같아요.
하지만, script 태그에 defer나 async 속성을 사용하면 이런 문제를 해결할 수 있기 때문에 반드시 하단에 배치할 필요는 없습니다!
또한 script 태그는 상단에 있는게 구조 파악에서도 유리하기 때문에 상단 head 태그에 두시는 것을 추천드려요~

script async
script defer

지금과 같은 경우 DOM을 조작하는 JS 이니 defer 속성을 추가하시면 되겠습니다~

</body>
</html>
106 changes: 106 additions & 0 deletions scripts/auth.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
한 파일에서 로그인, 회원가입 공용으로 JS를 작성해주셨네요~
둘이 공통되는 부분이 많이 이렇게 해주신 것 같아요.
다만 그렇게 되면 코드가 복잡해져서 유지보수성이 떨어지고 로그인 페이지의 경우 불필요한 코드까지 알아야합니다~
가능하면 공통 로직들은 분리해주시고 각 페이지별 JS를 따로 만드시는 것을 추천드려요!

Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
const form = document.querySelector("form");
const inputs = document.querySelectorAll("form input");
const button = document.querySelector("button.complete-btn");

const email = document.querySelector("input#email");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
email이라는 변수명은 변수의 타입을 알기 어려우므로 emailInput 같이 더 해당값을 잘 설명하는 이름으로 바꿔주시는 것을 추천드립니다.

const nickname = document.querySelector("input#nickname");
const password = document.querySelector("input#password");
const passwordCheck = document.querySelector("input#password-check");

const passwordContainers = document.querySelectorAll(".password-container");

function wrongInput(node, text){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
해당 파일의 이름들을 보면서 주영님이 고민하시고 네이밍을 하셨다는것이 느껴집니다~
다만 일반적으로 함수명은 동사 + 대상의 방식으로 이루어집니다. 해당 함수명은 에러 메시지를 나오게 하는 함수의 동작과는 잘 어울리지 않는 것 같아요.
showErrorMessage나 validateInput과 같은 이름을 추천드려요!

node.classList.add("wrong");
node.classList.remove("correct");

let wrongMessage;
if (node.nextElementSibling?.tagName === 'DIV'){
wrongMessage = node.nextElementSibling;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
이렇게 접근하시는 것도 나쁘지 않지만 이는 HTML 구조와 긴밀하게 연관되게 되므로,
HTML에서 위치만 변경되어도 작동되지 않을 위험이 커집니다~
가능하면 부모요소로 접근해 해당 요소 내에서 재탐색을 해보세요.

} else {
wrongMessage = document.createElement('div');
wrongMessage.setAttribute("class","wrong-message");
}
Comment on lines +16 to +22
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
로직이 복잡한 부분이 있다면 함수로 분리하시면 코드가 더 명확해집니다.

function getErrorMessageTag(node) {
    if (node.nextElementSibling?.tagName === 'DIV')  return node.nextElementSibling;
    
    const errorMessage = document.createElement('div');
    errorMessage.setAttribute("class", "wrong-message");
    
    return errorMessage;
}

function wrongInput(node, text) {
    node.classList.add("wrong");
    node.classList.remove("correct");

    const errorMessage = getErrorMessageTag(node);
    errorMessage.textContent = text;

    if (!node.nextElementSibling?.isSameNode(errorMessage)) {
        node.after(errorMessage);
    }
}

wrongMessage.textContent = text;
node.after(wrongMessage);
}

function correctInput(node) {
node.classList.add("correct");
node.classList.remove("wrong");

if (node.nextElementSibling?.tagName === 'DIV'){
node.nextElementSibling.remove();
}
}

function passwordMatch() {
if (password.value !== passwordCheck.value) {
wrongInput(passwordCheck, "비밀번호가 일치하지 않습니다.");
} else {
correctInput(passwordCheck);
}
}

email.addEventListener("focusout", (e) => {
if (!e.target.value) {
wrongInput(e.target, "이메일을 입력해주세요.");
} else if (!e.target.validity.valid) {
wrongInput(e.target, "잘못된 이메일 형식입니다.");
} else {
correctInput(e.target);
}
});

nickname?.addEventListener("focusout", (e) => {
if (!e.target.value) {
wrongInput(e.target, "닉네임을 입력해주세요.");
} else {
correctInput(e.target);
}
});

password.addEventListener("focusout", (e) => {
if (!e.target.value) {
wrongInput(e.target, "비밀번호를 입력해주세요.");
} else if (e.target.value.length < 8) {
wrongInput(e.target, "비밀번호를 8자 이상 입력해주세요.");
} else {
correctInput(e.target);
}
});

if (passwordCheck) {
password.addEventListener("change", () => {
if (passwordCheck.value) {
passwordMatch();
}
});
passwordCheck.addEventListener("input", () => {
if (password.value) {
passwordMatch();
}
});
}

form.addEventListener("focusout", () => {
for (let input of inputs) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
for of 문을 쓰실때 재할당하는 것이 아니라면 const 로 선언해주세요!

Suggested change
for (let input of inputs) {
for (const input of inputs) {

if (!input.classList.contains("correct")){
return;
}
}
button.removeAttribute("disabled");
});

passwordContainers.forEach( (node) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ 수정요청

Suggested change
passwordContainers.forEach( (node) => {
passwordContainers.forEach((node) => {

node.addEventListener("click", (e) => {
if (e.target.tagName === "IMG") {
if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){
Comment on lines +95 to +97
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 제안
if 문이 중첩되면 가독성이 떨어지므로 아래처럼 early return문을 사용하시는 것을 추천드려요.

Suggested change
node.addEventListener("click", (e) => {
if (e.target.tagName === "IMG") {
if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){
node.addEventListener("click", (e) => {
if (e.target.tagName !== "IMG") return;
if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){

e.currentTarget.firstElementChild.setAttribute("type", "text");
e.target.setAttribute("src", "images/ic_eye_visible.svg");
} else {
e.currentTarget.firstElementChild.setAttribute("type", "password");
e.target.setAttribute("src", "images/ic_eye_invisible.svg");
}
}
});
} )
11 changes: 6 additions & 5 deletions signup.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@
</a>
<form>
<label for="email">이메일</label>
<input id="email" name="email" type="email">
<input id="email" name="email" type="email" placeholder="이메일을 입력해주세요">
<label for="nickname">닉네임</label>
<input id="nickname" name="nickname">
<input id="nickname" name="nickname" placeholder="닉네임을 입력해주세요">
<label for="password">비밀번호</label>
<div class="password-container">
<input id="password" name="password" type="password">
<input id="password" name="password" type="password" placeholder="비밀번호를 입력해주세요">
<button class="eye-btn" type="button"><img src="images/ic_eye_invisible.svg"></button>
</div>
<label for="password-check">비밀번호 확인</label>
<div class="password-container">
<input id="password-check" name="password-check" type="password">
<input id="password-check" name="password-check" type="password" placeholder="비밀번호를 다시 한 번 입력해주세요">
<button class="eye-btn" type="button"><img src="images/ic_eye_invisible.svg"></button>
</div>
<button class="complete-btn" type="submit">회원가입</button>
<button class="complete-btn" type="button" onclick="location.href='/login.html'" disabled>회원가입</button>
</form>
<div class="simple-login">
간편 로그인하기
Expand All @@ -45,5 +45,6 @@
이미 회원이신가요? <a href="/login.html">로그인</a>
</div>
</main>
<script src="scripts/auth.js"></script>
</body>
</html>
28 changes: 26 additions & 2 deletions styles/auth.css
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ form label {

form input {
width: 100%;
padding: 16px 24px 14px;
padding: 15px 24px ;
margin-bottom: 24px;
border-radius: 12px;
background-color: var(--gray100);
Expand All @@ -46,6 +46,27 @@ form input {
line-height: 26px;
}

form input.wrong {
margin-bottom: 8px;
border: solid 1px var(--red);
}

form input.correct {
border: solid 1px var(--blue);
}

form input::placeholder {
color: var(--gray400);
}

form .wrong-message {
margin: 8px 16px 18px;
color: var(--red);
font-weight: 600;
font-size: 14px;
line-height: 24px;
}

form .password-container {
position: relative;
justify-content: center;
Expand All @@ -56,7 +77,6 @@ form .password-container .eye-btn {
padding: 0;
position: absolute;
top: 16px;
bottom: 16px;
right: 24px;
}

Expand All @@ -75,6 +95,10 @@ form .complete-btn {
line-height: 32px;
}

form .complete-btn:disabled {
background-color: var(--gray400);
}

.simple-login {
width: 100%;
padding: 16px 23px;
Expand Down
19 changes: 10 additions & 9 deletions styles/color.css
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
:root {
--blue: #3692FF;
--gray900: #111827 ;
--gray800: #1F2937 ;
--gray700: #374151 ;
--gray600: #4B5563 ;
--gray500: #6B7280 ;
--gray400: #9CA3AF ;
--gray200: #E5E7EB ;
--gray100: #F3F4F6 ;
--gray50: #F9FAFB ;
--red: #F74747;
--gray900: #111827;
--gray800: #1F2937;
--gray700: #374151;
--gray600: #4B5563;
--gray500: #6B7280;
--gray400: #9CA3AF;
--gray200: #E5E7EB;
--gray100: #F3F4F6;
--gray50: #F9FAFB;
}
Loading
Loading