[김준우] Sprint4

[wnsdn00]

요구사항

기본

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

심화

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

스크린샷

멘토에게

• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[GANGYIKIM]

준우님 4번째 미션 제출 고생하셨습니다.
제가 드린 피드백 반영은 필수가 아니니 편하게 참고해주세요.
그리고 다음 PR 배포 주소도 포함되어 있고 구현하신 사항들도 표시되어 있으면 더 좋을 것 같습니다.

[GANGYIKIM]

💊 제안

아마 script 태그를 HTML의 하단에 배치하신 이유가 script가 문서의 렌더링을 막지 않도록 하기 위해서이실 것 같아요.
하지만, script 태그에 defer나 async 속성을 사용하면 이런 문제를 해결할 수 있기 때문에 반드시 하단에 배치할 필요는 없습니다!
또한 script 태그는 상단에 있는게 구조 파악에서도 유리하기 때문에 상단 head 태그에 두시는 것을 추천드려요~

script async
script defer

지금과 같은 경우 DOM을 조작하는 JS 이니 defer 속성을 추가하시면 되겠습니다~

[GANGYIKIM]

💊 제안
로그인에서 사용하는 js 파일과 에러메시지나 로직 측면에서 동일한 것이 많습니다~
이런 경우 중복을 줄이기 위해 반복 사용되는 함수, 정규식, 에러메시지들은 다른 파일로 분리하시고 각 js 파일에서 가져와서 사용하시는 것을 추천드려요!

[GANGYIKIM]

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

[GANGYIKIM]

💊 제안
해당 변수가 태그임을 일 수 있도록 명명해주시면 더 좋을 것 같아요~

Suggested change

	const passwordToggle = document.querySelector("#password-toggle");
	const passwordToggleButton = document.querySelector("#password-toggle");

[GANGYIKIM]

💊 제안
JS 를 통해 스타일을 변경해주셔도 괜찮지만, error와 관련된 클래스를 만들어두시고 해당 클래스를 통해 스타일링하시면
코드도 줄고 css관련은 css 파일에 적으니 더 좋을 것 같네요!

function showError(input, errorElement, message) {
    input.classList.add("error");
    errorElement.textContent = message;
    errorElement.classList.add("error");

    updateSignupButtonState();
  }

  function hideError(input, errorElement) {
    input.classList.remove("error");
    errorElement.textContent = "";
    errorElement.classList.remove("error");

    updateSignupButtonState();
  }

[GANGYIKIM]

💊 제안
해당 함수를 보면 if문 조건에 해당하는 경우 false를 반환하니 분기를 이렇게 나눠주실 필요가 없을 것 같아요~
또한 isValid 변수는 단순히 true 또는 false 값을 저장하는 용도로만 사용되므로, 별도로 선언할 필요 없이 바로 반환하는 것이 더 효율적일 것 같아요~

Suggested change

	function validateInput() {
	let isValid = true;
	if (emailInput.value.trim() === "" || emailError.textContent !== "") isValid = false;
	if (usernameInput.value.trim() === "" || usernameError.textContent !== "") isValid = false;
	if (passwordInput.value.trim() === "" || passwordError.textContent !== "") isValid = false;
	if (confirmPasswordInput.value.trim() === "" || confirmPasswordError.textContent !== "") isValid = false;
	return isValid;
	}
	function validateInput() {
	if (emailInput.value.trim() === "" || emailError.textContent !== "" || usernameInput.value.trim() === "" || usernameError.textContent !== "" || passwordInput.value.trim() === "" || passwordError.textContent !== "" || confirmPasswordInput.value.trim() === "" || confirmPasswordError.textContent !== "") return false;
	return true;
	}

혹은 아래처럼도 가능합니다.

Suggested change

	function validateInput() {
	let isValid = true;
	if (emailInput.value.trim() === "" || emailError.textContent !== "") isValid = false;
	if (usernameInput.value.trim() === "" || usernameError.textContent !== "") isValid = false;
	if (passwordInput.value.trim() === "" || passwordError.textContent !== "") isValid = false;
	if (confirmPasswordInput.value.trim() === "" || confirmPasswordError.textContent !== "") isValid = false;
	return isValid;
	}
	function validateInput() {
	return (
	emailInput.value.trim() !== "" && emailError.textContent === "" &&
	usernameInput.value.trim() !== "" && usernameError.textContent === "" &&
	passwordInput.value.trim() !== "" && passwordError.textContent === "" &&
	confirmPasswordInput.value.trim() !== "" && confirmPasswordError.textContent === ""
	);
	}

[GANGYIKIM]

💊 제안
login 파일에서도 동일한 정규식을 사용하고 있는데 그때마다 선언해서 쓰면 문제가 있을 것 같아요.
정규식을 반복해서 선언하면 유지보수가 어려워지고, 코드의 일관성이 떨어질 수 있습니다. 만약 정규식을 수정해야 할 때 여러 곳에서 변경해야 하고 실수로 서로 다른 정규식을 사용할 위험도 커집니다.
이렇게 두개의 파일에서 사용하고 있으니 해당 정규식과 test 함수를 따른 파일로 분리해 사용하시는 것을 추천드려요!

[GANGYIKIM]

💊 제안
이벤트 리스너에서 콜백 함수를 전달할 때, 익명 함수로 감싸지 않고 바로 함수 참조를 전달하는 것이 더 간결하고 효율적일 것 같습니다~

Suggested change

	passwordInput.addEventListener("input", function () {
	validateConfirmPassword();
	});
	passwordInput.addEventListener("input", validateConfirmPassword);

[GANGYIKIM]

❗️ 수정요청
버튼이 disabled 상태이면 click 되지 않으므로 아래처럼 작성하셔도 충분할 것 같습니다.

Suggested change

	signupButton.addEventListener("click", function (event) {
	event.preventDefault();
	if (!signupButton.disabled) {
	location.href = "login.html";
	}
	});
	signupButton.addEventListener("click", function (event) {
	event.preventDefault();
	location.href = "login.html";
	});

[GANGYIKIM]

❗️ 수정요청
JS 가 아니라 html 에서 button에 disabled 속성을 true로 주고 시작하는게 더 좋을 것 같아요.
JS에서 이렇게 해야할 이유를 잘 모르겠고 이렇게 되면 HTML 에서 button이 활성화된채로 렌더링됩니다~