Skip to content

[이재원] sprint4 #71

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

Conversation

jaewon8405
Copy link
Collaborator

요구사항

기본

로그인

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

심화

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

주요 변경사항

스크린샷

image

멘토에게

  • 기능을 구현하면서 중복되는 기능이 많았는데 이 기능들을 최소화할 수 있는 방법을 알고 싶습니다.
  • 제 생각에는 하나에 js파일로 나타낼 수 있을 거 같았는데 그냥 각 페이지마다 js 파일을 따로 두는 게 더 좋은 지 궁금합니다.
  • 이러한 기능들을 구현할 때 팁이나 구상하는 방법과 같은 것들을 알고 싶습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

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

@baeggmin baeggmin left a comment

Choose a reason for hiding this comment

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

첫 JS 미션 수행하시느라 고생많으셨습니다! 👍👍
질문 주신 내용에 대해서는 인라인 코멘트로 리뷰 남겼으니 참고 부탁드립니다.

function handleFocustOutId() {
const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
const value = $userId.value.trim();
if (value === '') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

$xxxError.style.display = 'block';
$xxxError.innerText = 'xxx를 입력해주세요.';
$xxx.style.border = '2px solid red';
xxxCheck = false;

위와 같은 코드 라인이 이메일, 닉네임, 비밀번호 등에서 계속 반복되고 있습니다.
아래와 같이 추상화해서 중복 코드를 제거하면 좋을 것 같습니다.

function showError(input, errorEl, message) {
  errorEl.style.display = 'block';
  errorEl.innerText = message;
  input.style.border = '2px solid red';
}

function clearError(input, errorEl) {
  errorEl.style.display = 'none';
  input.style.border = '';
}

let nicknameCheck = false;
let passwordAuthCheck = false;

function btnCheck() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수명은 보다 직관적이게, 그리고 가급적 동사로 시작되도록 지어주시는게 좋습니다!
updateSubmitButtonState 정도로 수정하면 좋을것 같네요.

}
}

function handleFocustOutId() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이메일 인풋과 관련된 이벤트핸들러이니 handleFocuseOutEmail 이 더 적합할 것 같네요.

const $viewIcon = document.querySelector('#viewIcon');
const $viewIcon2 = document.querySelector('#viewIcon2');
let idCheck = false;
let passwordCheck = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

passwordCheck라는 변수명이 비밀번호 일치 여부인지, 비밀번호 유효성인지 불명확한 것 같습니다. isPasswordValid, isPasswordConfirmed 등으로 명확히 분리하면 좋을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

login.js 와 signup.js 에서 아래와 같은 유틸 함수가 중복되어 있습니다.

  • 이메일 형식 검사
  • 비밀번호 길이 검사
  • 값이 비어있는지 확인하는 로직

이러한 구조는 유지보수에 불리하고, 검증 기준이 바뀔 경우 양 파일을 모두 수정해야 하므로 실수를 유발하기 쉽습니다. 이런 유효성 판단 함수는 아래와 같이 ** 공통 유틸 함수**로 분리 하면 좋을 것 같네요.

// validation.js 등으로 분리
export function showError(input, errorEl, message) { ... }
export function clearError(input, errorEl) { ... }
export function isEmailValid(value) { ... }
export function isNotEmpty(value) { ... }

</div>
<div class="form-group">
<label for="password">비밀번호</label>
<div class="input-wrapper">
<input id="password" type="password" name="password" />
<img src="images/password-icon1.png" alt="보기 아이콘" />
<img id="viewIcon" src="images/eye-close.png" alt="보기 아이콘" />
<p id="passwordError" style="display: none; color: red"></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 에러 메세지 요소를 보여줄때, JS에서 직접 인라인 스타일을 조작하고 계신데요, 이는 간단한 방법이긴 하지만 CSS 클래스와 충돌하거나 덮어쓰기 위험이 있어 유지보수에 불리합니다.

차라리 아래와 같이 에러 메시지용 클래스를 정의하고, js에서는 클래스 추가/제거만 담당하는 구조로 수정해보시면 어떨까요??

/* CSS */
.error-message {
  display: none;
  color: red;
  font-size: 14px;
}

.error-message.visible {
  display: block;
}


function handleFocustOutId() {
const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
const value = $userId.value.trim();
Copy link
Collaborator

Choose a reason for hiding this comment

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

공백 입력을 고려한 trim() 메소드 활용 좋습니다! 👍

function handlePassWordView() {
const isPassWordVisible = $password.type === 'text';
$password.type = isPassWordVisible ? 'password' : 'text';
$viewIcon.src = isPassWordVisible
Copy link
Collaborator

Choose a reason for hiding this comment

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

src 컨트롤로 토글 버튼 구현해주신 점 좋네요!👏

@baeggmin
Copy link
Collaborator

제 생각에는 하나에 js파일로 나타낼 수 있을 거 같았는데 그냥 각 페이지마다 js 파일을 따로 두는 게 더 좋은 지 궁금합니다.
로그인과 회원가입에 중복되는 로직이 많긴하지만, 그렇다고 파일 하나로 관리하기엔 분명 구현 로직이 상이한 부분도 꽤 많은 것 같습니다. 이런 경우에는 중복되는 로직만 공통 파일로 추상화하고, 각 페이지별 Js 에서 공통 모듈을 호출하는 방식이 적합한 것 같습니다. 대략적으로 아래와 같은 구조가 될 것 같네요.

scripts/
├── validation.js   ← 공통 유효성 검사 유틸 (isEmailValid, showError 등)
├── dom.js    ← 공통 토글 함수, 버튼 상태 업데이트 등 (또는 login.js / signup.js 내부에 선언)
├── login.js        ← login 페이지 전용 처리만
└── signup.js       ← signup 전용 처리

@baeggmin
Copy link
Collaborator

이러한 기능들을 구현할 때 팁이나 구상하는 방법과 같은 것들을 알고 싶습니다.
제가 이런 기능을 구현할 때 주로 고려하는 기준은 크게 아래 세 가지 정도입니다.
처음부터 이 모든 걸 완벽히 신경 쓰기는 어렵지만, 개발에 익숙해질수록 자연스럽게 몸에 배게 될거에요.
지금처럼 하나하나 고민하면서 구현해보시다보면 자연스럽게 성장하실거라 믿습니다 😊

  1. 공통 로직과 개별 로직 구분하기
  • 값이 비어있는지, 이메일 형식인지, 에러 출력 -> 공통으로 처리
  1. UI 요소와 상태 분리
  • UI 컨트롤은 class를 추가/제거해서 처리
  • UI 상태를 컨트롤하는 변수를 두지않고, 계산식으로 판단. ex) 모든 필드가 유효할 때만 button.disabled = false
  1. 기능 단위로 함수 나누기
  • validateEmail(), updateSubmitState(), togglePasswordView()처럼 작고 목적이 명확한 함수로 분리

@baeggmin baeggmin merged commit f2bb5ef into codeit-bootcamp-frontend:Basic-이재원 Jun 16, 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