[김희수] Sprint4#89
Hidden character warning
Conversation
ByungyeonKim
left a comment
ByungyeonKim
left a comment
There was a problem hiding this comment.
안녕하세요 희수님~!
다음이 드디어 리액트 미션이군요! 🤩
고생 많으셨어요.
먼저, 개략적으로 피드백 드릴게요. 자세한 내용은 아래 리뷰를 참고해 주세요. 😀
- CSS 파일을 나눈 것처럼 JS 파일도 비슷하게 구성하면 좋을 것 같아요. 현재는 기능이 작아서, 기능마다 모듈을 분리하기 보다는 공통 로직은
auth.js, 페이지별 로직은login.js,signup.js로 나누는 건 어떨까요? 구조가 더 일관성 있게 될 것 같아요. - 인라인 스타일이 아닌, 태그마다 클래스명은 왜 지어주는 걸까요? 여러 이유가 있지만, 재사용 측면에서도 생각해보면 좋을 것 같아요. 현재 로그인 페이지와 회원가입 페이지의 UI가 같은 게 많기 때문에, 통일할 클래스명이 몇몇 보입니다.
- 이벤트 위임 패턴과 버블링을 이해하면 현재 코드를 개선 시킬 수 있어요.
- 좋은 사용자 경험(UX)을 위한 기능을 추가해보면 좋을 것 같아요! 예를 들어, 즉각적인 유효성 검사를 추가해보는 건 어떨까요? 필드 입력이 제대로 됐는지 필드를 벗어날 때 알 수 있을 뿐만 아니라, 타이핑하면서 즉시 알 수 있도록 개선 시켜보면 좋을 것 같아요.
There was a problem hiding this comment.
공통 CSS를 auth.css로 분리하신 점, 너무 좋습니다!
폴더 구성을 현재 프로젝트 크기에 맞게 잘 구성해주셨네요. 👍
여기서, 각 클래스에 폰트 패밀리를 선언하지 않고 reset.css같은 파일에서 body 태그에 한번에 적용시키는 건 어떨까요? 텍스트 관련 속성은 body에서 정의 시 기본적으로 자식 요소에 상속되어 적용돼요.
다만 폼 컨트롤 관련 요소는 상속이 되지 않기 때문에 아래 선언들도 필요해요.
input,
button,
textarea,
select {
font-family: inherit;
}최신 CSS Reset에 대한 더 많은 내용은 아래 링크를 참고해 주세요.
| .eye_wrapper_re { | ||
| position: relative; |
There was a problem hiding this comment.
클래스명을 좀 더 직관적인 password_container, password_box, password_wrapper와 같은 이름은 어떨까요?
그리고 .eye_wrapper_re와 .eye_wrapper는 하나의 클래스로 사용해도 좋을 것 같아요!
| const inputSelector = button.dataset.target; | ||
| const input = document.querySelector(inputSelector); | ||
| const img = button.querySelector('.eye_icon_img'); |
There was a problem hiding this comment.
오 dataset 속성을 활용해서 해결 하셨네요. 👍
앞서 말씀드린 것처럼, 비밀번호와 비밀번호 확인 클래스를 통일시키면 이전 방법으로도 아래처럼 구현이 가능해요:
const wrapper = button.closest('.eye_wrapper');
const input = wrapper.querySelector('.password_input');
const img = button.querySelector('.eye_icon_img');| checkButtonState(); | ||
| }); | ||
|
|
||
| passwordInput?.addEventListener("focusout", () => { |
There was a problem hiding this comment.
focusout 이벤트도 구현이 가능하지만, blur 이벤트로 구현도 가능해요. 둘의 차이점은 이벤트 버블링 유무예요.
focusout 이벤트는 버블링이 되지만, blur 이벤트는 버블링이 되지 않습니다. 따라서 현재 코드에서는 각 요소마다 이벤트를 등록하고 있기 때문에, blur 이벤트가 좀 더 적합해요.
그럼 focusout 이벤트는 버블링이 되기 때문에 이벤트 위임 시 사용하면 되겠죠! 😀
| const emailInput = document.querySelector(".email_input"); | ||
| const emailError = document.querySelector(".email_error"); | ||
|
|
||
| const passwordInput = document.querySelector(".password_input"); | ||
| const passwordError = document.querySelector(".password_error"); | ||
|
|
||
| const passwordReInput = document.querySelector(".password_re_input"); | ||
| const passwordReError = document.querySelector(".password_re_error"); | ||
|
|
||
| const nicknameInput = document.querySelector(".nickname_input"); | ||
| const nicknameError = document.querySelector(".nickname_error"); | ||
|
|
||
| const loginBtn = document.querySelector(".login_btn"); | ||
| const signupBtn = document.querySelector(".signup_btn"); |
There was a problem hiding this comment.
회원가입 페이지처럼 입력 필드가 많은 경우, 각 필드에 개별적으로 이벤트 리스너를 등록하면 코드가 금방 복잡해지고, 셀렉터도 많아져요. 여기서 실시간 유효성 검사를 위해 input 이벤트까지 처리하려고 하면 복잡도는 더 커지게 되죠.
이처럼 코드가 늘어나는 상황에서는 이벤트 위임(Event Delegation) 패턴을 적용하는 것이 좋아요. 특정 부모 요소에 단 하나의 이벤트 리스너만 등록하면, 자식 요소에서 발생하는 이벤트들을 효율적으로 처리할 수 있어, 코드를 훨씬 간결하고 유지보수하기 쉽게 만들 수 있습니다. 😀
2 participants
요구사항
기본
로그인
회원가입
심화
주요 변경사항
멘토에게