Conversation
S-J-Kim
left a comment
There was a problem hiding this comment.
안녕하세요 채림님
프론트엔드 멘토 김선종입니다.
우선, 코드를 아주 깔끔하게 작성해주셨네요. 그래서인지 제가 코멘트 드릴게 없었습니다. 다만 참고하면 좋을 몇가지를 코멘트로 남겼습니다.
과제 하느라 고생하셨습니다
| <body> | ||
| <div class="container"></div> | ||
| <div class="container"> | ||
| <header> |
| const clock = document.querySelector('h2.clock'); | ||
|
|
||
| // clock 기능 구현 | ||
| function getClock() { |
There was a problem hiding this comment.
갑자기 시계를 만들어봤던 것이 기억이 나서 추가해봤는데 감사합니다 😆
| } | ||
|
|
||
| //입력 받은 todo를 To Do 밑에 보여주기 | ||
| function paintToDo(newToDoObj) { |
There was a problem hiding this comment.
todo 목록을 데이터로 가지고 있기 때문에, 이 함수를 입력받은 todo object를 렌더링 할 수 있도록 (새로 입력하는 경우에서 더욱 확장) 리팩토링 해보면 좋을 것 같네요.
There was a problem hiding this comment.
넵!! 저도 그 부분에 있어서 고민이 있었는데 수정이 필요한 것 같습니다! 조언해주신 대로 리팩토링 해보겠습니다! 감사합니다!! 😃
corinthionia
left a comment
There was a problem hiding this comment.
안녕하세요 채림 님 🙌🏻
전체적으로 깔끔한 코드와 직관적인 네이밍으로 리뷰하기 정말 수월했습니다 😀 특히 초 단위까지 표시되는 시계를 구현하신 점, 스크롤바의 디자인까지 고려하신 점 등 세세한 부분까지 신경쓰신 모습이 인상적이었습니다! 크게 수정해야 하는 부분은 없었고 몇 가지 suggestion 정도만 남겨 드립니다. 또한 말씀하신 스크롤바 영역의 흰 박스 문제 관해서도 답변 남겨 드렸으니 확인해 주세요!
과제하시느라 고생 많으셨고 스터디 시간에 뵙겠습니다~
index.html
Outdated
| class="toDoInput" | ||
| type="text" | ||
| placeholder="할 일을 입력하세요" | ||
| /> | ||
| <button class="addToDoBtn">+</button> | ||
| </form> | ||
| </header> | ||
| <div class="list"> | ||
| <div class="toDo"> | ||
| <div> | ||
| <span>🔮 To Do</span> | ||
| <span class="toDoNum">(0)</span> | ||
| </div> | ||
| <ul class="toDoList"></ul> | ||
| </div> | ||
| <div class="done"> | ||
| <span>❤️🔥 Done</span> | ||
| <span class="doneNum">(0)</span> | ||
| <ul class="doneList"></ul> |
There was a problem hiding this comment.
HTML 태그의 id/class 이름은 kebab-case로 짓는 게 일반적입니다.
| class="toDoInput" | |
| type="text" | |
| placeholder="할 일을 입력하세요" | |
| /> | |
| <button class="addToDoBtn">+</button> | |
| </form> | |
| </header> | |
| <div class="list"> | |
| <div class="toDo"> | |
| <div> | |
| <span>🔮 To Do</span> | |
| <span class="toDoNum">(0)</span> | |
| </div> | |
| <ul class="toDoList"></ul> | |
| </div> | |
| <div class="done"> | |
| <span>❤️🔥 Done</span> | |
| <span class="doneNum">(0)</span> | |
| <ul class="doneList"></ul> | |
| class="todo-input" | |
| type="text" | |
| placeholder="할 일을 입력하세요" | |
| /> | |
| <button class="add-todo-Btn">+</button> | |
| </form> | |
| </header> | |
| <div class="list"> | |
| <div class="toDo"> | |
| <div> | |
| <span>🔮 To Do</span> | |
| <span class="todo-num">(0)</span> | |
| </div> | |
| <ul class="todo-list"></ul> | |
| </div> | |
| <div class="done"> | |
| <span>❤️🔥 Done</span> | |
| <span class="done-num">(0)</span> | |
| <ul class="done-list"></ul> |
| toDoCount(); | ||
| doneToDoCount(); |
There was a problem hiding this comment.
이 두 함수가 매번 같이 호출되는데, 개인적으로 하나로 합쳐도 될 것 같다는 생각이 듭니다!
There was a problem hiding this comment.
오ㅏ.. 저는 왜 그 생각을 못했을까요ㅠ ㅠ 완전 좋은 것 같아요 !! 수정해보겠습니다 🤩
| function toDoCount() { | ||
| const toDoLeft = toDos.filter((toDo) => toDo.isCompleted === false); | ||
| toDoNum.innerHTML = `(${toDoLeft.length})`; | ||
| } |
There was a problem hiding this comment.
es6 문법인 arrow function을 써 보셔도 좋을 것 같아요
| function toDoCount() { | |
| const toDoLeft = toDos.filter((toDo) => toDo.isCompleted === false); | |
| toDoNum.innerHTML = `(${toDoLeft.length})`; | |
| } | |
| const toDoCount = () => { | |
| const toDoLeft = toDos.filter((toDo) => toDo.isCompleted === false); | |
| toDoNum.innerHTML = `(${toDoLeft.length})`; | |
| } |
| id: Date.now(), | ||
| isCompleted: false, | ||
| }; | ||
| toDos.push(newToDoObj); |
There was a problem hiding this comment.
push() 메소드를 이용해서 배열의 값을 직접 바꾸는 것보다는 새로운 배열을 만들어 할당해 주는 방법을 더 추천드립니다.
이 글을 참고하시면 좋을 것 같습니다!
| toDos.push(newToDoObj); | |
| toDos = [...toDos, newToDoObj]; |
style.css
Outdated
| width: 350px; | ||
| height: 200px; | ||
| border-radius: 20px; | ||
| overflow: scroll; |
There was a problem hiding this comment.
말씀하셨던 스크롤바 영역에 생기는 흰 박스는 overflow: scroll로 설정하여 발생하는 문제인 듯합니다. overflow: scroll은 가로축, 세로축 스크롤이 모두 생기기 때문에 지금처럼 세로축 스크롤만 필요한 경우에는 overflow-y: scroll 또는 overflow: auto를 사용하시면 됩니다. 저는 개인적으로 overflow: auto를 선호한답니다!
| overflow: scroll; | |
| overflow: auto; |
There was a problem hiding this comment.
드디어 ...!! 해결방법을 찾았네요 감사합니다 !!!! 완전 거슬리는 부분이었는데 프짱님 덕분에 해결했어요 !! 🥰
|
안녕하세요 채림님! 이번에 코드 리뷰 파트너가 된 유세은 입니다. |
|
안녕하세요 채림님 코드 리뷰 파트너인 전시원입니다. |
SeieunYoo
left a comment
There was a problem hiding this comment.
채림님 바닐라 js로 코드 짜신 거 너무 감동입니다...........역시...
| ::-webkit-scrollbar { | ||
| width: 6px; | ||
| } | ||
|
|
||
| /* 스크롤바 막대 설정*/ | ||
| ::-webkit-scrollbar-thumb { | ||
| height: 17%; | ||
| background-color: rgb(182, 223, 224); | ||
| border-radius: 10px; | ||
| background-clip: padding-box; | ||
| } |
| const hours = String(date.getHours()).padStart(2, '0'); | ||
| const minutes = String(date.getMinutes()).padStart(2, '0'); | ||
| const seconds = String(date.getSeconds()).padStart(2, '0'); |
There was a problem hiding this comment.
padstart를 이용하면 글자수를 맞출 수 있군요 ! 좋은 정보 저도 알아갑니다~
js/todo.js
Outdated
| const toDoInput = document.querySelector('input.toDoInput'); | ||
| const toDoNum = document.getElementsByClassName('toDoNum'); | ||
| const doneNum = document.getElementsByClassName('doneNum'); | ||
| const submitForm = document.getElementsByClassName('submitForm'); |
There was a problem hiding this comment.
변수 이름이 모아져 있어서 훨신 보기 편한것 같습니다 !! 이름도 직관적이라서 딱 알아볼 수 있어서 좋았습니당
index.html
Outdated
| <script src="js/clock.js"></script> | ||
| <script src="js/todo.js"></script> | ||
| <script src="js/background.js"></script> |
There was a problem hiding this comment.
와우 파일 분리에 js 분리까지 ..! 저도 배워갑니다.. 기능별로 구분하니까 훨씬 보기 좋아요!
안녕하세요! CEOS 프론트 15기 김채림입니다. 분명 바닐라 자바스크립트로 to do list를 만들어본 경험이 있는데 처음하는 것처럼 새로웠던 과제였습니다. 다시 한 번 자바스크립트를 제대로 공부해봐야겠다는 생각이 든 시간이었어요. 여전히 좋은 코드는 어떻게 써야하는지 잘 모르겠고 어렵네요 ...ㅜ 만들면서도 이게 맞나????라는 질문을 계속 했던 것 같습니다.
💻 배포링크
https://vanilla-todo-15th-i384hlwil-chaaerim.vercel.app/
📗 추가한 내용
크게 추가를 한 부분은 없고 투두리스트다 보니 시간이 같이 표시되면 좋을 것 같아 간단히 위에 추가해보았습니다!
✏️ 어려웠던 부분
가장 어려움을 겪었던 부분은 todo와 done todo를 카운팅하는 부분이었습니다. todo 객체의
isCompleted가true,false일 때를 나누어 done todo와 todo의 개수를 세어보려고 했는데 잘 읽어들이지 못하는 것 같았습니다. 그래서 처음에 만들었던 코드를 지우고 count를 하는 전역변수를 만드려다가... 제가 done으로 투두를 이동시키는moveToDone메소드에서 isCompleted를 바꾸고 toDos배열에 반영을 안했다는 사실을 뒤늦게 깨달았습니다..기능단위로 커밋을 해보는 것이 처음이라 커밋도 어려운 부분 중 하나였던 것 같습니다.
😅 아쉬운 부분
시간 부족의 관계로 로컬스토리지를 활용하지 못했는데 이 부분이 아쉽습니다. 이 부분은 주말에 다시 추가해보려고 합니다. 그리고 스크롤바 밑에 하얀 박스가 생겨서 거슬리는데 해결을 못했습니다ㅜ 혹시 해결법을 아신다면 알려주시면 감사하겠습니다!!
DOM은 무엇인가요?
DOM이란 프로그래밍 인터페이스로 HTML에 접근해서 이를 조작할 수 있는 모델을 뜻합니다. DOM을 통해 자바스크립트를 가지고 HTML에 접근하여 삭제, 수정 등을 가능하게 해줍니다.
HTML (tag) Element를 JavaScript로 생성하는 방법은 어떤 것이 있고, 어떤 방법이 가장 적합할까요?
document.createElement()를 이용해서 HTML 요소를 추가할 수 있습니다.Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
Semantic tag란 의미가 있는 태그를 뜻합니다.
<div>,<span>과 같은 경우는 태그 이름만 봐서는 어떤 내용이 포함되어있는지 유추할 수가 없지만<form>,<article>과 같은 시맨틱 태그를 활용하면 내용 짐작이 가능해집니다.검색엔진 같은 경우도 태그를 기반으로 검색 키워드의 우선순위를 판단하므로 적절하게 시맨틱 태그를 활용하는 것이 좋습니다.
저도 div, span을 사용하기 전에 다시 한 번 고민하는 습관을 들여야겠습니다 .. ㅎ
Flexbox Layout은 무엇이며, 어떻게 사용하나요?
Flexbox Layout을 이용하면 box와 item을 행, 열로 자유자재로 배치 시켜줄 수 있습니다. flexbox 의 컨테이너 박스에 적용되는 속성값들이 존재하고, 각각의 item에 적용할 수 있는 속성값이 존재합니다. 또한 flexbox에는 중심축과 반대축 이 있는데(수평축, 수직축), 중심축을 수평, 수직에 두느냐에 따라 반대축이 바뀌도록 설정할 수도 있습니다.
저는 보통 수평으로 반복적인 것을 배치할 때
<li>로 만들고 flexbox를 이용해서 가로로 놓이게 하는 방법을 사용했던 것 같습니다.JavaScript가 다른 언어들에 비해 주목할 만한 점에는 어떤 것들이 있나요?
JavaScript는 타입을 명시할 필요가 없는 인터프리터 언어로 클라이언트에서 처리됩니다. 사용하기 편하기 때문에 웹문서를 동적으로 만들어야 할 때 많이 사용하는 것 같습니다. 변수형을 선언하지 않아도 된다는 점과 컴파일 과정이 없다는 것 또한 JavaScript의 장점이 될 수 있다고 생각합니다.
코드에서 주석을 다는 바람직한 방법은 무엇일까요?
다른 사람들도 빠르게 이해할 수 있도록 간결하고 정확하게 주석을 다는 것이 중요한 것 같습니다.
부족한 부분이 많을 텐데 알려주시면 감사하겠습니다!
리뷰 많이많이 달아주세요! 기다리겠습니다!! 😆