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 #126

Conversation

itscold96
Copy link
Collaborator

@itscold96 itscold96 commented Jun 14, 2024

배포 사이트

주소: https://panda-market-itscold96.netlify.app

요구사항

기본

로그인

  • 이메일 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 에 유효한 값을 입력하면 ‘회원가입' 버튼이 활성화 됩니다.
  • 활성화된 ‘회원가입’ 버튼을 누르면 “/signup” 로 이동합니다

심화

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

주요 변경사항

  • 제작한 함수들에 JSDOC 추가
  • 로그인, 회원가입에 쓰일 js 파일을 form.js과 password.js로 분리
  • class에 따른 routing 기능 구현

스크린샷

image image image

멘토에게

  • 클래스명으로 엘리먼트를 찾아 routing 기능을 넣어주는 방식으로 구현하였는데, 해당 방식이 좋은 방법인지 잘 모르겠습니다.
  • 함수를 설명하기 위해 JSDOC을 사용하였는데, 이 부분이 코드 가독성에 도움이 되는 지 봐주셨으면 좋겠습니다.
  • 반응형의 경우, html font-size로 전체를 줄이는 방식이 포함되어 있는데, 이로 인해 피그마 디자인과 차이가 나다보니,
    해당 부분을 없애는 것이 좋을 지 여쭤보고 싶습니다.
  • constants 폴더에 있는 상수화는 이전 part1 멘토님과 상의하여,
    향후 스프린트 미션이 리액트에서 JSX를 사용하게 되면 적용시키는 것으로 하였습니다.
  • part2 멘토님께 드리는 첫 PR이네요, part2 기간동안 잘 부탁드리겠습니다.

itscold96 added 24 commits June 5, 2024 16:54
- 웹 접근성을 고려하기 위해 div 엘리먼트를 for 프로퍼티를 사용하는 label 엘리먼트로 변경
- div에서 label로 변함에 따라, css도 조정
- action : 페이지 이동이 필요할 때 이동할 페이지의 주소를 넣는다.
- method : 파라미터 전송방식. get방식과 post방식이 있으며 대소문자 구분x
태블릿, 모바일 사이즈에 따른 가변 스타일 적용
- 태블릿 화면에서 PC화면 이미지보다 커지기에 픽셀이 깨지는 부분을 발견하여,
3배수 크기 이미지로 교체
- 브라우저 렌더링 과정에서 head 태그 내부에 스크립트 태그가 있다면,
HTML 파싱 도중 자바스크립트 파일을 다운로드 하게 되어 HTML 파싱 시간이 늦어지는 단점을 발견함

- body 마지막에 script 태그가 위치하면,  HTML 파싱이 완료된 후에 자바스크립트 파일을 다운로드 하게 되어 사용자가 화면을 빨리 볼 수 있다고 함

-util 폴더 이름을 좀더 직관적으로 utils로 변경
- 기존 auth.js 파일을 form.js password.js 파일로 분리
@itscold96 itscold96 requested a review from arthurkimdev June 14, 2024 12:25
@itscold96 itscold96 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 14, 2024
Copy link
Collaborator

@arthurkimdev arthurkimdev left a comment

Choose a reason for hiding this comment

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

함수를 설명하기 위해 JSDOC을 사용하였는데, 이 부분이 코드 가독성에 도움이 되는 지 봐주셨으면 좋겠습니다.

-> 너무 좋습니다. 현업에서도 타입스크립트를 사용하지 않는 프로젝트에 특히 많이 사용해요.

