-
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 all commits
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 +1,114 @@ | ||
//😍CEOS 20기 프론트엔드 파이팅😍 | ||
// 날짜 포맷 | ||
function formatDate(date) { | ||
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()]; | ||
} | ||
|
||
function dateDisplay(date) { | ||
document.querySelector('.date').innerHTML = formatDate(date); | ||
document.querySelector('.day').innerHTML = formatDay(date); | ||
} | ||
|
||
// 투두 렌더링 | ||
function loadTodoList(date) { | ||
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 함수를 잘 구현해주셨네요! 함수의 역할에 맞게 명칭을 잘 설정해주신 것 같습니다. fetch: 데이터를 단순히 가져오는 작업에 집중된 단어로, 데이터를 가져오는 작업만 수행하고, 저장하거나 처리하지 않는 경우에 사용하는 반면, 가빈님께서 구현해주신 loadTodoList 함수는,
지원님 코드리뷰에도 언급했었는데, 함수명이나 변수명을 짓는데 있어 고민되신다면, 카카오 fe 개발자분의 발표 영상 참고해보시면 좋을 것 같아요! |
||
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
|
||
}); | ||
|
||
updateLeftNum(todoList) | ||
} | ||
|
||
// 투두 추가 | ||
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 = ''; | ||
}); | ||
|
||
// 남은 할 일 개수 | ||
function updateLeftNum(todoList) { | ||
const leftNum = todoList.filter(todo => !todo.completed).length; | ||
document.querySelector('.leftNum').innerHTML = `할 일 ${leftNum}개`; | ||
} | ||
|
||
// 날짜 이동 | ||
let currentDate = new Date(); | ||
dateDisplay(currentDate); | ||
loadTodoList(currentDate); | ||
|
||
document.querySelector('img[src*="toYesterday"]').addEventListener('click', () => { | ||
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); | ||
}); | ||
|
||
// 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)); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,94 @@ | ||
/* 본인의 디자인 감각을 최대한 발휘해주세요! */ | ||
@import url("https://cdn.jsdelivr.net/gh/orioncactus/[email protected]/dist/web/static/pretendard.css"); | ||
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. 사실 저는 예전에 @font-face를 통해서 폰트를 직접 호스팅하는 방식으로 코딩했었거든요. 그 때는 사이트가 좀 커졌기 때문에 배포 시 로딩이 오래 걸릴까봐 해당 방법을 택했던 건데, 이 방식에 익숙해진 나머지 전 이번에도 직접 호스팅해서 불러왔어요... 그런데 생각해보니 투두리스트 과제처럼 규모가 작은 프로젝트의 경우에는 가빈 님처럼 CDN 방식으로 가져오는 것이 훨씬 효율적인 것 같네요!! 저도 다음에는 프로젝트의 규모나 배포 속도 등을 전반적으로 고려해서 폰트 불러오는 방식을 다양화해야겠어요 😄 |
||
|
||
* { | ||
margin: 0; | ||
padding: 0; | ||
box-sizing: border-box; | ||
font-family: "Pretendard"; | ||
} | ||
|
||
body { | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
height: 100vh; | ||
background: #F3FAFF; | ||
} | ||
|
||
.container { | ||
display: flex; | ||
justify-content: space-between; | ||
align-items: center; | ||
width: 50%; | ||
min-width: 550px; | ||
max-width: 650px; | ||
margin: 0 auto; | ||
} | ||
|
||
.main { | ||
text-align:left; | ||
background: white; | ||
padding: 7% 15% 7% 15%; | ||
border-radius: 10px; | ||
box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1); | ||
flex-grow: 1; | ||
margin: 0 20px; | ||
} | ||
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. |
||
|
||
.leftNum { | ||
color: #91D1FF; | ||
margin-bottom: 8px; | ||
font-weight: 500; | ||
} | ||
|
||
.day { | ||
margin-top: 8px; | ||
font-weight: 500; | ||
} | ||
|
||
.todoList { | ||
list-style: none; | ||
padding: 0; | ||
margin-top: 20px; | ||
} | ||
|
||
.todoList li { | ||
display: flex; | ||
align-items: center; | ||
padding: 10px 0; | ||
border-bottom: 1px solid #C0C0C0; | ||
margin-top: 5px; | ||
margin-bottom: 5px; | ||
} | ||
|
||
span { | ||
margin-left: 5%; | ||
width: 80%; | ||
} | ||
|
||
.todoList img { | ||
width: 20px; | ||
height: 20px; | ||
cursor: pointer; | ||
} | ||
|
||
.todoInput { | ||
width: 100%; | ||
padding: 10px; | ||
border: 1px solid #91D1FF; | ||
border-radius: 10px; | ||
} | ||
|
||
form { | ||
margin-top: 20px; | ||
} | ||
|
||
img { | ||
cursor: pointer; | ||
} | ||
|
||
img[src*="toYesterday"], | ||
img[src*="toTomorrow"] { | ||
width: 40px; | ||
height: 40px; | ||
} |
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 파일에 li 요소들이 정의되어 있는데, 이미 JS 파일에서
이 부분을 통해 투두리스트를 항목을 동적으로 추가하고 있습니다. 따라서 ul 안에 li 요소를 미리 넣어둔 것은 살짝 불필요하다고 느껴집니다..! JS의 innerHTML에 의해서 미리 정의된 li 요소는 사용되지 않거나 덮어쓰이게 되니까요. 다음과 같이 li 요소를 삭제하고 JS에서 리스트를 동적으로 생성하는 방식으로만 처리하는 것이 어떨까요?