Conversation
corinthionia
left a comment
There was a problem hiding this comment.
안녕하세요 윤일 님!
과제 하시느라 고생 많으셨습니다 🙂 코멘트에도 남기긴 했지만 완료/삭제 함수가 추가 함수 내부에 구현되어 있어 이를 따로 분리해 보시면 좋을 것 같아요! reset.css 까지 찾아서 적용하시고 노력이 돋보이는 과제였습니다 👍🏻 코멘트 몇 개 남겼으니 확인 부탁드려요 🙌
| v2.0 | 20110126 | ||
| License: none (public domain) | ||
| */ | ||
|
|
There was a problem hiding this comment.
우와 이런 것까지 적용해 보셨군요 👍🏻👍🏻
reset.css는 말 그대로 기본 스타일을 모두 초기화해 버려서 일일이 새로 지정하기 번거로울 수 있어요! 비슷하게 normalize.css라는 게 있는데, 이건 브라우저마다 상이한 부분만 스타일을 통일시켜 주는 역할을 합니다 🙂 이것도 알아보시면 좋을 것 같아요 🙌
There was a problem hiding this comment.
이런 게 있다는 걸 윤일님 덕분에 처음 알고 갑니다..🫢 기본적으로 적용된 마진 때문에 고민이 많았는데 이런 방법이 있었다니..!!
참고해서 다음 과제부터 적용해보겠습니다🥰🥰
There was a problem hiding this comment.
맞아요! 모두 초기화하는 reset.css보단 부분적으로 초기화해주는 normalize.css가 편리한 거 같아요! 다음 과제에 바로 적용해보려구요👍🏻👍🏻👍🏻
script.js
Outdated
| let todoInput = document.getElementById('todo-input'); // 할 일 입력창 | ||
| let todoList = document.getElementById('todo-list'); // 할 일 리스트창 | ||
| let addTodoBtn = document.getElementById('add-todo-btn'); // 버튼 | ||
| let doneTodoList = document.getElementById('done-todo-list'); // 완료된 할 일 리스트창 | ||
| let todoListTitle = document.getElementById('todo-list-title'); // 할 일 제목 | ||
| let doneTodoListTitle = document.getElementById('done-todo-list-title'); // 완료된 할 일 제목 |
There was a problem hiding this comment.
값이 재할당되는 경우가 아니라면 const 키워드를 사용하는 게 좋습니다!
script.js
Outdated
| if (!todoInput.value) { | ||
| alert('공백은 입력할 수 없습니다.'); |
script.js
Outdated
| $li.innerText = todoInput.value; | ||
| todoList.appendChild($li); | ||
| todoInput.value = ''; | ||
| todoInput.focus(); |
script.js
Outdated
| todoCount(); | ||
| } | ||
| // 완료 | ||
| function doneTodo(event) { |
There was a problem hiding this comment.
addTodo() 안에 삭제하는 로직과 doneTodo()이 함께 있는데 따로 분리해 보면 좋을 것 같아요!
또 doneTodo()에서 인자로 받는 event는 사용하지 않으니 지워도 될 것 같습니다!
script.js
Outdated
| doneLi.setAttribute('id', 'done-todo-item'); | ||
| doneBtn.setAttribute('id', 'done-delete-btn'); |
There was a problem hiding this comment.
li태그와 button태그에 style을 추가하려고 넣었는데 여러 개가 추가될 수 있단 점을 간과했네요 ! 수정하겠습니다 😀
script.js
Outdated
| function todoCount() { | ||
| let count = todoList.getElementsByTagName('li').length; | ||
| todoListTitle.innerText = `📋 TO DO (${count})`; | ||
| } |
There was a problem hiding this comment.
위의 사진처럼 초기 로딩 시 todoCount 함수가 호출되지 않아 투두의 개수가 카운팅되지 않고 있습니다! (초기에는 0으로 보여지는 게 더 일관성 있겠죠?) 이럴 때에는 DOMContentLoaded 이벤트를 사용할 수 있습니다 🙂 DOMContentLoaded 이벤트는 브라우저가 DOM 트리를 완성하는 즉시 발생하는 이벤트로, 아래 코드처럼 document 객체에 이벤트 리스너를 등록하여 사용할 수 있습니다.
document.addEventListener('DOMContentLoaded', () => {
todoCount();
});이걸 사용하면 localStorage에서 초기에 데이터를 불러오는 데에도 사용할 수 있지 않을까요~?
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| background: radial-gradient(#e66465, #9198e5); |
style.css
Outdated
| box-shadow: 0 0 25px rgba(0, 0, 0, 0.25); | ||
| } | ||
| .header { | ||
| /* justify-content: center; */ |
| #add-todo-btn { | ||
| height: 60%; | ||
| margin: 1%; | ||
| border: 0px; | ||
| background-color: transparent; | ||
| } |
There was a problem hiding this comment.
버튼에 cursor: pointer 옵션을 추가하여 hover 시 커서 모양을 바꾸는 것도 적용하면 좋을 것 같아요~
addTodo와 doneTodo의 기능 분리, 할 일 삭제, 할 일 개수 업데이트 기능 함수 세분화
jejukyj
left a comment
There was a problem hiding this comment.
변수명과 클래스명을 어떤 기능을 하는 요소인지 직관적으로 명시해두셔서 코드 보기 편했습니다😆
css reset 하는 법 등 유용한 지식도 얻고 가요😚
| v2.0 | 20110126 | ||
| License: none (public domain) | ||
| */ | ||
|
|
There was a problem hiding this comment.
이런 게 있다는 걸 윤일님 덕분에 처음 알고 갑니다..🫢 기본적으로 적용된 마진 때문에 고민이 많았는데 이런 방법이 있었다니..!!
참고해서 다음 과제부터 적용해보겠습니다🥰🥰
| if (!todoText) { | ||
| alert('공백은 입력할 수 없습니다.'); | ||
| } |
There was a problem hiding this comment.
윤일님도 truthy로 조건문을 구성하셨네요! '==='을 사용하는 것보다 훨씬 깔끔한 것 같아요
| $li.innerText = todoText; | ||
| todoList.appendChild($li); | ||
| todoInput.value = ''; // 입력 필드 초기화 | ||
| todoInput.focus(); // 입력 필드로 커서 이동 |
| <div class="container"></div> | ||
| <div class="container"> | ||
| <div class="header"> | ||
| <header>📚 투두리스트</header> |
| blockquote:before, | ||
| blockquote:after, | ||
| q:before, | ||
| q:after { | ||
| content: ''; | ||
| content: none; | ||
| } |
There was a problem hiding this comment.
blockquote와 q를 :before, :after 요소를 적용해서 이와 같이 스타일링한 이유가 궁금해요!
There was a problem hiding this comment.
reset.css는 제가 초기화한게 아닌 양식입니다! 제가 사용한 양식 사이트 올려드릴게요.
사용하면서 느낀 점은 모든 것을 다 초기화 해주기 때문에 <h1>태그 같은 경우에는 폰트나 굵기가 다 사라져서 태그를 사용하는 의미가 사라진다고 느꼈습니다. 그래서 위 코드에서는 h태그 관련 초기화를 삭제하고 사용했습니다. 이러한 점들 때문에 위에서 주현님께서 추천해주신 normalize.css를 사용해보려고 합니다! 여은님도 한번 사용해보시면 좋을 거 같아요👍🏻👍🏻👍🏻
There was a problem hiding this comment.
오! 이런게 있는지 몰랐어요! 기본 태그에서 필요없는 속성을 한번에 제거할 수 있어서 너무 편하네요😀 또 새롭게 알아갑니당
script.js
Outdated
| let $li = document.createElement('li'); | ||
| let $btn = document.createElement('button'); |
There was a problem hiding this comment.
li와 btn 변수 앞에 $기호를 붙이는 표기법이 궁금해요! DOM 요소인 점을 나타내는걸까요??👀
There was a problem hiding this comment.
달러($)기호는 document.getElementById()의 아이디 값이나, document.querySelector()의 태그 값 혹은 아이디나 클래스처럼 한 개만 선언할 때, 유일한 변수를 표시하기 위해 사용했어요! 하지만 41번째 줄에서는 doneLi를 선언하면서 달러 기호를 붙이지 않아서 일관성이 없는 부분 때문에 수정하려고 합니다!
There was a problem hiding this comment.
오홍 그렇군요! 저도 비슷한 상황에서 한번 적용해보겠습니다!
| if (!todoText) { | ||
| alert('공백은 입력할 수 없습니다.'); | ||
| } |
There was a problem hiding this comment.
공백 입력 방지를 알리는 부분 좋아요! 추가로 return 문을 넣는다면 공백 입력된 li 태그가 생성되는 것을 완전히 막을 수 있을 것 같습니다!
There was a problem hiding this comment.
return으로 공백 방지 조건 후 함수를 종료하면 다른 경우 없이 막을 수 있겠네요!! 정확한 피드백 감사합니다😊
|
|
||
| // 삭제 버튼 클릭 시 해당 li 삭제 | ||
| $btn.addEventListener('click', (e) => { | ||
| e.stopPropagation(); // 이벤트 전파 막기 -> li태그의 자식요소로 삭제버튼이 있기 때문에 버튼을 눌러도 완료항목으로 이동하는 문제발생 |
There was a problem hiding this comment.
오옹 e.preventDefault()에 대해서만 알고 있었는데, 부모 엘리먼트에 이벤트 전파를 막는 e.stopPropagation()는 처음 알았네요! 좋은 지식 얻고갑니다👍🏻👍🏻
There was a problem hiding this comment.
네 맞습니다!! 다른 방법으로는 HTML태그를 세분화해서 특정 태그에만 클릭이벤트를 적용하면 e.stopPropagation()을 사용 하지 않아도 된다는 사실까지 알아두시면 좋을 거 같아요🥰
ongheong
left a comment
There was a problem hiding this comment.
안녕하세요 윤일님! 이번 주차에 윤일님의 코드리뷰를 하게 되어 기쁩니다!
기능 구현을 최우선으로 한 코드라고 하셨지만 함수별로 기능을 잘 구분하여 나눠주셔서 보기 편했습니다! 투두리스트 페이지의 스타일도 깔끔하고 좋아요!
저도 이번 미션을 하면서 부족한 점을 많이 느꼈는데, 앞으로도 미션 해결하면서 같이 실력 키워나가봐요~~~!!
| // input으로 입력 받은 할 일을 리스트창에 추가 | ||
| function addTodo(event) { | ||
| event.preventDefault(); // form 태그의 기본 새로고침 기능 지우기 |
script.js
Outdated
| let $li = document.createElement('li'); | ||
| let $btn = document.createElement('button'); |
There was a problem hiding this comment.
해당 변수들이 재할당되는 일이 없기 때문에 이런 경우 가급적이면 const 키워드를 사용하는 게 좋습니다!
| } | ||
|
|
||
| $li.innerText = todoText; | ||
| todoList.appendChild($li); |
There was a problem hiding this comment.
append()와 appendChild()의 차이를 아시나요?!
둘 다 비슷한 기능을 해서 해당 코드에서는 크게 상관은 없지만, 이번 기회에 알아보는 것도 좋을 것 같습니다 🙂
참고자료 - append와 appendChild
| } | ||
|
|
||
| // 할 일 개수 업데이트 | ||
| function todoFlagCount(isTodo) { |
There was a problem hiding this comment.
(개인적인 의견입니다)
할 일 개수를 카운팅 하는 함수를 따로 빼고 중복을 줄인 점 아주 좋습니다!! 다만, 해당 함수를 사용할 때 todoFlagCount(true) 형식으로 사용하게 되는데, 이러면 '투두 플래그(?)를 카운팅 하겠다', '투두 플래그(?)를 카운팅 하지 않겠다'라고 생각할 것 같아요. 그러면 결국 todoFlagCount가 뭐지? 하면서 해당 함수를 찾아보게 되고, 다시 읽어 보면서 그 의미를 파악하는 데 시간이 걸릴 것 같습니다 🥵
말이 길어지긴 했는데, 결국은 함수명이 조금 모호한 것 같다는 생각입니다! 그래서 차라리 아래처럼 todoFlagCount 함수의 인자를 객체 형태로 받으면 key와 value를 명시하면서 어떠한 값이 true일 때를 나타내는지 알기 쉬울 것 같아요!
| function todoFlagCount(isTodo) { | |
| function todoFlagCount({ isTodo }) { ... } | |
| todoFlagCount({ isTodo: true }); | |
| todoFlagCount({ isTodo: false }); |
There was a problem hiding this comment.
그리고 이건 다른 말이긴 하지만,, 제가 투두리스트를 만들었을 때에는 할일/완료한일 목록을 각각 배열에 저장하여 관리했습니다! 그러면 할일 개수를 셀 때 배열의 length 값을 사용하면 되니까 편하더라구요!
There was a problem hiding this comment.
아 배열에 저장하면 개수를 셀 때나 가져올 때가 편하겠군요! react로 할 때 반영해보겠습니다!
함수의 기능을 나누면서 이게 맞는걸까.. 고민했는데 알려주신대로 함수의 인자를 객체 형태로 받으면 가독성이 더 좋아질 거 같습니다!
감사합니다
style.css
Outdated
| .header { | ||
| flex-direction: row; | ||
| } | ||
| header { | ||
| margin: 5% 5% 0; | ||
| font-size: 24px; | ||
| } |
There was a problem hiding this comment.
.header가 <header>를 감싸는 부분 같은데, 이럴 경우 wrapper를 붙여 .header-wrapper와 같은 이름을 많이 사용합니다!


안녕하세요! 유레카 프론트엔드 1기 김윤일입니다.
이번 미션은 저의 실력을 마주하게 된 미션이었습니다. 제가 뭘 모르는 지 알게 되었고 리액트가 얼마나 좋은지에 대해서도 조금이나마 느끼게 되었습니다. 시작은 막막했지만 하나하나 익혀가면서 만들고 직접 구현하고 나니 뿌듯하네요🥰
기능 구현을 최우선으로 한 코드라 정신 없고 가독성이 많이 떨어집니다. 더 나은 방법이나 조언은 코드리뷰로 해주시면 수정하겠습니다.
여러분의 피드백은 저를 더 성장하게 합니다! 감사합니다😃
알게 된 부분
개선하고 싶은 부분
KEY QUESTION
1) DOM이란 무엇인가요?
2) JavaScript로 DOM을 조작하는 방법은 어떤 것이 있나요?
querySelector,getElementById같은 메서드를 이용하여 요소에 접근하거나createElement를 사용하여 요소를 생성할 수 있습니다. 추가적으로innerHTML로 html태그를 변경하고, 태그 내용을 수정 가능합니다.3) Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
header,main,footer,aside,nav등4) Flexbox Layout은 무엇이며, 어떻게 사용하나요?
display: flex,flex-direction,justify-content,align-items등flex-grow,flex-shrink,align-self등배포
Vercel 사용하여 배포했습니다.https://vanilla-todo-kja1wl5px-kyoul10121s-projects.vercel.app