[이수정] Sprint 4 #41
[이수정] Sprint 4 #41
Conversation
There was a problem hiding this comment.
수정님~! 이번 미션도 멋지게 마무리 하셨네요 💯
코드를 구조화하시려는 모습이 보기 좋습니다!! dom 접근과 관련해 조금 더 명시적인 방법을 사용하면 좋을 거 같습니다 :)
<script type="module" src="./scripts/login.js"></script>과 같이 type="module" 속성을 주어야만 login.js 파일에서 /common/members.js 파일을 불러올 수 있는 것으로 보입니다. 해당 속성을 사용해야만 하는 이유가 궁금합니다.
-> 모듈은 비교적 최근에 나온 문법입니다! 본래는 모든 자바스크립트 파일이 전역 스코프에서 실행되어 왔어요. 모듈 환경이 지원되기 시작했고, 브라우저에게 해당 파일을 모듈로 해석하라고 명시해주는 것이 말씀하신 속성의 역할입니다 :)
공통함수 members.js가 코드량이 많다보니, 더 효율적이고 가독성 좋게 바꿀 수 있는 방법이 있을지 궁금합니다.
-> 함수나 변수 이름을 잘 지어주시고 추상화를 잘 해주시면 가독성에 많은 도움이 됩니다
. 추상화 관련해서는 리뷰에도 남겨드렸어요 :)
그리고 하나의 함수는 하나의 일을 하는 것이 이상적입니다! (언제나 그럴 수는 없지만요)
정의 해주신 checkValidation을 예로 들면 유효성 검사도 하고 에러 UI도 핸들링하고 있죠. checkValidation을 사용할 때 UI까지 수정하고 있다는 것을 알 수 있는 이름으로 지어주시거나, 로직을 분리하시는 게 좋을 거 같네요! 또한 모든 유효성 검사를 한 번에 하고 있다보니, 추가 사항이 생길 때 마다 함수가 비대해지고 재사용이 힘들어지는 구조를 가지고 있습니다. 🤔
login.js, signup.js 두 파일이 비슷하게 생긴 점, signup.html에서 두 스크립트 파일을 불러오는 점에 개선점이 있을지 궁금합니다.
-> 지금도 충분히 잘하셨습니다 :) 위 말씀드린대로 함수를 작게 쪼개고, 공통된 부분을 모아 구조화하면 재사용성을 늘릴 수 있습니다! 다만, 지금은 자바스크립트 개념 학습에 충실하셔도 충분하다고 생각합니다. 아래 세 가지 정도만 조금씩 실천해주세요~! :)
- 함수, 변수명 잘 짓기
- 단일 책임(되로록 함수는 하나의 일을 하도록)
- 함수로 코드 가독성 높이기
| export function onVisibilityChange(e) { | ||
| if (e.target.localName !== "button") return; | ||
| const button = e.target; | ||
| const input = document.getElementById(button.id).previousElementSibling; |
There was a problem hiding this comment.
비밀번호 숨기는 로직을 하나의 함수로 처리 하셨군요! 👍
There was a problem hiding this comment.
html dataset을 이용해서 input id를 가져오는 방법도 있겠네요~! (참고만 해주세요!)
| return; | ||
| } | ||
|
|
||
| const inputField = input.parentElement?.parentElement; |
There was a problem hiding this comment.
이런 부분들은 가독성을 위해 함수로 추상화 하시면 좋습니다~!
handleErrorMessage 같은 함수 내에 해당 로직을 정리해놓으면, 함수 이름만 보고 여기서 에러 메세지를 다루는구나~ 하고 이해할 수 있지만 지금의 경우는 한 줄 한 줄 읽어야하죠 :)
나중에 에러 메세지 관련된 부분을 이해해야 하거나 수정해야 할 때는 해당 함수만 찾아가면 되구요!
| const inputField = input.parentElement?.parentElement; | ||
| const errorParagraph = document.getElementsByClassName(className)[0]; | ||
|
|
||
| if (result) errorParagraph && inputField?.removeChild(errorParagraph); |
There was a problem hiding this comment.
약간은 복잡한 느낌이 있는데,
<span id="email-error"></span>
미리 html에 정의하고 display, content만 수정할 수도 있을 거 같네요!
| let isEmptyInput = false; | ||
| const form = document.getElementsByTagName("form")[0]; | ||
| for (let inputField of form.children) { | ||
| if (!inputField.classList.value.includes("input-field")) break; |
There was a problem hiding this comment.
위와 마찬가지 입니다 :) 추상화로 가독성을 높여주세요!
| default: | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
previousElementSibling, parentsElement 등을 사용해서 dom 접근하는 코드가 많이 보이는데 dataset, id 등을 좀 더 활용해 보셔도 좋을 거 같습니다 :)
기존에 사용하고 계시는 dom 접근법은 간단하게 사용하면 괜찮지만, 갈수록 유지 보수가 어려워집니다!
HTML 구조에 따라 코드가 쉽게 깨지는 문제가 있고 html 구조를 같이 봐야 하기 때문에 코드를 이해하기가 힘들어지죠 😢
우선 참고만 해주세요~!
|
참고 코드 남겨드립니다. 공통된 부분을 구조화해서 재사용성을 높여 필요한 부분만 import 할 수 있습니다. 모든 로직을 이해하실 필요도 없고, 이렇게도 하는구나~하고 대충 보시면 될 거 같아요! 🤣 // -------- members.js -------
function validateAndUpdateUI() {
const value = this.$element.value;
this.error = this.validations.find((validation) => {
return !validation.test(value);
})?.message;
this.error
? this.$element.classList.add("error")
: this.$element.classList.remove("error");
this.$errorElement.innerHTML = this.error || "";
}
export const email = {
$element: document.getElementById("email"),
$errorElement: document.getElementById("email-error"),
validations: [
{
test: (value) => !!email,
message: "비어있습니다",
},
{
test: (value) =>
/^[A-Za-z0-9._%-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/.test(email),
message: "잘못된 이메일입니다",
},
],
error: null,
isDirty: false,
validateAndUpdateUI,
};
export const password = {
$element: document.getElementById("password"),
$errorElement: document.getElementById("password-error"),
validations: [
{
test: (value) => value.length === 0,
message: "비었음",
},
{
test: (value) => value.length >= 8,
message: "8글자 이상 ",
},
],
error: null,
isDirty: false,
validateAndUpdateUI,
};
//... 중략
export const updateSubmitButtonState = (formElements) => {
const submitButton = document.querySelector("#submit");
const formValid = Object.values(formElements).every(
(element) => !element.error && element.isDirty
);
submitButton.disabled = !formValid;
};
export const handleInputForm = (formElements) => (e) => {
const { id } = e.target;
const targetElement = formElements[id];
targetElement.isDirty = true;
targetElement.validateAndUpdateUI();
updateSubmitButtonState(formElements);
};
// --------login.js------
import { email, password, updateSubmitButtonState, handleInputForm } from './common/members.js'
const formElements = {
email,
password,
};
const handleInputEvent = handleInputForm(formElements);
const $form = document.querySelector("form");
$form.addEventListener("input", handleInputEvent);
```; |
2 participants
04. 스프린트 미션 4
요구사항
스프린트 미션 4 시안
기본 요구사항
체크리스트 [기본]
로그인
이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
input 에 빈 값이 있거나 에러 메세지가 있으면 ‘로그인’ 버튼은 비활성화 됩니다.
Input 에 유효한 값을 입력하면 ‘로그인' 버튼이 활성화 됩니다.
활성화된 ‘로그인’ 버튼을 누르면 “/items” 로 이동합니다
회원가입
체크리스트 [심화]
주요 변경사항
스프린트 미션 3 리뷰 반영
<p>태그는 문단을 나타낼 때만 사용으로 변경<input type="checkbox" />에서<button type="button" />사용으로 변경signup.css의 중복되는 스타일 제거스프린트 미션 4
스크린샷
로그인
회원가입
멘토에게
<script type="module" src="./scripts/login.js"></script>과 같이type="module"속성을 주어야만login.js파일에서/common/members.js파일을 불러올 수 있는 것으로 보입니다. 해당 속성을 사용해야만 하는 이유가 궁금합니다.signup.html에서 두 스크립트 파일을 불러오는 점에 개선점이 있을지 궁금합니다.