-
Notifications
You must be signed in to change notification settings - Fork 22
Basic 배상빈 sprint4 #38
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
Basic 배상빈 sprint4 #38
The head ref may contain hidden characters: "Basic-\uBC30\uC0C1\uBE48-sprint4"
Conversation
…폼 CSS 도입 및 레이아웃·반응형 스타일 개선
- common.css 추가하여 공통 컴포넌트 분리 - 페이지별 헤더 네이밍 분리 (page-header, form-header) - CSS 변수 확대 (색상, 간격) - 비밀번호 토글 버튼 wrapper 구조 개선 - README 업데이트
- 폼 검증 로직을 설정 기반으로 구현 - 비밀번호 보기/숨기기 토글 기능 구현 - passwordConfirm 검증 로직 개선 (password 비어있을 때 처리) - null 안전성 체크 추가 (optional chaining) - README.md 업데이트 (완료된 작업 반영)
humonnom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 전체적으로 요구사항도 잘 구현하셨고
코드도 깔끔하게 잘 짜셨어요
그런데 form.js에서 하나 건의 드리자면,
UI 업데이트가 두번 일어나게 되는 부분을 수정해보시면 어떨까 하는데요.
UI 업데이트 중복 발생 이유
form.addEventListener("focusout", (e) => {
if (e.target.matches("input")) {
validateInput(e.target); // validateInput에서 UI변경이 일어남
updateButtonState(); // 함수 내부에서 validateInput을 호출하기 때문에 UI변경이 또 발생
}
})UI 업데이트가 두번 일어나는 것 확인하기
// form.js에서 아래 부분을 고치면
// line 58(validateInput)
showError(input, msg + Date.now()); // 에러메세지를 띄울때 시간을 같이 표기
// line 77(focusout 이벤트 리스너)
setTimeout(() => {
updateButtonState(); //
},3000)default.mov
- 입력값을 하나 지워서 7글자로 만듬
- 포커스 아웃 -> 에러 메시지와 함께 ...40로 끝나는 숫자 표시됨(첫번째 UI업데이트)
- 5초후 -> 버튼이 disabled되며 메시지 뒤의 숫자가 ...42로 바뀝니다.(두번째 UI업데이트)
해결방법(pseudo코드)
- UI상태 업데이트 함수와 validation을 담당하는 함수를 분리합니다.
// validate 담당
function validateInput(input) {
// 이 함수에서는 UI업데이트x, validation만 담당
return true or false // 결과를 boolean으로 리턴
}
// 업데이트 담당
function updateMessage(input) {
const isValid = validateInput(input)
if (isValid) { 성공 표시 }
else { 에러 표시 }
}
function updateButtonState() {
const inputs = form.querySelectorAll("input[data-validate]");
const allValid = [...inputs].every(validateInput); // 검증
submitButton.disabled = !allValid; // 버튼 상태만 업데이트
}
form.addEventListener("focusout", (e) => {
if (!e.target.matches("input[data-validate]")) return;
// 현재 필드 메시지 업데이트
updateMessage(e.target)
// 버튼 상태만 업데이트
updateButtonState();
});이렇게 바꾸면 에러 메시지 UI 업데이트가 한번만 일어나게 할 수 있을 것 같은데
한번 수정을 고려해주세요.
나머지는 코멘트로 남겨두었으니 참고 부탁드릴게요!
| } | ||
|
|
||
| const messages = { | ||
| required: "필수 입력 항목입니다.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 강사님 :)
해당부분 확인했습니다.
계속 변경하고 리팩토링하는 과정에서 요구사항을 놓친거같습니다.
이 부분은 조금 더 신경쓰겠습니다.
이전 요구사항도 몇가지 놓쳤던것같네요.
에러메시지가 2번 호출되는현상도 분리하여 처리하겠습니다.
확인감사합니다.
| </div> | ||
| </div> | ||
| </div> | ||
| <script src="../assets/utility/validation.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(필수) 사용하지 않는 js 파일은 가져오지 않도록 수정해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당부분은 기존에 별도 함수로 분리해서 가지고 있다가 프로젝트가 작아서 하나로 합치는 과정에서 수정누락되었습니다.
모듈형태로 리팩토링하여 수정하도록 하겠습니다 :)
| const isValid = param ? fn(value, param, input) : fn(value); | ||
|
|
||
| if (!isValid) { | ||
| const msg = typeof messages[name] === "function" ? messages[name](param) : messages[name]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const msg = typeof messages[name] === "function" ? messages[name](param) : messages[name]; | |
| const msg = messages[name](param) |
- messages를 아래처럼 함수 형태로 통일하면, type을 검사할 필요가 없어지지 않을까요?
const messages = {
required: () => "필수 입력 항목입니다.",
email: () => "이메일 형식이 올바르지 않습니다.",
min: (len) => `최소 ${len}자 이상 입력해주세요.`,
max: (len) => `최대 ${len}자 이하 입력해주세요.`,
match: () => "비밀번호가 일치하지 않습니다.",
};- 그리고 만약에 함수형태로 통일한다면, 이름도 바뀌면 좋을 거 같네요. 이름만 보고도 함수라는 걸 알 수 있게요.
const getErrorMessage = {...}- 아니면 switch case로 바꿔도 될 것 같네요.
function getErrorMessage(name, param) {
switch (name) {
case "required": return "필수 입력 항목입니다.";
case "email": return "이메일 형식이 올바르지 않습니다.";
case "min": return `최소 ${param}자 이상 입력해주세요.`;
case "max": return `최대 ${param}자 이하 입력해주세요.`;
case "match": return "비밀번호가 일치하지 않습니다.";
default: return "";
}
}
// 사용부
const msg = formatValidationMessage(name, param);| <div class="form-block__input-wrapper"> | ||
| <input class="form-block__input" type="password" id="password" name="password" placeholder="비밀번호를 입력해주세요." data-validate="required|min:8|max:20"> | ||
| <button class="form-block__button-eye" type="button"> | ||
| <img class="form-block__button-eye-icon" src="../assets/images/icons/eye.svg" alt="눈 열기"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(권장) a가 아닌 button으로 감싸주신것 잘하셨는데, 조금 더 명확한 의미 전달이 되면 좋을 것 같네요.
동작 중심으로 짧게 서술해주시면 좋아요.
• 권장: “비밀번호 표시” 또는 “비밀번호 숨기기”
토글 상태에 따라 바꾸면 더 좋습니다. 버튼이 비밀번호를 보여주는 상태면 “비밀번호 숨기기”, 숨긴 상태면 “비밀번호 표시”
| <span class="oauth-block__text">간편 로그인하기</span> | ||
|
|
||
| <div class="oauth-block__group"> | ||
| <a class="oauth-block__button" href="https://www.google.com/" target="_blank"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <a class="oauth-block__button" href="https://www.google.com/" target="_blank"> | |
| <a class="oauth-block__button" href="https://www.google.com/" target="_blank" aria-label="Google로 로그인"> |
(질문) 이렇게 aria-label 사용하면 스크린리더 대응은 충분할 것 같은데, sr-only 적용된 span 사용하신 이유가 따로 있을까요?

폼 검증 로직 리팩토링
📋 변경 사항 요약
폼 검증 로직을
config객체 기반에서data-validate속성 기반의 선언적 검증 시스템으로 리팩토링했습니다.🎯 주요 개선 사항
1. 선언적 검증 시스템 도입
변경 전:
config객체로 login/signup 분리 관리form.dataset.type으로 폼 타입 구분validator함수와isInvalid플래그로 상태 관리변경 후:
data-validate="required|email|max:50"형태로 검증 규칙 선언rules객체로 규칙 기반 검증 (required, email, min, max, match)validateInput()함수로 각 input을 독립적으로 검증2. 코드 구조 개선
invalid/valid클래스로 시각적 피드백updateButtonState()함수로 모든 필드 검증 후 버튼 활성화/비활성화3. 검증 규칙 예시
📁 변경된 파일
assets/js/form.js- 검증 로직 리팩토링pages/login.html-data-validate속성 추가pages/signup.html-data-validate속성 추가README.md- 변경사항 문서화✅ 테스트 체크리스트
🔍 변경 전후 비교
변경 전 구조
변경 후 구조
💡 장점
data-validate속성으로 인해 HTML과 JavaScript가 밀접하게 결합됨data-validate속성의 자동완성 및 타입 체크 지원이 제한적rules객체에 추가하고, HTML의data-validate속성에도 규칙을 추가해야 함. 조건이 복잡해질수록 유지보수 비용이 증가함🚀 향후 개선 계획
1. name 기반 검증으로 전환
data-validate속성 기반 검증name속성을 기준으로 각 필드를 명시적으로 검증focusout이벤트에서e.target.name을 기준으로 분기 처리2. 각 input에 직접 이벤트 추가
focusout이벤트를 한 곳에서 처리3. 검증 규칙 모듈화
rules객체가form.js내부에 포함4. input type별 이벤트 처리 분기
focusout이벤트만 처리text,email,password등:focusout이벤트checkbox,radio:change이벤트select:change이벤트