[문주영] Sprint4 #162
[문주영] Sprint4 #162
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
주영님 4번째 미션 제출 고생하셨습니다!
반복을 줄이시려고 하신 점과 이름에 대해 고민하신 점이 잘 보이는 코드였어요 👍
이렇게 다음 미션도 화이팅입니다!
-
현재는 로그인/회원가입 페이지에 에러 메세지를 띄우는 것을 js파일에서 요소를 만들어서 추가하는 방식으로 구현하였는데, input란 아래에 미리 에러 메세지에 대한 내부가 빈 div 태그를 만들어 놓고 해당 요소의 textContent를 변화시키는 방식으로도 구현이 가능할 것 같습니다. 둘 중 어떤 방법을 더 권장하실까요?:
지금 요구사항에서는 html에 error가 들어갈 tag를 미리 만들어두시고 해당 태그의 노출 여부와 내부 문자열을 에러 상황에 따라 바꿔주는 방법을 추천드립니다. -
하단 버튼 (로그인, 회원가입)의 페이지 이동을 type="button" onclick="location.href='/login.html'" 속성으로 구현하였는데 type을 submit이 아니더라도 문제가 되나요? (데이터를 보낼 수 있나요?) submit type을 고수해야 한다면 구현할 다른 방법은 어떤 것이 있는지 궁급합니다.:
질문을 잘 이해하지 못하겠습니다. 버튼의 타입이 submit이어야 form 요소에서 제출이벤트가 발생할시 해당 버튼의 onclick 이벤트가 실행되고, 해당 버튼이 click 되엇을 때도 form이 제출됩니다. 타입을 button으로 하셔야하는 이유가 있나요? 또한 구현할 다른 방법이 있냐고 하셨는데 원하시는 것이 어떤 것인지 몰라 답변드리기 어려울 것 같습니다~ 더 궁금하시다면 DM으로 질문해주세요!
| 판다마켓이 처음이신가요? <a href="/signup.html">회원가입</a> | ||
| </div> | ||
| </main> | ||
| <script src="scripts/auth.js"></script> |
There was a problem hiding this comment.
💊 제안
아마 script 태그를 HTML의 하단에 배치하신 이유가 script가 문서의 렌더링을 막지 않도록 하기 위해서이실 것 같아요.
하지만, script 태그에 defer나 async 속성을 사용하면 이런 문제를 해결할 수 있기 때문에 반드시 하단에 배치할 필요는 없습니다!
또한 script 태그는 상단에 있는게 구조 파악에서도 유리하기 때문에 상단 head 태그에 두시는 것을 추천드려요~
지금과 같은 경우 DOM을 조작하는 JS 이니 defer 속성을 추가하시면 되겠습니다~
There was a problem hiding this comment.
💊 제안
한 파일에서 로그인, 회원가입 공용으로 JS를 작성해주셨네요~
둘이 공통되는 부분이 많이 이렇게 해주신 것 같아요.
다만 그렇게 되면 코드가 복잡해져서 유지보수성이 떨어지고 로그인 페이지의 경우 불필요한 코드까지 알아야합니다~
가능하면 공통 로직들은 분리해주시고 각 페이지별 JS를 따로 만드시는 것을 추천드려요!
|
|
||
| const passwordContainers = document.querySelectorAll(".password-container"); | ||
|
|
||
| function wrongInput(node, text){ |
There was a problem hiding this comment.
💊 제안
해당 파일의 이름들을 보면서 주영님이 고민하시고 네이밍을 하셨다는것이 느껴집니다~
다만 일반적으로 함수명은 동사 + 대상의 방식으로 이루어집니다. 해당 함수명은 에러 메시지를 나오게 하는 함수의 동작과는 잘 어울리지 않는 것 같아요.
showErrorMessage나 validateInput과 같은 이름을 추천드려요!
| const inputs = document.querySelectorAll("form input"); | ||
| const button = document.querySelector("button.complete-btn"); | ||
|
|
||
| const email = document.querySelector("input#email"); |
There was a problem hiding this comment.
💊 제안
email이라는 변수명은 변수의 타입을 알기 어려우므로 emailInput 같이 더 해당값을 잘 설명하는 이름으로 바꿔주시는 것을 추천드립니다.
|
|
||
| let wrongMessage; | ||
| if (node.nextElementSibling?.tagName === 'DIV'){ | ||
| wrongMessage = node.nextElementSibling; |
There was a problem hiding this comment.
💊 제안
이렇게 접근하시는 것도 나쁘지 않지만 이는 HTML 구조와 긴밀하게 연관되게 되므로,
HTML에서 위치만 변경되어도 작동되지 않을 위험이 커집니다~
가능하면 부모요소로 접근해 해당 요소 내에서 재탐색을 해보세요.
| let wrongMessage; | ||
| if (node.nextElementSibling?.tagName === 'DIV'){ | ||
| wrongMessage = node.nextElementSibling; | ||
| } else { | ||
| wrongMessage = document.createElement('div'); | ||
| wrongMessage.setAttribute("class","wrong-message"); | ||
| } |
There was a problem hiding this comment.
💊 제안
로직이 복잡한 부분이 있다면 함수로 분리하시면 코드가 더 명확해집니다.
function getErrorMessageTag(node) {
if (node.nextElementSibling?.tagName === 'DIV') return node.nextElementSibling;
const errorMessage = document.createElement('div');
errorMessage.setAttribute("class", "wrong-message");
return errorMessage;
}
function wrongInput(node, text) {
node.classList.add("wrong");
node.classList.remove("correct");
const errorMessage = getErrorMessageTag(node);
errorMessage.textContent = text;
if (!node.nextElementSibling?.isSameNode(errorMessage)) {
node.after(errorMessage);
}
}| } | ||
|
|
||
| form.addEventListener("focusout", () => { | ||
| for (let input of inputs) { |
There was a problem hiding this comment.
💊 제안
for of 문을 쓰실때 재할당하는 것이 아니라면 const 로 선언해주세요!
| for (let input of inputs) { | |
| for (const input of inputs) { |
| button.removeAttribute("disabled"); | ||
| }); | ||
|
|
||
| passwordContainers.forEach( (node) => { |
There was a problem hiding this comment.
❗️ 수정요청
| passwordContainers.forEach( (node) => { | |
| passwordContainers.forEach((node) => { |
| node.addEventListener("click", (e) => { | ||
| if (e.target.tagName === "IMG") { | ||
| if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){ |
There was a problem hiding this comment.
💊 제안
if 문이 중첩되면 가독성이 떨어지므로 아래처럼 early return문을 사용하시는 것을 추천드려요.
| node.addEventListener("click", (e) => { | |
| if (e.target.tagName === "IMG") { | |
| if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){ | |
| node.addEventListener("click", (e) => { | |
| if (e.target.tagName !== "IMG") return; | |
| if (e.currentTarget.firstElementChild.getAttribute("type") === "password"){ |
2 participants
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게
type="button" onclick="location.href='/login.html'"속성으로 구현하였는데 type을 submit이 아니더라도 문제가 되나요? (데이터를 보낼 수 있나요?) submit type을 고수해야 한다면 구현할 다른 방법은 어떤 것이 있는지 궁급합니다.