-
Notifications
You must be signed in to change notification settings - Fork 10
[1주차] 장자윤 과제 제출합니다. #10
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
base: main
Are you sure you want to change the base?
Conversation
chaeyoungwon
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.
과제하시느라 고생 많으셨습니다!! 커밋 기록이 다 날아갔다니 정말 아쉽네요.. 🥲
앞으로 과제를 Vercel로 배포하신다면, 배포 과정에서 다양한 오류를 접하실 수 있을 텐데요.
코드 품질을 사전에 관리하기 위해 .husky를 활용해 커밋 단계에서 린트나 테스트를 자동으로 실행해 두는 것도 도움이 될 것 같습니다! 🙌
| <!-- 입력 목록 --> | ||
| <div class="inputContainer"> | ||
| <input placeholder="할 일을 입력해주세요" id="todoInput" required /> | ||
| <span class="appendTodo">+</span> |
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.
button 태그를 사용해도 좋을 거 같아용
| todoInput.addEventListener("keydown", (event) => { | ||
| if (event.key === "Enter") { | ||
| appendTodo.click(); | ||
| } | ||
| }); |
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.
승선님 리뷰에도 남겨놨었는데, 한글 입력 후 엔터로 등록을 할 경우 마지막 글자가 두 번 입력되는 현상이 있습니다!
이 문제는 IME 조합형 입력 중 keydown 이벤트가 조합이 완료되기 전에 먼저 실행되면서 발생하는 이슈인데요.
아래 참고 자료들에서 안내하는 것처럼 KeyboardEvent의 isComposing 속성을 활용하면
이런 상황을 방지할 수 있으니 참고해보시면 좋을 것 같아요 👍
https://developer.mozilla.org/ko/docs/Web/API/KeyboardEvent/isComposing
https://velog.io/@goldfrosch/is-composing
| div.innerHTML = ` | ||
| <input type="checkbox" class="checkboxButton" id="checkbox-${index}" ${ | ||
| todo.checked ? "checked" : "" | ||
| } /> | ||
| <span class="todoContent">${todo.text}</span> | ||
| <button class="deleteButton">x</button> | ||
| `; |
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.
현재 renderTodos 함수에서 innerHTML을 통해 사용자 입력값(todo.text)을 그대로 삽입하고 있는데요,
innerHTML은 입력값을 HTML로 파싱하기 때문에 악성 스크립트가 실행될 수 있는 XSS 보안 취약점을 초래할 수 있습니다!
따라서 createElement, appendChild, textContent를 활용해 해당 코드를 재구현하는 방법도 고려해 보시면 좋겠습니다 🙂
| `; | ||
|
|
||
| const checkbox = div.querySelector(`#checkbox-${index}`); | ||
| const todoContent = div.querySelector(".todoContent"); |
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 todoContent = div.querySelector(".todoContent");
현재 코드에서 선언만 되고 실제로는 사용되지 않고 있는데요, 불필요한 코드는 제거해 주시면 더 좋을 거 같습니다!
| @@ -0,0 +1,161 @@ | |||
| * { | |||
| text-align: center; | |||
| font-family: Pretendard; | |||
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.
Pretendard는 기본 제공 폰트가 아니기 때문에 CDN, @import, @font-face 등의 방식으로 폰트를 불러와야 실제로 적용됩니다!
다음 과제에서 적용해보시면 좋겠네요 :)
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.
전반적으로 rem단위를 사용하신 것이 인상깊네요!
| // 버튼 클릭 시 날짜 변경 | ||
| prevButton.addEventListener("click", () => { | ||
| changeDate(-1); | ||
| console.log("날짜:", currentDate.value); |
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.
console.log는 개발 과정에서 디버깅 용도로 사용되지만, 실제 배포 시에는 불필요하므로 제거해 주시는 것이 좋습니다!
앞으로 프로젝트를 진행하실 때도 참고해 주시면 좋겠습니다 :)
| <button class="dateButton" id="prevButton"><</button> | ||
| <input type="date" id="currentDate" /> | ||
| <button class="dateButton" id="nextButton">></button> |
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.
현재 렌더링에는 문제가 없지만, 부등호는 잠재적으로 태그 시작/종료 기호이므로, 상황에 따라 오동작할 수 있습니다!
따라서 안전하게 표시하려면 < → <, > → >와 같이 HTML 엔티티로 대체해줘도 좋을 거 같아용
|
|
||
| const defaultDate = `${yyyy}-${mm}-${dd}`; | ||
| currentDate.value = defaultDate; | ||
| localStorage.setItem("lastSelectedDate", defaultDate); |
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.
페이지 새로고침 시 마지막으로 선택했던 날짜가 유지되도록 구현해 주신 점이 인상 깊었습니다!
다만, 페이지를 닫고 다시 열었을 때는 오늘 날짜가 기본값으로 보이는 편이 더 자연스러울 수도 있을 것 같습니다. 이 경우 localStorage 대신 sessionStorage를 활용해 보시는 것도 좋겠습니다!!
| <main> | ||
| <!-- 입력 목록 --> | ||
| <div class="inputContainer"> | ||
| <input placeholder="할 일을 입력해주세요" id="todoInput" 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.
required 속성을 넣어주신 점이 인상깊네용 😲
결과 화면
느낀점 및 배운점
Review Questions
DOM (Document Object Model)
이벤트 흐름 제어 (버블링 <-> 캡쳐링)
addEventListener(event, handler)addEventListener(event, handler, {capture:true})addEventListener(event, handler, true)event.stopPropagation() // 더 이상 전파되지 않음event.preventDefault() // 브라우저 기본 동작 막기클로저와 스코프
배포 링크 : https://vanilla-todo-22nd-seven.vercel.app/
+ 깃 명령 관련해서 문제 생겨서 다시 하다가 레포지토리 다 날려먹은 관계로 커밋 기록이 거의 없습니다.. :(