-
Notifications
You must be signed in to change notification settings - Fork 11
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
[1주차] 이가빈 미션 제출합니다. #4
base: master
Are you sure you want to change the base?
Changes from 1 commit
31fd404
9117f2a
3337810
09c8b27
624f086
59999d5
b63ae55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,106 @@ | ||
// 날짜 포맷 | ||
function formatDate(date) { | ||
const year = date.getFullYear(); | ||
const month = (date.getMonth() + 1).toString(); | ||
const day = date.getDate().toString(); | ||
return `${year}년 ${month}월 ${day}일`; | ||
} | ||
const year = date.getFullYear(); | ||
const month = (date.getMonth() + 1).toString(); | ||
const day = date.getDate().toString(); | ||
return `${year}년 ${month}월 ${day}일`; | ||
} | ||
|
||
// 요일 포맷 | ||
function formatDay(date) { | ||
const days = ['일요일', '월요일', '화요일', '수요일', '목요일', '금요일', '토요일']; | ||
return days[date.getDay()]; | ||
} | ||
const days = ['일요일', '월요일', '화요일', '수요일', '목요일', '금요일', '토요일']; | ||
return days[date.getDay()]; | ||
} | ||
|
||
function dateDisplay(date) { | ||
document.querySelector('.date').textContent = formatDate(date); | ||
document.querySelector('.day').textContent = formatDay(date); | ||
} | ||
|
||
document.querySelector('.date').innerHTML = formatDate(date); | ||
document.querySelector('.day').innerHTML = formatDay(date); | ||
} | ||
|
||
// 투두 렌더링 | ||
function loadTodoList(date) { | ||
const todoList = getTodoList(date); | ||
const todoListContainer = document.querySelector('.todoList'); | ||
todoListContainer.innerHTML= ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 코드는 loadTodoList() 함수가 실행될 때마다 innerHTML을 전부 초기화하고 다시 생성하는 방식으로 되어 있습니다. 작은 작업에서는 큰 무리가 없지만, 만약 투두 항목이 많아진다면 렌더링 성능이 저하될 수 있어 보입니다! 모든 투두리스트를 한 번에 삭제하고 추가하는 방식보다, 새로운 투두가 추가되었을 때만 새 li 요소를 DOM에 추가하고 기존 투두가 수정되었을 때는 해당 항목만 업데이트하도록 수정하면 어떨까요? 최대한 기존 DOM을 활용하는 방식으로요..! |
||
|
||
todoList.forEach((todo, index) => { | ||
const todoItem = document.createElement('li'); | ||
const checkedStatus = todo.completed ? 'checked' : 'unchecked'; | ||
const textColor = todo.completed ? '#C0C0C0' : '#000000'; | ||
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이렇게 삼항연산자로 간결하게 표현해주셔서 가독성이 좋네요😊 |
||
|
||
todoItem.innerHTML = ` | ||
<img src="./src/${checkedStatus}.svg" class="toggleComplete"> | ||
<span style="color: ${textColor};">${todo.text}</span> | ||
<img src="./src/deleteBtn.svg" class="deleteBtn" style="display: none;"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. innerHTML 방식을 활용해서 할일을 간단하게 dom에 추가해주셨네요!! innerHTML은 간단하게 dom 조작이 가능하다는 장점이 있지만, 만약 요소 노드에 기존 자식 노드가 존재했다면, 기존 자식 노드들을 제거하고 할당한 html 마크업을 파싱해서 dom에 반영하기 때문에 주의해야 할 필요가 있을 것 같아요! https://tecoble.techcourse.co.kr/post/2021-04-26-cross-site-scripting/ |
||
`; | ||
|
||
// 체크박스 | ||
todoItem.querySelector('.toggleComplete').addEventListener('click', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 각 할 일 항목마다 이벤트 리스너가 반복해서 추가되는 것 같아요! 이번 키 퀘스천이였던, 이벤트 버블링의 개념을 활용해보는 것은 어떨까요?! 이벤트 버블링은 하위 요소에서 발생한 이벤트가 상위 요소로 전파되는 것이므로, 부모 요소(.todoList)에 한 번만 이벤트 리스너를 추가한 후에, |
||
todo.completed = !todo.completed; | ||
saveTodoList(date, todoList); | ||
loadTodoList(date); | ||
}); | ||
|
||
// 삭제 버튼 | ||
todoItem.addEventListener('mouseover', () => { | ||
todoItem.querySelector('.deleteBtn').style.display = 'inline'; | ||
}); | ||
|
||
todoItem.addEventListener('mouseout', () => { | ||
todoItem.querySelector('.deleteBtn').style.display = 'none'; | ||
}); | ||
|
||
todoItem.querySelector('.deleteBtn').addEventListener('click', () => { | ||
todoList.splice(index, 1); | ||
saveTodoList(date, todoList); | ||
loadTodoList(date); | ||
}); | ||
|
||
todoListContainer.appendChild(todoItem); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 가빈님의 코드를 보면, todoList에 forEach를 돌려 각각의 할일마다, DOM에 할일을 추가하고 있는 것 같아요. '만약 자바스크립트 코드에 DOM이나 CSSOM을 변경하는 DOM API가 사용된 경우 DOM이나 CSSOM이 변경된다. 이때 변경된 DOM과 CSSOM은 다시 렌더 트리로 결합되고 변경된 렌더 트리를 기반으로 레이아 appendChild()는 dom에 직접적으로 요소를 추가하는 메서드이기 때문에, 할일 요소별로 이 메서드를 호출한다면 이로 인해 리플로우와 리페인팅이 빈번히 발생해서 서능이 저하될 수 있어요. https://developer.mozilla.org/ko/docs/Web/API/DocumentFragment
|
||
}); | ||
} | ||
|
||
// 투두 추가 | ||
function addTodoItem(date, todoText) { | ||
if (!todoText) return; | ||
|
||
const todoList = getTodoList(date); | ||
const newTodo = { text: todoText, completed: false }; | ||
todoList.push(newTodo); | ||
saveTodoList(date, todoList); | ||
loadTodoList(date); | ||
} | ||
|
||
document.querySelector('#inputForm').addEventListener('submit', (e) => { | ||
e.preventDefault(); | ||
const todoInput = document.querySelector('.todoInput'); | ||
addTodoItem(currentDate, todoInput.value); | ||
todoInput.value = ''; | ||
}); | ||
|
||
// 날짜 이동 | ||
let currentDate = new Date(); | ||
dateDisplay(currentDate); | ||
// 어제 날짜로 이동 | ||
loadTodoList(currentDate); | ||
|
||
document.querySelector('img[src*="toYesterday"]').addEventListener('click', () => { | ||
currentDate.setDate(currentDate.getDate() - 1); | ||
dateDisplay(currentDate); | ||
loadTodoList(currentDate); | ||
currentDate.setDate(currentDate.getDate() - 1); | ||
dateDisplay(currentDate); | ||
loadTodoList(currentDate); | ||
}); | ||
|
||
// 내일 날짜로 이동 | ||
|
||
document.querySelector('img[src*="toTomorrow"]').addEventListener('click', () => { | ||
currentDate.setDate(currentDate.getDate() + 1); | ||
dateDisplay(currentDate); | ||
loadTodoList(currentDate); | ||
currentDate.setDate(currentDate.getDate() + 1); | ||
dateDisplay(currentDate); | ||
loadTodoList(currentDate); | ||
}); | ||
|
||
function loadTodoList(date) { | ||
const todoList = document.querySelector('.todoList'); | ||
todoList.textContent = ''; | ||
} | ||
|
||
|
||
// local storage 관련 코드 | ||
function getTodoList(date) { | ||
const storedTodos = localStorage.getItem(date.toDateString()); | ||
return storedTodos ? JSON.parse(storedTodos) : []; | ||
} | ||
|
||
function saveTodoList(date, todoList) { | ||
localStorage.setItem(date.toDateString(), JSON.stringify(todoList)); | ||
} |
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.
loadTodoList 함수를 잘 구현해주셨네요! 함수의 역할에 맞게 명칭을 잘 설정해주신 것 같습니다.
fetch: 데이터를 단순히 가져오는 작업에 집중된 단어로, 데이터를 가져오는 작업만 수행하고, 저장하거나 처리하지 않는 경우에 사용하는 반면,
load는, 데이터를 가져와서 처리하거나 저장하는 작업까지 포함하는 경우에 적합하다고 하더라구요.
가빈님께서 구현해주신 loadTodoList 함수는,
지원님 코드리뷰에도 언급했었는데, 함수명이나 변수명을 짓는데 있어 고민되신다면, 카카오 fe 개발자분의 발표 영상 참고해보시면 좋을 것 같아요!
https://www.youtube.com/watch?v=emGLxi0LvNI&t=556s