Conversation
- Verifying password now doesn't show error when it's empty. - And it's now working responsively to password input.
- Changed the <form> tag in login and signup htmls into <div> with 'pseudo-form' class to make the button work as intended.
| </header> | ||
| <main> | ||
| <div class="container"> | ||
| <form> | ||
| <label for="email">이메일</label> | ||
| <div class="pseudo-form"> |
There was a problem hiding this comment.
여기는 가짜 폼태그보다 진짜 폼태그를 써주는게 어떨까요?
| <div class="pseudo-form"> | |
| <form class="signup-form"> |
| <label for="email"> | ||
| 이메일 | ||
| <span class="email-empty error-message hidden">이메일을 입력해주세요.</span> | ||
| <span class="email-wrong-format error-message hidden">잘못된 이메일 형식입니다.</span> | ||
| </label> | ||
| <input id="email" name="email" class="text-input" placeholder="이메일을 입력해주세요"> | ||
| <label for="password">비밀번호</label> | ||
|
|
There was a problem hiding this comment.
제 생각에는 에러메세지는 의미적으로 라벨과 상관이 없다고 생각합니다.
재헌님께서 에러메세지를 position: absolute로 선언하고 라벨을 position: relative를 선언 해 줌으로서 에러메세지 위치를 잡으려고 라벨안에 두신것 같은데 이러한 의도가 맞을까요?
그렇다면 라벨과 인풋을 div로 감싸는 마크업을 더 낫다고 생각이 듭니다.
<div>
<label for="email"> 이메일 </label>
<input
id="email"
name="email"
class="text-input"
placeholder="이메일을 입력해주세요"
/>
<span class="email-empty error-message hidden"
>이메일을 입력해주세요.</span
>
<span class="email-wrong-format error-message hidden"
>잘못된 이메일 형식입니다.</span
>
</div>| if (btn.classList.contains('login')) btn.addEventListener('click', () => { btnLink(loginButtonLink) }); | ||
| else if (btn.classList.contains('signup')) btn.addEventListener('click', () => { btnLink(signupButtonLink) }); |
There was a problem hiding this comment.
버튼 관련 로직을 재사용 또는 통일 시키려고 하신 노력이 보이시는것 같네요.
제 생각에는 signupEventHandler 자바스크립트 파일에는 회원가입에 관한 로직만 있는게 더 좋다고 생각합니다. 그러나 20번줄을 보면 로그인 버튼 관련 로직이 있는 것 같네요. 로그인 버튼 로직은 로그인 자바스크립트 파일에 작성해주시는게 더 좋습니다.
로직이 다른 파일에 섞이게되면 이게 스노우볼이 되어서 나중에는 관리가 힘들어진다고 생각합니다.
There was a problem hiding this comment.
function btnLink(ref) {
location.href = ref;
}
export { btnLink }btnLink의 구현체인데, 이정도로 짧은 코드는 화살표함수를 써서 인라인으로 구현하는게 어떨까요?
| form.addEventListener('focusout', eventFunctionsPackage); | ||
| form.addEventListener('input', eventFunctionsPackage); |
There was a problem hiding this comment.
이 부분이 요구사항과 맞지 않는 부분이 있는 것 같습니다.
요구사항에서는 focus out일때 validation을 하도록 안내가 되어있습니다.
하지만 지금 코드에서는 focus out 및 유저가 타이핑을 하는 매순간 마다 validation이 진행되고 있습니다.
focus out일때 validation을 진행하고 input 입력중일때는 에러메세지를 없애는 로직으로
focus out의 로직과 input 이벤트의 로직을 분리하는게 어떨까요?
실무에서는 재헌님이 하신 방법도 UX를 고려해서 사용하는 방법이긴 합니다만 스프린트 미션에서는 요구사항이 명시되어있어서 우선 그것을 따르는게 맞는것 같아요 ㅎㅎ
| function isCorrectEmailFormat(email) { | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| return emailRegex.test(email); | ||
| } |
There was a problem hiding this comment.
정규 표현식을 사용하신점도 좋았고 테스트하는 로직을 함수로 감싸주신점도 잘 하셨습니다 !
| const passwordEmptyMessage = document.querySelector('.password-empty'); | ||
| const passwordWrongFormatMessage = document.querySelector('.password-wrong-format'); | ||
|
|
||
| const isLongEnough = (str) => str.length >= 8; |
There was a problem hiding this comment.
로직을 이해하기 쉽게 함수로 빼신 점 잘 하셨습니다 !
| validChecklist.every((e) => e === true) | ||
| ? btn.classList.add('valid') | ||
| : btn.classList.remove('valid'); | ||
| const isValid = [...inputs].every(e => !e.classList.contains('errored')) && filledInputCheck(); // 모든 input에 errored 클래스가 없다 && 모든 input에 값이 있다 |
There was a problem hiding this comment.
every 메서드를 사용하신 점 잘하셨습니다 !
정규표현식을 사용하셨는데 이 방법을 많이 씁니다. 저는 실무에서 요구사항이 따로 없다면 form 태그가 제공하는 client side validation 방법을 사용합니다. https://developer.mozilla.org/en-US/docs/Learn/Forms/Form_validation |
|
스프린트 미션 4 수고 하셨습니다. |
요구사항
기본
심화
주요 변경사항
멘토에게