[전유진] sprint4

[yuj2n]

📝요구사항

배포 사이트 URL: https://pandamarket4-yuj2n.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과 비밀번호 확인 input의 값이 다른 경우, 비밀번호 확인 input 아래에 “비밀번호가 일치하지 않습니다..” 에러 메세지를 보입니다.

📖심화

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

💭궁금한 점

• 비밀번호 칸에 에러 메시지가 뜨면 눈 모양 아이콘이 같이 아래로 조금 이동하는 부분을 어떻게 수정하면 좋을까요?
• auth.js와 signup.js에서 동일 변수를 사용하면 변수 충돌이 일어나 js가 작동하지 않는 경우가 발생하는데 그 이유가 무엇인가요? 그리고 이러한 경우에 signup.js에서 중복 변수를 변수명을 다르게 선언하는 경우 말고 동일 변수로 해결할 수 있는 방법은 없을까요?
• login.js와 signup.js의 동일한 기능은 auth.js로 공동으로 가져가도록 해주었는데 적절하게 분배하였는지, 그리고 이렇게 작성하는 것이 괜찮은지 궁금합니다!

💻스크린샷

• 로그인(에러 메시지)
  
• 로그인(올바른 값 및 토글)
  
• 회원가입(에러 메시지)
  
• 회원가입(올바른 값 및 토글)
  

🙇‍♀️멘토에게

• 코드에서 불필요하게 사용된 부분이나 간결하게 작성할 수 있는 부분이 있다면 피드백 부탁드립니다!!
• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[yuj2n]

css와 js 사용에 있어서 잘못 사용하고 있는 부분이나 비효율적으로 작성하고 있는 코드가 있다면 피드백 부탁드립니다!🙇‍♀️

[yuj2n]

올바른 값 입력 후 삭제 시 파란 테두리가 사라지지 않아 작성하였는데
이렇게 작성하는 것이 괜찮을까요? 아니면 에러 속성이 덮도록 작성하는 것이 좋을까요?

[GANGYIKIM]

아마 error-input 클래스를 추가해도 해당 태그 클래스명에 correct-input이 있으면 correct-input이 우선 적용되신다는 말씀이신 것 같아요.
이를 해결하기 위해 css에서 error-input과 correct-input의 명시도를 보고 error-input의 명시도를 높이거나 후순위에 적어 correct-input이라는 클래스명이 태그에 존재해도 error-input이 있다면 무조건 적용되게 할 수 있으나, 그렇게되면 클래스명과 상태가 적절하지 않은 것 같아 지금 방식이 더 나은 것 같습니다.

다만 제가 알기로는 인풋의 border는 회색이 기본이고, 에러 발생 시 빨강, 포커스시 파랑으로 알고 있습니다.
이러한 요구사항을 구현하실때는 클래스를 두개로 나눠 작업하지 않으셔도 됩니다~

[yuj2n]

에러 메시지가 추가 시 눈 모양 아이콘이 해당 너비의 비율을 맞추기 위해 아래로 일정 크기만큼 내려가는데
내려가지 않게 하려면

1. querySelector로 js에서 아이콘에 위치를 수정해주는 class나 id를 추가해주는 방식으로 해결하는 것이 좋을까요?
2. 아니면 애초에 내려가는 것을 방지하도록 css적으로 어떻게 해결할 수 있는지 궁금합니다!

[GANGYIKIM]

질문에 남겨주셔서 코드리뷰에서 답변 드렸으니 참고해주세요!
참고로 css 적으로 수정하시려면 반응형 수치가 아닌 top: 20px 같은 식으로 고정값을 입력해주시면 됩니다!

[yuj2n]

signup.js 기능이 동작하지 않아 변수명을 따로 작성해주었더니 잘 적용되었습니다.

[GANGYIKIM]

그 전 코드가 어떤지 몰라 추측으로 답변을 드리자면, 아마 auth.js에 있는 변수명과 동일한 변수명으로 변수들을 선언하셔서
signup.html에서는 두 파일다 불러오게 되니 의도하신대로 동작하지 않으신것이 아닐까 싶습니다.

[GANGYIKIM]

유진님 4번째 미션 제출 고생하셨습니다~
그전에 제출하신 3주차 PR 이후 부분부터 코드리뷰 드렸으니 참고해주세요~
다음 React 미션도 화이팅입니다!

---

• 회원가입 페이지에서 비밀번호 입력 후 비밀번호 확인 입력하고 다시 비밀번호를 변경하면 비밀번호 확인 인풋은 재확인 로직을 실행하지 않네요~ 한번 다양한 케이스를 테스트해보세요.

• 비밀번호 칸에 에러 메시지가 뜨면 눈 모양 아이콘이 같이 아래로 조금 이동하는 부분을 어떻게 수정하면 좋을까요?:
  비밀번호 인풋의 경우 해당 버튼이 absolute로 띄워져 있고 이의 기준을 부모인 password-container로 잡고있기 때문에 에러메시지가 추가되면 해당 부모의 height가 길어져 이동되게됩니다. css로도 수정이 가능하겠지만 저는 html의 구조를 바꾸셔서 input과 button를 한번 더 감싸고 해당 태그를 기준으로 button의 위치를 정해줄 것 같습니다.

