Skip to content

Conversation

@summerDev96
Copy link
Collaborator

요구사항

기본

로그인

  • 이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 "이메일을 입력해주세요." 빨강색 에러 메세지를 보입니다.
  • 이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 "잘못된 이메일 형식입니다" 빨강색 에러 메세지를 보입니다.
  • 비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 "비밀번호를 입력해주세요." 에러 메세지를 보입니다
  • 비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 "비밀번호를 8자 이상 입력해주세요." 에러 메세지를 보입니다.
  • input 에 빈 값이 있거나 에러 메세지가 있으면 '로그인' 버튼은 비활성화 됩니다.
  • input 에 유효한 값을 입력하면 '로그인' 버튼이 활성화 됩니다.
  • 활성화된 '로그인' 버튼을 누르면 "/items" 로 이동합니다

회원가입

  • 이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 "이메일을 입력해주세요." 빨강색 에러 메세지를 보입니다.
  • 이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 "잘못된 이메일 형식입니다" 빨강색 에러 메세지를 보입니다.
  • 닉네임 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 "닉네임을 입력해주세요." 빨강색 에러 메세지를 보입니다.
  • 비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 "비밀번호를 입력해주세요." 에러 메세지를 보입니다
  • 비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 "비밀번호를 8자 이상 입력해주세요." 에러 메세지를 보입니다.
  • 비밀번호 input과 비밀번호 확인 input의 값이 다른 경우, 비밀번호 확인 input 아래에 "비밀번호가 일치하지 않습니다.." 에러 메세지를 보입니다.
  • input 에 빈 값이 있거나 에러 메세지가 있으면 '회원가입' 버튼은 비활성화 됩니다.
  • input 에 유효한 값을 입력하면 '회원가입' 버튼이 활성화 됩니다.
  • 활성화된 '회원가입' 버튼을 누르면 로그인 페이지로 이동합니다

심화

  • 눈 모양 아이콘 클릭시 비밀번호의 문자열이 보이기도 하고, 가려지기도 합니다.
  • 비밀번호의 문자열이 가려질 때는 눈 모양 아이콘에는 사선이 그어져있고, 비밀번호의 문자열이 보일 때는 사선이 없는 눈 모양 아이콘이 보이도록 합니다.

주요 변경사항

  • CSS 파일의 구조를 분리하였습니다.

  • 회원가입, 로그인 페이지의 css가 form container 제외하고 모든 클래스가 동일해서 common.css에 추가하였습니다.

  • CSS 작성 시 관련된 클래스 밑에 미디어 쿼리를 분리해서 배치하였습니다.

배포 링크

https://sprint-misson4-suminbae.netlify.app/

스크린샷

screencapture-sprint-misson4-suminbae-netlify-app-login-2025-04-29-21_16_08

screencapture-sprint-misson4-suminbae-netlify-app-signup-2025-04-29-21_17_22

멘토에게

  • common.css 관련 코드가 아직 긴 것 같은데 flex관련된 코드를 클래스로 빼는 것이 좋을까요?

@summerDev96 summerDev96 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Apr 29, 2025
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
자바스크립트 처음 쓰시는건데 어렵지않게 잘 쓰셨네요.
조금만 더 다듬어봅시다 👍

주요 리뷰 포인트

  • 코드 중복 제거
  • 유지보수를 고려한 개발
  • 이미지 최적화

Comment on lines +161 to +190
.form__container__banner {
background-color: var(--color-background-light-blue);
border-radius: 8px;
padding: 16px 23px;
font-size: var(--font-size-sm);
font-weight: 500;
color: var(--color-secondary-800);
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 24px;
}

.form__container__banner__icon {
display: flex;
gap: 16px;
}

.form__container__signup {
display: flex;
justify-content: center;
gap: 4px;
font-size: var(--font-size-xxs);
font-weight: 500;
color: var(--color-secondary-800);
}

