-
Notifications
You must be signed in to change notification settings - Fork 11
[1주차] 원채영 미션 제출합니다. #6
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
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.
테마를 낮과 밤모드를 설정 할 수 있는 것이 맘에 들었습니다!! 이것 역시 부드럽게 전환되어 보는 눈이 편해진 것 같기도 합니당
특히 카톡과 비슷하게 해놓은 점이 신선하게 느껴지기도 하였습니다 ฅ՞•ﻌ•՞ฅ
프린텐다드 체가 카톡이랑 비슷해서 잘 어우러진다고 느끼기도 한 것 같네용
햄버거 메뉴 구현도 transition 사용해서 깔끔하게 되어 있어서 정말 좋았습니다!
게다가 전반적으로 코드 구성이나 주석이 잘 작성되어 있어 보기에 편했습니다
아까 쓰다가 실수로 submit를 날려서 새로씁니다 ... 바닐라 JS 처음이라 리뷰가 미숙한 점은 양해 부탁드리겠습니다!
function loadTheme() { | ||
const isDarkMode = JSON.parse(localStorage.getItem("darkMode")); | ||
body.classList.toggle("dark-mode", isDarkMode); | ||
themeBtn.textContent = isDarkMode ? "☀️" : "🌙"; | ||
} | ||
|
||
function toggleTheme() { | ||
const isDarkMode = body.classList.toggle("dark-mode"); | ||
themeBtn.textContent = isDarkMode ? "☀️" : "🌙"; | ||
localStorage.setItem("darkMode", isDarkMode); | ||
} |
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.
테마 관련 코드들을 theme.js에서 관리하는 건 어떨까 싶습니당
todos = todos.map((todo) => | ||
todo.id === id ? { ...todo, completed: isCompleted } : todo | ||
); |
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.
map으로 새로운 배열을 만드는 것보다 find를 사용해서 그부분만 수정해주는 건 어떨까용
if (completed) { | ||
todoText.style.textDecoration = "line-through"; | ||
todoText.style.color = "gray"; | ||
} |
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.
스타일을 직접변경해주는 것 보다 classList add를 사용해서 바꿔주면 유지보수에 더 용이할 것 같습니다!
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.
저도 동의합니다. css 파일에 관련 코드를 명시해두고, classList에서 add, remove를 해주는 방식이 렌더링 프로세스에 있어서도 불필요한 relayout, repaint 등의 작업을 줄여줍니다!
const input = document.querySelector(".input"); | ||
const enterButton = document.querySelector(".enter"); | ||
const todoContainer = document.querySelector(".container"); | ||
const currentDateSpan = document.getElementById("currentDate"); | ||
const prevButton = document.getElementById("prev"); | ||
const nextButton = document.getElementById("next"); | ||
const datePicker = document.getElementById("datePicker"); | ||
const themeBtn = document.querySelector(".theme"); | ||
const body = document.body; | ||
const homeButton = document.querySelector("h2"); | ||
const sidebar = document.querySelector(".sidebar"); | ||
const hamburger = document.querySelector(".hamburger"); | ||
const closeBtn = document.querySelector(".close"); | ||
const todoCountSpan = document.getElementById("todo-count"); |
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.
돔 요소를 그룹화해서 구분하면 가독성이 더 올라갈 듯 합니다
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.
말씀해주신 것처럼 관련 코드는 전체적인 파일의 가독성을 낮출 수가 있어요. 따라서, 다른 parseDom.js 같은 방식으로 해당 코드들을 옮겨두고 모두 export를 해준 뒤, import 문으로 구조 분해하여 받아 사용하는 것은 어떨까요?
다만 그 과정에서 dom의 생성 이전에 script 태그로 들어간 Javascript 파일이 실행되면 예상과 다르게 실행될 수 있으니 async, defer 등의 방법을 적용해보셔두 좋구요. 관련 링크 참고하시면 좋을 것 같습니다.
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.
20기 운영진을 했던 19기 출신 김승완이라고 합니다! 과제 진행하시느라 고생 정말 많으셨고, 콜백이나 함수형 프로그래밍도 잘 활용하시고, 전체적인 스타일링을 css로 잘 정리해두시는 모습이 너무 좋았던 것 같습니다.
동기분들 코드 리뷰도 열심히 해주시고, 리뷰를 받은 것에 대해 항상 리팩터링까지 해보는 경험을 하셨으면 좋겠습니다 :)
const input = document.querySelector(".input"); | ||
const enterButton = document.querySelector(".enter"); | ||
const todoContainer = document.querySelector(".container"); | ||
const currentDateSpan = document.getElementById("currentDate"); | ||
const prevButton = document.getElementById("prev"); | ||
const nextButton = document.getElementById("next"); | ||
const datePicker = document.getElementById("datePicker"); | ||
const themeBtn = document.querySelector(".theme"); | ||
const body = document.body; | ||
const homeButton = document.querySelector("h2"); | ||
const sidebar = document.querySelector(".sidebar"); | ||
const hamburger = document.querySelector(".hamburger"); | ||
const closeBtn = document.querySelector(".close"); | ||
const todoCountSpan = document.getElementById("todo-count"); |
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.
말씀해주신 것처럼 관련 코드는 전체적인 파일의 가독성을 낮출 수가 있어요. 따라서, 다른 parseDom.js 같은 방식으로 해당 코드들을 옮겨두고 모두 export를 해준 뒤, import 문으로 구조 분해하여 받아 사용하는 것은 어떨까요?
다만 그 과정에서 dom의 생성 이전에 script 태그로 들어간 Javascript 파일이 실행되면 예상과 다르게 실행될 수 있으니 async, defer 등의 방법을 적용해보셔두 좋구요. 관련 링크 참고하시면 좋을 것 같습니다.
currentDateSpan.textContent = formatDate(currentDate); | ||
datePicker.value = currentDate; | ||
updateTodoCount(); | ||
} | ||
|
||
// 할 일 리스트 불러오기 (로컬 스토리지) | ||
function loadTodos() { | ||
todoContainer.innerHTML = ""; |
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.
좀 드문 경우이기는 한데, innerHTML 속성을 이용하는 것은 xss
공격에 취약점을 보일 수 있어요. 따라서 위에서 해주신 것처럼 textContent 속성을 이용하시면 보안적으로 더 우수하다고 생각합니다.
if (completed) { | ||
todoText.style.textDecoration = "line-through"; | ||
todoText.style.color = "gray"; | ||
} |
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.
저도 동의합니다. css 파일에 관련 코드를 명시해두고, classList에서 add, remove를 해주는 방식이 렌더링 프로세스에 있어서도 불필요한 relayout, repaint 등의 작업을 줄여줍니다!
function addTodo() { | ||
const text = input.value.trim(); | ||
if (!text) return; | ||
|
||
const todos = JSON.parse(localStorage.getItem(currentDate)) || []; | ||
const newTodo = { id: crypto.randomUUID(), text, completed: false }; | ||
|
||
todos.push(newTodo); | ||
localStorage.setItem(currentDate, JSON.stringify(todos)); | ||
addTodoElement(newTodo.id, text); | ||
input.value = ""; | ||
updateTodoCount(); | ||
} |
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.
불필요한 텍스트의 경우 입력을 무시해버리게 early return
을 잘 활용하셨네요! 이 방식도 충분히 좋고, 아예 button 요소의 disabled
속성을 이용하셔도 좋다고 생각합니다. 해당 입력을 addTodo()
함수가 씹어버리게 하는 것은 조건을 따로 변수로 잡고 disabled
속성에 부여해주는 것이 매우 좋다고 생각합니다.
개인적으로 html 요소에 있는 내장 속성을 최대치로 활용하는 것이 유지 보수에 있어서 더 깔끔하더라구요!
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.
카카오톡 오리지널 테마 같은 색 조합과 다크 모드를 구현하신 것이 인상 깊었어요! 적용하신 트랜지션들이 너무 아름답습니다. 부족한 실력으로 코드 리뷰해봤습니다😊
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 todoCountSpan = document.getElementById("todo-count"); | ||
|
||
// 현재 선택된 날짜 | ||
let currentDate = new Date().toISOString().split("T")[0]; |
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.
.toISOString().split("T")[0]를 세 번 사용하셨는데 개발 중 다른 format을 선택하게 될 경우를 대비해 format 함수로 분리해봐도 좋을 거 같아요~!
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.
할 일을 화면에 표시할 때(line 37), 추가할 때(line 78), 삭제할 때(line 90), 완료를 체크할 때 (line 98), 개수를 업데이트할 때(line 108) 매번 localStorage에서 getItem을 하는 것 같습니다. 화면에 표시할 때 불러온 데이터를 나머지 상황에서도 사용할 거 같은데, 한 번 불러와 변수에 저장한 후 사용해봐도 좋을 거 같습니다!
// 사이드바 열기/닫기 기능 | ||
function openSidebar() { | ||
sidebar.style.left = "0"; | ||
} | ||
|
||
function closeSidebar() { | ||
sidebar.style.left = "-250px"; | ||
} | ||
|
||
function handleOutsideClick(event) { | ||
if (!sidebar.contains(event.target) && !hamburger.contains(event.target)) { | ||
closeSidebar(); | ||
} | ||
} |
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.
별도의 트랜지션 설정이 안 보이는데 애니메이션 적용이 어떻게 된 건가요?! 신기하네요. . .
배포 링크
🔗VanillaJS-Todo
미션을 진행하면서
평소 리액트를 주로 사용하다 보니 HTML, CSS, JavaScript만을 활용한 개발이 오랜만이었습니다. 직접 다시 작성해보니 기억이 가물가물했던 부분들을 떠올리고, 기본기를 점검할 수 있는 좋은 기회였습니다.
초기 구성을 고민하던 중, 카카오톡의 ‘나에게 보내는 메시지’ 기능을 활용해 할 일을 적어두는 제 모습을 떠올렸고, 이를 바탕으로 투두 리스트를 제작하기로 했습니다. 디자인적으로는 고유 색상값을 활용해 해당 기능과 비슷한 무드를 연출하고자 했습니다.
다만, 리액트를 사용할 수 없는 환경이다 보니 코드가 길어지고, 추가로 구현하고 싶은 기능이 많아도 간단하게 작성하기 어려운 점이 있었습니다. 이에 고민 끝에, 다크모드 버튼을 구현하여 기본적인 기능 외에도 사용자 경험을 향상시키는 요소를 추가해보았습니다 !!
이번 과제를 하면서 JS 파일을 어떻게 하면 가독성 좋고 깔끔하게 작성할 수 있을지, 그리고 유지보수를 편하게 할 수 있을지 고민했는데,, 아직은 많이 부족하다고 느꼈습니다,, 그리고 확실히 리액트를 사용하면 상태 관리나 컴포넌트 관리가 훨씬 직관적이라는 걸 다시 한 번 느꼈고, 리액트의 소중함을 깨달았습니다...😂
이번 과제를 하면서 코드를 더 깔끔하게 작성하는 방법을 배우고 싶어졌습니다.
Key Questions
1. DOM은 무엇인가요?
DOM (Document Object Model) : 트리 구조
DOM tree: 브라우저가 HTML 문서를 로드한 후 파싱하여 생성하는 모델
2. 이벤트 흐름 제어(버블링 & 캡처링)이 무엇인가요?
3. 클로저와 스코프가 무엇인가요?
a. 클로저: 함수가 선언될 당시의 스코프(lexical scope)를 기억하는 함수!
⇒ 내부함수(클로저)가 유효한 상태에서 외부함수가 종료되어도, 외부함수의 변수를 참조할 수 있다.
→ 데이터 은닉, 상태 유지 등의 용도에 사용된다
b. 스코프: 변수의 유효범위 ⇒ 참조대상 식별자를 찾아내기 위함
전역 스코프: 스크립트 전체에서 참조된다
지역 스코프 (local/function-level): 정의된 함수 내에서만 참조된다
렉시컬 스코프란 ??