• auth.js와 signup.js에서 동일 변수를 사용하면 변수 충돌이 일어나 js가 작동하지 않는 경우가 발생하는데 그 이유가 무엇인가요? 그리고 이러한 경우에 signup.js에서 중복 변수를 변수명을 다르게 선언하는 경우 말고 동일 변수로 해결할 수 있는 방법은 없을까요?:
  관련해서 코멘트 남겼으니 코멘트 참고해주세요~

• login.js와 signup.js의 동일한 기능은 auth.js로 공동으로 가져가도록 해주었는데 적절하게 분배하였는지, 그리고 이렇게 작성하는 것이 괜찮은지 궁금합니다!:
  코딩을 할때 중복을 줄이는 것은 기본적으로 좋은 자세입니다. 두 페이지에서 기능상 같이 사용할 수 있는 부분이 많아 재활용하고자 하신 것은 좋습니다. 지금 작성하신 코드내에서는 적절하게 분배하신 것 같습니다.

[GANGYIKIM]

❗️ 수정요청
해당 함수에서 참조하는 emailErr라는 값이 auth.js에서 선언하신 변수인 것 같습니다.
해당 변수가 전역 변수로 선언되어 있긴 하지만 이렇게 가지고 오시는 것은 구조를 파악하기도 어렵고 변수 명에 영향을 많이 받게 됩니다.
가능하면 아래처럼 작성해서 명확성과 가독성을 높여주세요.

import { emailErr } from './auth.js';

function toggleButton() {
    if (emailErr.textContent === "") { ... }
}

function toggleButton() {
    const emailErr = document.getElementById("emailErr");

    if (emailErr.textContent === "") { ... }
}

[GANGYIKIM]

💊 제안
조건문이 길때는 주석도 좋지만 변수로 빼시면 가독성에 도움이 됩니다~
또한 이메일이나 비밀번호 인풋의 값이 없으면 각 인풋의 에러메시지가 있을 테니 아래 조건만 확인하셔도 될 것 같아요.

Suggested change

	function toggleButton() {
	// 이메일, 비밀번호 유효성 검사 및 값이 있는지 확인
	if (emailErr.textContent === "" && passwordErr.textContent === "" &&
	emailInput.value.trim() !== "" && passwordInput.value.trim() !== "") {
	loginButton.disabled = false;
	loginButton.classList.add("active");
	} else {
	loginButton.disabled = true;
	loginButton.classList.remove("active");
	}
	}
	function toggleButton() {
	const isFormValid = emailErr.textContent === "" && passwordErr.textContent === "";
	loginButton.disabled = !isFormValid;
	loginButton.classList.toggle("active", isFormValid);
	}

[GANGYIKIM]

💊 제안
form이 유효하지 않으면 submit button이 disabled 상태라 제출이 되지 않으므로 관련 코드는 없어도 될 것 같아요~

Suggested change

	if (!loginButton.disabled) {
	window.location.href = "/items.html";
	}
	window.location.href = "/items.html";

[GANGYIKIM]

그 전 코드가 어떤지 몰라 추측으로 답변을 드리자면, 아마 auth.js에 있는 변수명과 동일한 변수명으로 변수들을 선언하셔서
signup.html에서는 두 파일다 불러오게 되니 의도하신대로 동작하지 않으신것이 아닐까 싶습니다.

[GANGYIKIM]

아마 error-input 클래스를 추가해도 해당 태그 클래스명에 correct-input이 있으면 correct-input이 우선 적용되신다는 말씀이신 것 같아요.
이를 해결하기 위해 css에서 error-input과 correct-input의 명시도를 보고 error-input의 명시도를 높이거나 후순위에 적어 correct-input이라는 클래스명이 태그에 존재해도 error-input이 있다면 무조건 적용되게 할 수 있으나, 그렇게되면 클래스명과 상태가 적절하지 않은 것 같아 지금 방식이 더 나은 것 같습니다.

다만 제가 알기로는 인풋의 border는 회색이 기본이고, 에러 발생 시 빨강, 포커스시 파랑으로 알고 있습니다.
이러한 요구사항을 구현하실때는 클래스를 두개로 나눠 작업하지 않으셔도 됩니다~

[GANGYIKIM]

👍 칭찬
중복을 줄이신 점 좋아요!

[GANGYIKIM]

💊 제안
passwordInput처럼 변수명을 통해 해당 변수가 어떤 타입의 값을 알 수 있도록 명명하시는 것이 가독성 측면에서 좋습니다~

Suggested change

	const passwordVisibility = document.getElementById("passwordVisibility");
	const passwordVisibilityButton = document.getElementById("passwordVisibility");

[GANGYIKIM]

💊 제안
한 파일에서 최상단에 변수 선언들을 위치시키는 것처럼 일정한 순서로 코드가 작성되는 것이 가독성 측면에서 좋습니다~
지금 변수와 이벤트 바인딩문이 번갈아가며 작성되어 있고 그렇게 위치해야하는 이유도 잘 모르겠어요~
가능하면 이벤트 바인딩문은 변수 선언문보다 하단에 위치시키는 것을 추천드립니다!