-
Notifications
You must be signed in to change notification settings - Fork 7
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.
안녕하세요 지수 님!
코드도 정말 깔끔하고 군데군데 주석도 잘 작성해 주셔서 리뷰하기 정말 편했습니다 ㅎㅎ 코멘트 몇 개 남겨 두었으니 확인해 보시면 좋을 것 같습니다 🙂 고생하셨어요! 👍🏻
script.js
Outdated
let doneItems = []; |
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
키워드를 사용하는 게 좋습니다!
// 버튼 클릭 시 호출되는 이벤트 리스너를 설정 | ||
addButton.addEventListener("click", () => { | ||
const task = 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.
오 trim()
사용하는 디테일 아주 좋네요!!
function addTodoItem(task) { | ||
todoItems.push(task); | ||
renderTodoList(); // 할 일 목록을 렌더링 | ||
} | ||
|
||
// 지정된 인덱스의 항목을 배열에서 제거 | ||
function removeTodoItem(index) { | ||
todoItems.splice(index, 1); // 인덱스에 해당하는 항목을 제거 | ||
renderTodoList(); // 업데이트된 할 일 목록을 렌더링 | ||
} |
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.
다른 분 리뷰에도 달아 놓긴 했는데, push()
와 splice()
메서드보다는 spread 연산자 같이 원본 배열의 내용을 직접 변경하지 않는 방법을 사용하지 않는 것이 좋습니다!
JS의 immutability(불변성) 관련된 문제인데 이 글에 자세히 나와 있어서 참고하시면 좋을 것 같아요!
간단히 설명드리자면, 배열의 값을 직접 변경할 경우 디버깅이 어렵고 의도치 않은 결과를 불러올 수 있기 때문에 배열의 값을 직접 변경하는 방식이 아닌 새로운 배열을 만들어 할당하는 방식을 추천합니다. 따라서 값을 직접 변경하는 push()
, splice()
등의 메서드보다 새로운 배열을 반환하는 map()
, filter()
혹은 spread 연산자를 사용하는 방식을 권장합니다 👀 또한 나중에 React를 사용하실 때에도 배열 불변성을 지켜 주셔야 상태 변화 감지와 성능 최적화 부분에서 용이하답니다!
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 100제 문제풀면서 알게된 메소드를 적용해본다고 작성한 코드였는데! 알려주신 덕분에 배워갑니다!! 앞으로 어떤 메소드가 더 적절한지 고려하면서 코드를 작성하겠습니다!
// 항목을 완료 목록으로 이동 | ||
function completeTodoItem(index) { | ||
const completedTask = todoItems.splice(index, 1); // 완료된 항목을 todoItems에서 제거 |
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.
여기도 splice()
보다는 filter()
를 사용해 봅시다!
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.
render()
함수를 따로 분리하여 작성하신 점도 너무 좋네요
function renderTodoList() { | ||
todoList.innerHTML = ""; // 기존 목록 초기화 | ||
todoItems.forEach((item, index) => { |
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.
forEach()
사용 아주 좋습니다 👍🏻
|
||
// 완료된 항목 목록을 화면에 렌더링 | ||
function renderDoneList() { |
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.
renderDoneList()
함수도 결국 renderTodoList()
와 동일하게 투두 아이템을 화면에 그리는 역할을 하니, 한 단계 더 추상화하여 하나의 함수로 작성해 보는 것도 좋을 것 같아요 👍🏻
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 .delete-btn { | ||
cursor: pointer; |
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.
지수님의 주석 덕분에 각 코드별 기능을 바로바로 알 수 있었습니다🤩
그리고 직관적인 함수, 변수명으로 더 알아보기 쉬웠던 것 같습니다.
한 주동안 고생하셨습니다!
cursor: pointer; |
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 .delete-btn { | |
cursor: pointer; | |
li:hover { | |
cursor: pointer; |
이렇게 변경하면 모든 li 항목에 대해서 cursor: pointer
를 할 수 있을 것같아요!
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 completeBtn = document.createElement("span"); | ||
completeBtn.textContent = "✔️"; | ||
completeBtn.classList.add("complete-btn"); | ||
completeBtn.addEventListener("click", () => completeTodoItem(index)); |
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.
저도 화살표 함수 어려웠는데 이번 기회에 한번 작성해보쟈!했던 것 같아요! 감사합니다!!
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.
id명과 class명, 그리고 변수명이 명확하게 표현돼있어서 코드를 수월하게 볼 수 있었습니다! 주석도 꼼꼼히 써두셔서 편하게 볼 수 있었어요 많이 배워갑니다~!
if (task) { | ||
addTodoItem(task); | ||
todoInput.value = ""; // 입력 필드 초기화 | ||
} |
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.
오호 addTodo 함수를 실행시키는 조건으로 truthy를 사용하는 방법이 있군요🫢
이게 훨씬 깔끔한 것 같아요! 참고하겠습니다^~^
justify-content: center; | ||
align-items: center; | ||
height: 100vh; |
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 단위 하나 알고갑니다..! 고정값으로 컨테이너를 배열했는데 vh를 사용해봐야겠어요🤩
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.
저도 이번에 새로운것도 적용해보쟈해서 작성했는데! 같이 배워가니 너무 좋습니다!
안녕하세요, 유레카 1기 프론트엔드 이지수입니다.
첫 과제를 수행하면서 정말 부족함이 많다는것을 느끼는 한 주 였습니다. 특히 제가 알고있다고 생각한 개념들을 막상 코드로 작성하려고하니 막히는 부분이 많았던 것 같습니다. 깃도 정말 말썽이여서 이번기회에 깃허브 엄청 배웠던 것 같아요.🥲 🥹그럼에도 미션을 통해 평소 헷갈렸던 DOM에 대해 명확히 이해할 수 있어서 얻어가는게 많았던 시간이였습니다!!
미션 이후에도, JS,HTML,CSS를 누군가에게 정확히 설명 가능할 정도로 더 반복적으로 열심히 공부하겠습니다!
알게된 부분
1.appendChild와 append의 추가할 객체의수와 타입에 대한 차이점을 이해할 수 있었습니다.2.DOM에 사용되는 메소드를 코드에 적용하면서 구체적으로 적용할 수 있었습니다.
Key question
1.DOM이란 무엇인가요?2.JavaScript로 DOM을 조작하는 방법은 어떤 것이 있나요?
3.Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
4.Flexbox Layout은 무엇이며, 어떻게 사용하나요?
개선하고 싶은 부분
배포
vercel을 통해 배포하였습니다!https://vanilla-todo-git-jissssu-jissssus-projects.vercel.app/