Skip to content
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

[나윤주] sprint4 #109

Merged

Conversation

naynara87
Copy link
Collaborator

@naynara87 naynara87 commented Jun 14, 2024

판다마켓

사이트주소: https://pandamarket-momo.netlify.app/

요구사항

  • React와 같은 UI 라이브러리를 사용하지 않고 진행합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.

기본

로그인

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

회원가입

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

심화

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

스타일 관련

//scss 변환
sass --watch assets/scss/style.scss assets/css/style.css 

스크린샷

valid_error valid_success

멘토에게

  • �signup js 두가지 버전으로 코드 시도 해보았습니다.
  • 코드를 더 줄일 수 있을 것 같았는데 시간 관계상 우선 되는대로 진행 하였습니다.

@naynara87 naynara87 requested a review from Taero-Kim June 14, 2024 01:53
@naynara87 naynara87 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 14, 2024
@naynara87 naynara87 changed the base branch from main to Basic-나윤주 June 14, 2024 01:54
@Taero-Kim Taero-Kim assigned Taero-Kim and naynara87 and unassigned naynara87 Jun 14, 2024
assets/js/form-validation.js Show resolved Hide resolved
assets/js/form-validation.js Show resolved Hide resolved
assets/js/form-validation.js Show resolved Hide resolved
assets/js/form-validation.js Show resolved Hide resolved
}
},
rules: {
'val-email': {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p4;
val-email , val-password 등의 각 인풋의 유효성 관련 key들이 코드내에서 많이 사용되고 있네요!
요런 항목들은 한 번더 상수화해서 관리하면, 오타 등의 휴먼 에러를 더 줄일 수 있을 것 같아요!

보통 요렇게 상수화 시킨값들은 대문자와 언더바 패턴을 많이 사용해요!

const VALIDATE_LIST = Object.freeze({
  EMAIL: 'val-email',
  NICKNAME: 'val-nickname',
  CONFIRM_PASSWORD: 'val-confirm-password',
  ...
})

rules: {
  [VALIDATE_LIST.EMAIL]: {...},
  [VALIDATE_LIST.NICKNAME]: {...},
}


form.addEventListener('submit', function (event) {
event.preventDefault();
if (validateEmail() && validatePassword()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p4;
요거는 checkFormValidity()로도 가능하지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 그러네요. 중복 줄일 수 있네요!!

assets/js/password-toggle.js Show resolved Hide resolved
assets/js/signup.js Show resolved Hide resolved
}

}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3;
조건문들이 중첩이 많아서, 가독성이 다소 떨어져 보이는 것 같아요!
요럴때 switch 문을 활용해서 input.name 별로 분기를 나누고,
각 input.name에서 사용되는 로직들을 별개의 함수로 분리하면 가독성이 훨씬 좋을 것 같아요!

ex)

const checkEmailValue = () => {
  const emailValue = form.email.value.trim();
  const emailPattern = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,6}$/;

  if (emailValue === "") {
    setErrorFor(input, "이메일을 입력해주세요.");
    return;
  } 
  
  if (!emailPattern.test(emailValue)) {
    setErrorFor(input, "잘못된 이메일 형식입니다.");
    return;
  }

  setSuccessFor(input);
}

... 
// 기타 다른 인풋에 대한 검증  함수들..

function checkInputs(input) {
  switch (input.name) {
    case 'email':
      checkEmailValue();
      break;
    case 'password':
      checkPasswordValue();
      break;
    case 'confirmPassword':
      checkConfirmPasswordValue();
      break;
    case 'nickname':
      checkNickname();
      break;
    default:
      break;
  }
}

}
}

function checkAllInputs() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3;
위에서 checkEmailValue() 처럼 각 inputValue별로 함수를 분리해놓은 경우
여기서도 재사용이 가능할 것 같아요!

const emailValid = checkEmailValue(form.email.value, true);
...

필요한 경우에는 checkEmailValue()에 인자를 넘겨서,
상황에 맞게 재사용할 수 있도록 checkEmailValue()를 재구성 하는 것도 좋을 것 같아요!

const checkEmailValue = (emailValue, checkIfValid) => {
  const emailPattern = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,6}$/;

  const isEmailValueEmpty = emailValue === "";
  const isValidEmailPattern = emailPattern.test(emailValue);

  if(checkIfValid) return !isEmailValueEmpty && isValidEmailPattern;  --> 불리언 결과만 리턴 받고 싶은경우 checkIfValid 인자를 통해 여기서 불리언 값만 리턴

  if (emailValue === "") {
    setErrorFor(input, "이메일을 입력해주세요.");
    return;
  } 
  
  if (!emailPattern.test(emailValue)) {
    setErrorFor(input, "잘못된 이메일 형식입니다.");
    return;
  }

  setSuccessFor(input);
}

@Taero-Kim
Copy link
Collaborator

고생 많으셨어요, 윤주님!
전반적으로 기능도 잘 구현하셨고, 여러가지 시도를 하신 것에서도
많은 고민들이 느껴져서 좋았어요!

아래 사항들을 고민해보고 연습해보면 차차 더 코드가 좋아질 것 같아요!

  1. 직관적인 변수명 짓기
  2. 공통되는 부분들에 대한 함수화
  3. fucntion 키워드로 함수를 선언하는 대신 화살표 방식 함수 작성해보기

이외에 코드를 보며 몇가지 개선하면 좋을 점에 대해서 간단한 리뷰들 남겼습니다!
제가 남긴 리뷰에 대한 질문이나 기타 질문이 있으시면 언제든 편하게 말씀해주세요!

+) 리뷰를 남긴 코멘트 위의 p(1~5); 마크의 의미는 아래와 같습니다! 참고하면 좋을 것 같아요!

p1; 꼭 반영해주세요 (Request changes)
리뷰어는 PR의 내용이 서비스에 중대한 오류를 발생할 수 있는 가능성을 잠재하고 있는 등 중대한 코드 수정이 반드시 필요하다고 판단되는 경우, P1 태그를 통해 리뷰 요청자에게 수정을 요청합니다. 리뷰 요청자는 p1 태그에 대해 리뷰어의 요청을 반영하거나, 반영할 수 없는 합리적인 의견을 통해 리뷰어를 설득할 수 있어야 합니다.

p2; 적극적으로 고려해주세요 (Request changes)
작성자는 P2에 대해 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장합니다.

p3; 웬만하면 반영해 주세요 (Comment)
작성자는 P3에 대해 수용하거나 만약 수용할 수 없는 상황이라면 반영할 수 없는 이유를 들어 설명하거나 다음에 반영할 계획을 명시적으로(JIRA 티켓 등으로) 표현할 것을 권장합니다. Request changes 가 아닌 Comment 와 함께 사용됩니다.

p4; 반영해도 좋고 넘어가도 좋습니다 (Approve)
작성자는 P4에 대해서는 아무런 의견을 달지 않고 무시해도 괜찮습니다. 해당 의견을 반영하는 게 좋을지 고민해 보는 정도면 충분합니다.

p5; 그냥 사소한 의견입니다 (Approve)
작성자는 P5에 대해 아무런 의견을 달지 않고 무시해도 괜찮습니다.

@naynara87
Copy link
Collaborator Author

제가 남긴 리뷰에 대한 질문이나 기타 질문이 있으시면 언제든 편하게 말씀해주세요!

+) 리뷰를 남긴 코멘트 위의 p(1~5); 마크의 의미는 아래와 같습니다! 참고하면 좋을 것 같아요!

넵!!

@Taero-Kim Taero-Kim merged commit 0d53f2e into codeit-bootcamp-frontend:Basic-나윤주 Jun 16, 2024
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