[1주차] 이윤서 과제 제출합니다.#2
Conversation
ryu-won
left a comment
There was a problem hiding this comment.
1주차 과제하시느라 고생많으셨습니다! 👍 시간이 많이 부족하셨을텐데 선택요건도 빠짐없이 챙기고, 나아가 테마 전환까지 구현해 완성도 높은 결과물을 보여주셨네요!
| --add-btn-hover-bg: #5c7348; | ||
| } | ||
|
|
||
| /* 다크 테마 - 블루베리 컨셉 */ |
| li.innerHTML = ` | ||
| <div class="todo-left"> | ||
| <label class="checkbox-container"> | ||
| <input type="checkbox" ${todo.completed ? "checked" : ""} onclick="toggleTodo(${index})"> |
There was a problem hiding this comment.
지금 todo 아이템 하나하나마다 전부 이벤트가 붙고 있네요. todo아이템이 많아질 수록 그만큼 이벤트핸들러가 계속 생기는데, 부모에 이벤트 리스너를 하나만들어서 버블링(이벤트 위임)으로 처리하는 방식도 있을 것 같아요!
| margin-right: 10px; | ||
| } | ||
|
|
||
| #todo-text { |
There was a problem hiding this comment.
요건 id가 아니라 class로 변경하는 게 좋을 것 같아요! todo 아이템이 여러개일 수 있는데 한 페이지에 딱 하나만 존재해야 하는 id 와 맞지 않는 것 같아요!
| <button type="submit" id="add-btn">등록</button> | ||
| </form> | ||
|
|
||
| <p id="todo-list"></p> |
There was a problem hiding this comment.
여기서 p태그 보단 ul태그가 더 적합할 것 같아요! renderTodo 에서는 안에 li태그만 만들고 있어서요!
| `; | ||
|
|
||
| li.querySelector(".todo-text").textContent = todo.text; | ||
| todoListElement.appendChild(li); |
There was a problem hiding this comment.
forEach로 하나씩 appendChild 하면, 아이템이 100개면 100번의 DOM 조작이 발생해요.
const fragment = document.createDocumentFragment();
currentTodos.forEach((todo, index) => {
const li = document.createElement("li");
// ... li 구성 동일 ...
fragment.appendChild(li);
});
todoListElement.innerHTML = "";
todoListElement.appendChild(fragment);
DocumentFragment를 사용해서 마지막에 한번만 DOM 조작하는 방법은 어떨까요? DocumentFragment는 메모리 안에서만 존재하는 가상 컨테이너인데, 거기다 다 쌓아두고 마지막에 한 방에 넣으면 리페인팅이 1번만 일어나서 DOM조작 비용을 아낄 수 있습니당! 물론 현재 todo 개수가 많지 않다면 체감 차이는 크지 않을 수 있어서 참고만 해주세용
| <button class="delete-btn" onclick="deleteTodo(${index})">삭제</button> | ||
| `; | ||
|
|
||
| li.querySelector(".todo-text").textContent = todo.text; |
There was a problem hiding this comment.
여기서는 innerHTML로 하지 않고 textCotent로 해서 보안에 신경 써 주셨네요!
| :root { | ||
| --bg-color: #fff5f0; | ||
| --text-color: #5d4037; | ||
| --container-bg: #ffffff; | ||
| --accent-color: #ffab91; | ||
| --button-bg: #ffccbc; | ||
| --delete-btn-bg: #e65272; | ||
| --delete-btn-hover-bg: #c4415b; | ||
| --add-btn-bg: #799567; | ||
| --add-btn-hover-bg: #5c7348; | ||
| } | ||
|
|
There was a problem hiding this comment.
css 변수 활용하셨네요! 시간 부족하셨을텐데 꼼꼼하게 잘 챙기셨습니당👍
| const remainingTodos = currentTodos.filter((todo) => !todo.completed); | ||
| countElement.textContent = `남은 할 일 ${remainingTodos.length}개`; |
There was a problem hiding this comment.
filter 함수 대신 reduce 함수를 사용하는 방법도 있을 것 같아요!
현재 renderTodo는 여러 곳에서 호출되고 있는데, 호출될 때마다 filter로 새로운 배열을 만들고 있어요.
이 배열은 length 확인 후 바로 버려지기 때문에 메모리 낭비가 있을 수 있거든요!
| const remainingTodos = currentTodos.filter((todo) => !todo.completed); | |
| countElement.textContent = `남은 할 일 ${remainingTodos.length}개`; | |
| const remainingCount = currentTodos.reduce((acc, todo) => acc + (todo.completed ? 0 : 1), 0); | |
| countElement.textContent = `남은 할 일 ${remainingCount}개`; |
신경 썼던 점
1. 초기 방식
할 일 목록의 개수가 유동적인 상황을 고려하여, innerHTML을 이용해 템플릿 리터럴로 전체 구조를 한 번에 삽입하는 방식을 선택
2. 고민
전체 HTML 구조를 매번 문자열로 파싱하여 다시 그리는 것은 리소스 낭비가 발생할 수 있다고 판단
보안에 있어서도 XSS에 취약하다는 것을 알게됨
3. 개선 방안
UI 틀과 동적인 text를 분리하여 렌더링하도록 리팩토링을 진행
기타 느낀 점
기능이 추가될수록 HTML 구조를 문자열로 관리하는 것이 복잡해졌고, 특히 UI와 로직이 결합되어 있어 유지보수가 어려웠습니다. 코드가 길어지면서 한 눈에 어떤 코드인지 확인할 수 없는 점이 불편하게 느껴지기도 했습니다. 왜 React 같은 프레임워크가 컴포넌트 단위의 개발을 지향하는지 깨닫게 되었습니다.
배포 링크
https://week1-vanilla-todo-23rd.vercel.app/todo.html
Review Questions
DOM
이벤트 흐름 제어 (Bubbling & Capturing)
event.stopPropagation()을 사용하면 핸들러에서 이벤트를 처리하고 난 후의 버블링을 막을 수 있다addEventListener함수의 세 번째 인자에{ capture: true }또는 간단히true를 전달하면 해당 리스너는 캡처링 단계에서 실행Closure & Scope