.form__container__signup a {
color: var(--color-primary-100);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

요런건 특정 페이지 (예: signup.html)에서만 쓰이지않을까요? 재사용될필요가 있는지 확인해보고, 없다면 전역 스타일 (공용 스타일)을 처리하는 파일에서는 배제하도록할까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 로그인, 회원가입 공통 CSS 파일 추가하여 사용하려고 합니다

Comment on lines +9 to +23
.header__content {
padding: 0 24px;
max-width: 1520px;
margin: 0 auto;
color: #ffffff;
display: flex;
justify-content: space-between;
align-items: center;
}

@media screen and (min-width: 1200px) {
.header__content {
padding: 0 200px;
}
}
Copy link
Collaborator

@addiescode-sj addiescode-sj May 2, 2025

Choose a reason for hiding this comment

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

NIT: Scss같은 스타일 전처리기를 사용하면 이런식으로 중첩해줘도 되는데, 해당 문법이 바닐라 CSS에서는 유효하지않네요.
나중엔 스타일 전처리기를 사용하게되면 이런 형태로 중첩을 쓸수있다는걸 참고로만 알아두시면 될것같아요!

.header__content {
  padding: 0 24px;
  max-width: 1520px;
  margin: 0 auto;
  color: #ffffff;
  display: flex;
  justify-content: space-between;
  align-items: center;
  
  @media screen and (min-width: 1200px) {
      padding: 0 200px;
  }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SCSS 사용 시 참고하겠습니다

Comment on lines +17 to +20
<link rel="stylesheet" href="css/main.css" />
<link rel="stylesheet" href="css/base/common.css" />
<link rel="stylesheet" href="css/base/reset.css" />
<link rel="stylesheet" href="css/base/variables.css" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

물론 병렬로 로드될테지만 common.css나 main.css에서 variable이 필요하겠죠?
reset.css -> variable.css -> common.css -> main.css 순서로 바꿔주세요! :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

순서 수정하여 반영하겠습니다!

class="header__content__logo"
/></a>
<a href="/">
<picture>
Copy link
Collaborator

Choose a reason for hiding this comment

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

picture태그를 사용하셔도 괜찮지만 뷰포트에 따라 같은 이미지소스를 해상도만 다르게 최적화해서 보여주는 방식은 srcset, sizes 만으로도 충분합니다 :)

덧붙여, 첫 화면에 보이는 이미지가 아닌 스크롤을 쭉 내려야 보이는 이미지라면 레이지로딩을 적용할수도있고요 :)
아래 아티클 참고해보시고 리팩토링해볼까요?

참고

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

srcset, sizes 적용하는 것으로 변경하였습니다!

<!--3-->
<div class="feature__content">
<img src="img/Img_home_03.png" class="feature__content__img" />
<img
Copy link
Collaborator

Choose a reason for hiding this comment

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

특히 여기있는 배너 이미지는 스크롤을 쭉 내려야보일테니 초기말고 나중에 로드될수있도록 loading 속성을 써주시면 좋겠죠?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

하단 배너, 푸터쪽에 이미지 loading 속성 적용하였습니다

Comment on lines +8 to +53
/* 이메일 유효성 체크 */
const emailValidate = () => {
const regex = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/i;
let isValid = false;

if (!emailInput.value) {
emailInputMsg.innerText = "이메일을 입력해주세요";
isValid = false;
} else if (!regex.test(emailInput.value)) {
emailInputMsg.innerText = "잘못된 이메일 형식입니다 ";
isValid = false;
} else {
emailInputMsg.innerText = "";
isValid = true;
}
return isValid;
};

/* 이메일 인풋, 에러 메시지 클래스 변경 */
const updateEmailClass = (result) => {
emailInput.classList.remove("success", "error");
emailInputMsg.classList.remove("error");

if (result) {
emailInput.classList.add("success");
} else {
emailInput.classList.add("error");
emailInputMsg.classList.add("error");
}
};

