-
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주차] 김서연 미션 제출합니다. #3
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.
과제하시느라 정말 고생많으셨습니다! 우선 눈에 띄는 디자인이 정말 인상깊고, 투두리스트의 기능동작이 완벽합니다. 코드에서 주석이 잘 달려있고, 함수 분리도 용도에 맞게 잘 분리되어있어 코드의 역할이 무엇인지 파악하기가 쉬웠던 것 같습니다 👍 자주 쓰이는 상수를 전역으로 관리하고, 특히 자바스크립트에서는 DOM이라는 게 무거우니 렌더링관련해서 어떻게 하면 더 최적화될 수 있을지 생각해보면 앞으로도 많은 도움이 되실 것 같습니다! 또한, js파일의 모듈화도 고려해보면 좋을 것 같아요! 반응형까지...여러 모로 많은 신경을 쓰셨다는 게 결과물과 코드에서 보이는 것 같아 더 인상깊네요 😬 고생많으셨습니다!
|
||
<body> | ||
<div class="todo-header"> | ||
<h1 class="titles">To-do List</h1> |
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.
class, id를 사용하는 목적이 눈에 보이면 좋을 것 같아요. 저는 우선 class를 활용하고, 중복된 class명에서 따로 관리가 필요할 때 id를 사용합니다. 추후 리액트, Next.js에서는 id를 자주 사용하지 않지만, class명이나 활용에 있어 목적을 생각하면 더 좋은 코드를 짤 수 있다고 생각합니다 👍
|
||
// 새로운 할 일 생성 함수 / 저장 데이터 불러오는 용 인자 | ||
function makeTodoList(text = "", isChecked = false) { | ||
const todoInput = document.querySelector("#todoInput"); |
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.
자주 쓰이는 DOM요소들은 함수 바깥에 전역 스코프로 한번만 가져오고, 참조하면 좋을 것 같아요!
// Enter 키 입력 감지 | ||
function keyCodeCheck(event) { | ||
if ( | ||
window.event.keyCode === 13 && |
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.
window.event가 아닌
function keyCodeCheck(event) { if (event.keyCode === 13 && todoInput.value.trim() !== "") { makeTodoList(); } }
이런식으로 이벤트 객체를 직접 참조하면 더 좋을 것 같아요. 이러면 window가 아닌 다른 브라우저의 환경에서 발생할 수 있는 문제가 사라집니다!
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.
처음에 window.event
가 전역 이벤트를 받진 않을까 싶어 찾아보다가 mdn에 간결하게 잘 나와 있어서 다른 분들께도 도움이 될까 링크 첨부합니다!
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.
말씀해 주신 대로 window.event
대신 이벤트 객체를 직접 참조하는 방식이 더 안전하겠네요! 공유해 주신 문서 참고해서 제안해 주신 방식으로 수정해 보겠습니다. 감사합니다!
// 새로운 할 일 생성 함수 / 저장 데이터 불러오는 용 인자 | ||
function makeTodoList(text = "", isChecked = false) { | ||
const todoInput = document.querySelector("#todoInput"); | ||
const inputValue = text || todoInput.value.trim(); |
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.
사용자 입력값을 js 내부에서가 아닌 함수의 인자로 받아서 텍스트를 처리할 수도 있을 것 같아요
} | ||
|
||
// 미완료/완료 할 일 분리 | ||
function updateTodoList(liElement, isChecked) { |
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.
완료목록, 미완료목록을 DOM 말고 배열로 관리해도 좋습니다. 지금도 문제는 없지만 배열로 관리했을 때 로컬스토리지 저장/로딩에 더 용이할 수 있을 것 같아요!
const savedTodos = JSON.parse(localStorage.getItem("todos")) || []; | ||
savedTodos.forEach((todo) => { | ||
const newLi = createTodoElement(todo.text, todo.isChecked); | ||
updateTodoList(newLi, todo.isChecked); | ||
}); | ||
} |
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.
지금 각 li를 만들 때마다 updateTodoList로 DOM을 재조작하는데 투두리스트가 많아지면 DOM 변경이 많아져서 성능적으로 불편함이 있을 것 같아요. 전역으로 let todos =[] 배열로 두고, 할 일 추가/수정으로 배열이 변경되었을 때 renderTodos() 이런 느낌으로 한번에 DOM을 리렌더링하는 설계도 고려해보면 좋을 것 같습니다.
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.
확실히 리스트가 많아지면 말씀해 주신 방법대로 배열로 관리하는 게 성능 면에서 더 효율적일 것 같네요! 한 번 어떻게 적용해 보면 좋을지 고민해 봐야 할 것 같아요 좋은 피드백 감사합니다😊
function loadTodayInfo() { | ||
const todayInfo = document.querySelector("#todayInfo"); | ||
|
||
var today = new Date(); |
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.
var은 의도치 않는 스코프 이슈가 있을 수 있으니 let, const활용을 추천드립니다!
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.
스코프에 대해 제대로 이해하지 못한 상태로 코드를 짜기 시작해서 별다른 기준 없이 변수를 선언했던 것 같아요.💦 이번 과제 공부하면서 조금 더 잘 이해하게 되었으니 말씀해 주신 대로 적용하고 다른 부분도 다시 한 번 되짚어 봐야 할 것 같아요. 감사합니다!
|
||
var days = ["SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"]; | ||
|
||
todayInfo.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.
todayInfo.textContent =
${year}년 ${month+1}월 ${date}일 (${days[day]});
요런 느낌으로 조금 더 가독성있게 할 수 있을 것 같아요. 템플릿 문자열은 정말 편한 것 같습니다
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.
기능에 따라 함수를 쪼개서 작성해 주신 덕분에 코드 이해가 수월했습니다! 변수명 관련해서 제 생각을 남겨 봤으니 참고해 보셔도 좋을 것 같습니다!
} | ||
|
||
// 새로운 할 일 생성 함수 / 저장 데이터 불러오는 용 인자 | ||
function makeTodoList(text = "", isChecked = false) { |
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.
새 할 일을 생성한다는 의미에서 makeNewTodo
또는 createNewTodo
같은 이름의 함수명은 어떨까요? 실제로 전체 todoList를 업데이트하고 있기는 하지만, 의미적으로 봤을 때 makeTodoList는 새로운 할 일 하나만 생성한다는 의미보다는 전체 todoList의 초기 렌더링을 담당하는 의미로 비쳐질 수도 있을 것 같아요
} | ||
|
||
// 오늘의 날짜 업데이트 | ||
function loadTodayInfo() { |
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.
처음에는 로컬스토리지에서 데이터를 불러오는 함수가 loadTodos
여서 처음에는 loadTodayInfo
도 로컬스토리지에서 어떤 데이터를 받아오는 함수인가 생각했습니다. 저는 함수나 변수의 쓰임에 따라 일관된 접두사/접미사를 붙이는 걸 좋아하는데요, 로컬스토리지에서 데이터를 불러오는 것과 일반적인 데이터를 가공해서 DOM에 추가하는 것은 조금 성질이 다르다고 생각했던 것 같아요.
이런 관점도 있구나! 고민해 보시면 좋을 것 같습니다.
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.
확실히 함수 쓰임에 따라 일관적으로 함수명을 작성하는 게 가독성 면에서 훨씬 좋겠네요!! 함수명, 변수명에 대해 조금 더 고민해 볼 필요가 있을 것 같아요. 좋은 피드백 감사합니다! 😊
// Enter 키 입력 감지 | ||
function keyCodeCheck(event) { | ||
if ( | ||
window.event.keyCode === 13 && |
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.
처음에 window.event
가 전역 이벤트를 받진 않을까 싶어 찾아보다가 mdn에 간결하게 잘 나와 있어서 다른 분들께도 도움이 될까 링크 첨부합니다!
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 선택자는 css 공식 문법(backgroud-color 등)과 마찬가지로 띄어쓰기를 하이픈으로 구분하는 케밥 케이스가 권장됩니다!
#grid { | ||
display: grid; | ||
grid-template-columns: 1fr 1fr; | ||
} | ||
@media (max-width: 800px) { | ||
#grid { | ||
display: block; | ||
} | ||
} |
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.
미완료된 할일과 완료된 할 일을 구분해서 모여주는 방식이 깔끔해서 좋은거 같아요!
과제 너무 수고하셨습니다! ui가 초록초록 봄이 오는 것 같아서 이뻐요ㅎㅎ |
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.
전반적으로 px로 css를 작성하셨는데, rem으로 작성하면 반응형디자인 같은 경우에 더 잘 어울린다고 하네요...!
🚀 배포 링크
To-do List
💡 느낀 점
처음 투두리스트를 혼자 개발할 때는 구글링하며 코드를 작성했지만, 완전히 이해하지 못한 부분이 많았습니다. 이번 과제를 통해 다시 한 번 만들어 보면서 이전에 이해되지 않던 부분들이 해결되어 좋았습니다. 특히 의미를 모르고 무작정 사용하던 코드들의 개념을 공부할 수 있어 의미 있는 시간이었습니다!!
클로저와 스코프 개념은 공부한 이후에도 어렵고 잘 와닿지 않아서 코드를 쓸 때 잘 고려하지 못하고 써지는 대로 작성하게 된 것 같은데, 앞으로 코드를 작성할 때 더 깊이 고민해봐야겠다고 생각했습니다. 또한, 깃을 다루는 게 아직 익숙하지 않아서 커밋 컨벤션을 준수하며 커밋하는 과정이 어려웠지만, 더 연습하면 점점 익숙해질 것 같습니다!
❓ Key Questions
1️⃣ DOM이란?
DOM (Document Object Model)
document.querySelector()
등을 사용하여 특정 요소를 선택하고 조작할 수 있습니다.DOM의 주요 특징
✅ 트리 구조 → HTML 문서를 계층적으로 표현
✅ 라이브 특성 → DOM을 변경하면 즉시 웹페이지에 반영됨
✅ 조작 가능 → JavaScript로 요소를 추가, 수정, 삭제 가능
2️⃣ 이벤트 흐름 제어 (버블링 & 캡처링)
📌 이벤트 버블링 (Event Bubbling)
<button>
클릭 시 부모<div>
까지 이벤트가 전달될 수 있습니다.🔹 event.target과 this의 차이점
event.target
→ 이벤트가 발생한 가장 안쪽 요소를 가리킴this
→ 현재 실행 중인 핸들러가 할당된 요소를 가리킴📌 이벤트 캡처링 (Event Capturing)
addEventListener(event, handler, **true**)
옵션을 설정하면 캡처링 단계에서 이벤트를 감지할 수 있습니다.3️⃣ 클로저(Closure)와 스코프(Scope)
📌 클로저(Closure)란?
📌 스코프(Scope)란?
🔹 스코프 체인(Scope Chain)과 렉시컬 스코프(Lexical Scope)
📌
var
,let
,const
스코프 차이점var
{}
을 무시하고 동일 변수 취급let
{}
내부에서만 유효const
{}
내부에서만 유효, 재할당 불가🔹 정리
✔
var
는 함수 내에서만 유효하며, 블록{}
안에서도 같은 변수로 취급됨✔
let
과const
는 블록{}
내부에서만 유효한 변수✔
const
는 값을 변경할 수 없지만, 객체나 배열의 내부 값은 변경 가능함