Comment on lines +59 to +70
const showError = (input, message) => {
// 비밀번호 표시 숨김 아이콘을 position: absolute로 설정하여 위치를 잡아주기 위해
// password-wrapper로 password input이 한 번 감싸져 있는 것을 풀기 위함
const inputWrapper =
input.parentElement.classList[0] === INPUT_WRAPPER
? input.parentElement
: input.parentElement.parentElement;
inputWrapper.className = `${INPUT_WRAPPER} error`;
const small = inputWrapper.querySelector('small');
small.innerText = message;
validateSubmit();
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

공통된 로직 처리를 위해 showError 함수를 잘만드셨어요 👍
여기에 추가적으로 아래처럼 classList.toggle 사용해서 가독성을 향상 시킬 수 있습니다.
toggle 관련 API 문서는 아래에 남겨놓을게요.

const showError = (input, message) => {
  const inputWrapper = input.closest(`.${INPUT_WRAPPER}`);
  inputWrapper.classList.toggle('error', true);
  inputWrapper.classList.toggle('success', false);
  const errorElement = inputWrapper.querySelector('small');
  errorElement.innerText = message;
  validateSubmit();
};

https://developer.mozilla.org/ko/docs/Web/API/Element/classList

Comment on lines +77 to +84
const showSuccess = (input) => {
const inputWrapper =
input.parentElement.classList[0] === INPUT_WRAPPER
? input.parentElement
: input.parentElement.parentElement;
inputWrapper.className = `${INPUT_WRAPPER} success`;
validateSubmit();
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 위에와 마찬가지로 classList.toggle을 사용해서 리팩토링 해볼 수 있겠죠!?

Comment on lines +92 to +106
const validateEmail = (event) => {
const regex = new RegExp('[a-z0-9]+@[a-z]+.[a-z]{2,3}');
const email = event.target.value;

if (regex.test(email)) {
isValidEmail = true;
return showSuccess(event.target);
}

isValidEmail = false;
if (email === '') {
return showError(event.target, '이메일을 입력해주세요');
}
showError(event.target, '잘못된 이메일 형식입니다');
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

정규식 굳! 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

아래처럼도 변경할 수 있겠네요!

const EMAIL_REGEX = /^[a-z0-9]+@[a-z]+\.[a-z]{2,3}$/;

/**
 * 이메일 유효성 검사를 수행하는 함수
 *
 * @param {Event} event
 */
const validateEmail = (event) => {
  const email = event.target.value;

  if (EMAIL_REGEX.test(email)) {
    isValidEmail = true;
    showSuccess(event.target);
  } else {
    isValidEmail = false;
    showError(event.target, email === '' ? '이메일을 입력해주세요' : '잘못된 이메일 형식입니다');
  }
};

Comment on lines +186 to +188
inputEmail.addEventListener('focusout', (event) => validateEmail(event));
inputPassword.addEventListener('focusout', (event) => validatePassword(event));
authForm.addEventListener('submit', (event) => handleSubmit(event));
Copy link
Collaborator

Choose a reason for hiding this comment

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

요소 값이 없을 경우도 있을 수 있기 때문에, addEventListenerIfExists 와 같은 공통 함수를 만들어서 랩핑해서 사용하는 방법도 예외처리를 할수 있는 방법입니다.


/**
 * 이벤트 리스너를 등록하는 함수
 *
 * @param {HTMLElement} element
 * @param {string} eventType
 * @param {Function} handler
 */
const addEventListenerIfExists = (element, eventType, handler) => {
  if (element) {
    element.addEventListener(eventType, handler);
  }
};
...
addEventListenerIfExists(inputEmail, 'focusout', validateEmail);
...

Comment on lines +33 to +36
// add event listener
for (let passwordVisiblityIcon of passwordVisiblityIcons) {
passwordVisiblityIcon.addEventListener('click', handleEyeClick);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

forEach 사용하여 아래처럼도 변경할 수 있겠어요.

passwordVisibilityIcons.forEach(icon => {
  icon.addEventListener('click', handleEyeClick);
});

Comment on lines +1 to +21
import { moveLocation } from '../utils/location.js';

// elements
const moveToHomes = document.querySelectorAll('.move-to-home');
const moveToLogins = document.querySelectorAll('.move-to-login');
const moveToItems = document.querySelectorAll('.move-to-items');

// add event listener
for (let el of moveToHomes) {
el.addEventListener('click', () => moveLocation('/'));
}

for (let el of moveToLogins) {
el.addEventListener('click', () => moveLocation('/pages/login'));
}

for (let el of moveToItems) {
el.addEventListener('click', () => {
moveLocation('/pages/items');
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

클래스명으로 엘리먼트를 찾아 routing 기능을 넣어주는 방식으로 구현하였는데, 해당 방식이 좋은 방법인지 잘 모르겠습니다.

-> 음 클래스로 구현했다고해서 사실 좋다 / 나쁘다를 결정하기엔 어렵습니다만, 보통의 경우 클래스명은 주로 CSS 스타일링을 위해 많이 사용됩니다. 동일한 클래스를 스타일링과 기능(이벤트 핸들러) 모두에 사용하면 혼란을 초래할 수 있습니다. 예를들어 클래스 이름을 변경하면 스타일링뿐만 아니라 기능적인 부분도 변경해야 사이드가 발생하지 않는데, 이런 부분들을 놓칠 수 있기 때문이에요. (동료들과 함께 작업할 때 특히 그렇겠죠?)

그래서 데이터 속성을 사용하는 방법도 대안이될 수 있겠어요. 데이터 속성을 사용할 경우, 클래스명과 명확하게 역할이 분리되기 때문입니다. 어느 한 곳이 변경되도 사이드가 발생하지도 않죠. 데이터 속성으로도 한번 고민해봐주시면 좋을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

예를들면, 데이터 속성은 data-* 방법으로 추가할 수 있어요.

 <button data-navigate="/" class="navigate">Home</button>

아래처럼 가져올 수 있구요.

const navigableElements = document.querySelectorAll('[data-navigate]');

navigableElements.forEach(el => {
  el.addEventListener('click', (event) => {
    const path = event.currentTarget.getAttribute('data-navigate');
    moveLocation(path);
  });
});

@arthurkimdev
Copy link
Collaborator

반응형의 경우, html font-size로 전체를 줄이는 방식이 포함되어 있는데, 이로 인해 피그마 디자인과 차이가 나다보니,
해당 부분을 없애는 것이 좋을 지 여쭤보고 싶습니다.

-> 해당부분은 지난번 멘토링 때 답변 드린걸로 대체하겠습니다~

Copy link
Collaborator

@arthurkimdev arthurkimdev left a comment

Choose a reason for hiding this comment

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

민찬님 과제 제출하시느라 수고 많으셨습니다! 🙏

@arthurkimdev arthurkimdev merged commit 89f4280 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