/* 비밀번호 유효성 체크 */
const passwordValidate = () => {
let isValid = false;
if (!passwordInput.value) {
passwordInputMsg.innerText = "비밀번호를 입력해주세요";
isValid = false;
} else if (passwordInput.value.length < 8) {
passwordInputMsg.innerText = "비밀번호를 8자 이상 입력해주세요";
isValid = false;
} else {
passwordInputMsg.innerText = "";
isValid = true;
}
return isValid;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수 내부 body를 보면 유효성 체크의 대상만 바뀔뿐 똑같은 로직이 반복됩니다.
코드 중복을 줄이려면 이런 방법이 있을수있겠죠?

  • 유효성 검사 로직을 재사용 가능한 단위로 분리해보기
export const validators = {
  email: (value) => {
    if (!value) return { isValid: false, message: "이메일을 입력해주세요" };
    if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) {
      return { isValid: false, message: "올바른 이메일 형식이 아닙니다" };
    }
    return { isValid: true, message: "" };
  },

  password: (value) => {
    if (!value) return { isValid: false, message: "비밀번호를 입력해주세요" };
    if (value.length < 8) {
      return { isValid: false, message: "비밀번호는 8자 이상이어야 합니다" };
    }
    return { isValid: true, message: "" };
  },
};
  • 검사 대상에 따라 따로 함수를 만들지않고, 검사대상을 인자로 전달받아 검사를 수행해주는 구조로 함수 만들어보기
const validateInput = (input, errorMsgElement, validatorType) => {
  const { isValid, message } = validators[validatorType](input.value);
  
  errorMsgElement.innerText = message;
  
  // 스타일 업데이트
  input.classList.remove("success", "error");
  errorMsgElement.classList.remove("error");
  
  if (isValid) {
    input.classList.add("success");
  } else {
    input.classList.add("error");
    errorMsgElement.classList.add("error");
  }
  
  return isValid;
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

전달주신 코드 참고하여 적용하였습니다!

}
};

export default {
Copy link
Collaborator

Choose a reason for hiding this comment

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

위 코멘트대로 수정한다면 여기서 export할 대상도 validators, validateInput 두개만 남게 되겠죠? :)
코드양도 줄어들고, 의도가 명확해질수있습니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

유효성 검사를 공통화하니 코드가 정말 많이 줄어드네요!

Comment on lines +37 to +49
passwordIcon.forEach((icon) => {
icon.addEventListener("click", () => {
let passwordInput = icon.previousElementSibling;
let isShowPassword = passwordInput.type === "text";
passwordInput.type = isShowPassword ? "password" : "text";

icon.src = isShowPassword
? "img/btn_visibility_off.svg"
: "img/btn_visibility_on.svg";

icon.setAttribute("aria-pressed", !isShowPassword);
});
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

몇가지 개선할점이 보이네요!

우선 passwordIcon 변수명이 단수인데, iterator를 쓰고있습니다. 의도를 명확히 반영하려면 복수형태로 바꾸셔야겠죠?
두번째로, 확인해보니 실제로 passwordIcon은 한 페이지당 하나만 있는게 맞습니다. 그러면 이런 형태로 iterator를 사용하지않아도되겠죠?

Copy link
Collaborator

Choose a reason for hiding this comment

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

aria-pressed 속성을 토글해주신건 아주 잘하셨습니다 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

회원가입 페이지에는 passwordIcon이 두개여서 공통 변수를 사용하여 iterator를 사용하고 있습니다.
passwordIcon, passwordIcons 변수 두 가지로 나누는게 좋을까요?

@addiescode-sj
Copy link
Collaborator

질문에 대한 답변

멘토에게

  • common.css 관련 코드가 아직 긴 것 같은데 flex관련된 코드를 클래스로 빼는 것이 좋을까요?

유틸리티 클래스를 추가하기보다, 해당 파일에서 재사용될필요가 없는 CSS블락이 포함되어있는지를 중점적으로 먼저 보시는게 좋을것같습니다 :)

@addiescode-sj addiescode-sj mentioned this pull request May 2, 2025
18 tasks
@addiescode-sj addiescode-sj merged commit 0519b3f into codeit-bootcamp-frontend:Basic-배수민 May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants