-
Notifications
You must be signed in to change notification settings - Fork 10
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주차] 김다희 미션 제출합니다. #9
base: master
Are you sure you want to change the base?
Conversation
</head> | ||
|
||
<body> | ||
<main class="container"> | ||
|
||
<!-- 날짜 표기 --> |
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.
html에도 주석을 남기는 거 좋은거 같아요!
TodoButton.addEventListener('click', addTodo); //+ 버튼 클릭시 todo에 list를 추가 | ||
|
||
function addTodo(event){ | ||
event.preventDefault(); //event에 대한 기본 동작 실행 방지 |
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.
button에 type=submit
속성부여가 되어있지 않기 때문에, preventDefault
함수를 사용하지 않아도 괜찮을 거 같아요! preventDefault
함수를 사용해 기본동작 실행을 방지하는 이유는 form형식으로 제출되었을때 자동으로 실행되는 새로고침을 막기 위해서니깐요!
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.
그래서 저라면 div가 아니라 form 태그로 만들었을 거 같아요~! 버튼의 기본 type이 submit이라 그러면 인풋에 커서를 두고 enter를 치거나, 버튼을 click했을 때의 이벤트를 따로 처리해주지 않아도 form에 이벤트 핸들러를 submit으로 바로 붙여버리면 되거든요. 이때는 event.preventDefault()가 필요하겠죠?
deleteButton.classList.add('delete-button'); | ||
|
||
//공백 여부에 따른 처리 | ||
if(!inputField.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.
공백여부 체크까지! 좋은거 같아요!
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.
공백 여부 체크를 함수 상단에 두고, 빈 값일 경우 early return 처리 해준다면, 불필요한 노드 생성들을 막는데에 도움이 될 거 같아요~ 그리고 if문이 참일 때 실행하는 실행문에 중괄호가 생략되어있는데 추가되는게 좋을 거 같습니다!
inputField.addEventListener("keyup", function (event) { | ||
if (event.keyCode === 13) { | ||
event.preventDefault(); | ||
TodoButton.click(); | ||
} | ||
}); |
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.
(크롬브라우저 기준입니다!)
input value 마지막에 한글이 입력된 상태에서 enter를 입력하면 이벤트리스너가 두 번 호출되고 있는데 composing
상태 때문인 거 같아요.
크롬브라우저에서 한글을 입력하면 밑에 언더바가 생기는 걸 볼 수 있는데 이걸 composing
상태라고 하고, keyup
으로 이벤트리스너를 등록하면 이런 문제가 생기는 것 같더라구요.
keypress
타입으로 바꾸거나, if (e.nativeEvent.isComposing) return;
문을 추가해서 해결 할 수 있어요!
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.
keypress
도 deprecated되어서 isComposing
체크해주면 좋을 것 같아요!
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.
저도 제출하고 발견한 오류라 당황 했었는데 해결방안까지! 정말 감사합니다! 최고👍
} | ||
|
||
.done-button, | ||
.delete-button{ |
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.
button등 사용자가 클릭할 수 있는 컴포넌트에 hover
시 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.
보통 button은 global css에서 공통으로 cursor:pointer를 많이 적용해주는 거 같아요~!
디자인이 깔끔해서 좋아요! js뿐만 아니라 html, css에도 주석이 달려있어 읽기 편했습니다. |
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.
안녕하세요! 프론트파트장 배성준입니다🚀
코드에 대부분 주석을 달아주셔서 리뷰하기 정말 편했습니다!
리뷰하면서 성능에 대한 고민도 많이 해볼 수 있어서 재미있었던 것 같아요
이번과제 고생하셨어요!! 다음 과제도 기대하겠습니다!
deleteButton.classList.add('delete-button'); | ||
|
||
//공백 여부에 따른 처리 | ||
if(!inputField.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.
공백여부 체크 너무 좋습니다~~ 사소하지만 공백여부를 함수 초기에 해주면 중간에 불필요한 요소를 만들어두지 않을 것 같아요!
|
||
//enter키 입력시 + 버튼 누른 효과 | ||
inputField.addEventListener("keyup", function (event) { | ||
if (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.
keyCode
는 deprecated 되어서 key
속성을 사용하는 건 어떨까요 ??
if (event.keyCode === 13) { | |
if (event.key === "Enter") { |
const item = e.target; | ||
|
||
//delete 버튼 클릭시 삭제 | ||
if(item.classList[0] === "delete-button"){ |
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가 1개밖에 없다면 괜찮지만 여러 개 있을 가능성을 방지하기 위해 item.contains('delete-button')
을 사용하는 것을 권장합니다!
function todoListsNumber(){ | ||
const todoNumber = todoLists.getElementsByTagName("div"); | ||
const doneNumber = doneLists.getElementsByTagName("div"); | ||
|
||
showTodoNumber.innerText = `${todoNumber.length} lists to do`; | ||
showDoneNumber.innerText = `${doneNumber.length} lists are done! Way to go : )` | ||
} | ||
todoListsNumber(); |
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.
todoList의 개수를 세서 업데이트 하는 함수이니까 함수명을 updateListCounts
와 같은식으로 직관적으로 알아보기 편할 것 같아요!
todoDiv.appendChild(todoLi); | ||
todoDiv.appendChild(deleteButton); | ||
todoDiv.appendChild(doneButton); |
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.
todoDiv.append(todoLi,deleteButton,doneButton);
과 같이 한 줄로도 쓸 수도 있습니다! 단일 요소 추가할 때 appendChild
보다 append
가 더 유연해서 참고해주시면 될 것 같습니다~~ 찾아본 블로그글 하나 두고 갑니다!
inputField.addEventListener("keyup", function (event) { | ||
if (event.keyCode === 13) { | ||
event.preventDefault(); | ||
TodoButton.click(); | ||
} | ||
}); |
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.
keypress
도 deprecated되어서 isComposing
체크해주면 좋을 것 같아요!
//todoLists의 버튼 클릭 함수 | ||
todoLists.addEventListener('click', deleteDoneTodo); | ||
function deleteDoneTodo(e){ | ||
const item = e.target; | ||
|
||
//delete 버튼 클릭시 삭제 | ||
if(item.classList[0] === "delete-button"){ | ||
const todoItem = item.parentElement; | ||
todoItem.remove(); | ||
todoListsNumber(); | ||
} | ||
|
||
//done 버튼 클릭시 donelists로 이동 | ||
if(item.classList[0] === "done-button"){ | ||
const doneItem = item.parentElement; | ||
doneLists.appendChild(doneItem); | ||
doneItem.classList.add('done-div'); //done lists는 따로 css 부여 | ||
todoListsNumber(); | ||
} | ||
} | ||
|
||
//doneLists의 삭제 버튼 클릭 함수 | ||
doneLists.addEventListener('click', deleteDone); | ||
function deleteDone(e){ | ||
const item = e.target; | ||
|
||
if(item.classList[0] === 'delete-button'){ | ||
const doneItem = item.parentElement; | ||
doneItem.remove(); | ||
todoListsNumber(); | ||
} | ||
} |
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.
앗 이렇게 하면 제 경우 todoDiv에는 두가지 버튼이 있으니 부모 요소에 대해 이벤트 리스너를 부여하고 각 버튼을 따로 지정해서 기능을 주면 되니까 더 직관적인 코드를 짤 수 있을것 같아요! 혹은 말씀하신 대로 삭제하는 로직을 중복되지 않게 관리할 수도 있을거 같구요! 이벤트 리스너를 append한 부모 요소인 todoDiv에 주면 되는걸로 이해했는데 맞나요?
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.
맞아요! 저도 각각 리스너 주는게 좋나 생각했는데 다희님처럼 작성하는 게 더 좋은 것 같습니다!! 잘 짜신 것 같아요
body{ | ||
width: 100%; | ||
height: 100%; | ||
display: flex; | ||
justify-content: center; | ||
} |
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.
브라우저마다 기본으로 제공하는 스타일이 있어서 width:100%
로 주었을 때 하단에 의도치 않는 스크롤 바가 있을 수 있어서 디폴트 스타일에 대한 블로그글 하나 남깁니다~~
height: 3rem; | ||
border: 0.05rem solid #CBC0C0; | ||
border-radius: 10px; | ||
box-shadow: 0px 4px 4px #CBC0C0; |
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.
이런 그림자 디테일 너무 좋아요 떠있는 것 같습니다~~
.todo-li{ | ||
display: flex; | ||
align-items: center; | ||
flex-grow: 1; | ||
padding: 0 1rem; | ||
} |
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.
다희님 수고 많으셨어요!
prettier 설정이 안되어있는데, 해주시면 더 가독성 있게 코드 작성할 수 있을 거 같습니다 ㅎㅎ
function todoListsNumber(){ | ||
const todoNumber = todoLists.getElementsByTagName("div"); | ||
const doneNumber = doneLists.getElementsByTagName("div"); | ||
|
||
showTodoNumber.innerText = `${todoNumber.length} lists to do`; | ||
showDoneNumber.innerText = `${doneNumber.length} lists are done! Way to go : )` | ||
} |
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 todoListsNumber(){ | |
const todoNumber = todoLists.getElementsByTagName("div"); | |
const doneNumber = doneLists.getElementsByTagName("div"); | |
showTodoNumber.innerText = `${todoNumber.length} lists to do`; | |
showDoneNumber.innerText = `${doneNumber.length} lists are done! Way to go : )` | |
} | |
function todoListsNumber(){ | |
const todoNumber = todoLists.querySelectorAll("li").length; | |
const doneNumber = doneLists.querySelectorAll("li").length; | |
showTodoNumber.innerText = `${todoNumber.length} lists to do`; | |
showDoneNumber.innerText = `${doneNumber.length} lists are done! Way to go : )` | |
} |
.todo-list라는 선택자로 가져온 값의 하위 div의 개수를 세기보다는, 하위의 li 태그를 세는 게 더 직관적인 코드가 될 거 같아요~!
//todo list div 생성 | ||
const todoDiv = document.createElement("div"); | ||
todoDiv.classList.add("todo-div"); | ||
|
||
//todo list li 생성 | ||
const todoLi = document.createElement("li"); | ||
todoLi.classList.add("todo-li"); | ||
|
||
|
||
//todo list 삭제 button 생성 | ||
const deleteButton = document.createElement('button'); | ||
deleteButton.innerHTML = '<i class="fa-solid fa-circle-minus" style="color: #ff5f58;"></i>' | ||
deleteButton.classList.add('delete-button'); | ||
|
||
//todo list done button 생성 | ||
const doneButton = document.createElement('button'); | ||
doneButton.innerHTML = '<i class="fa-solid fa-circle-check" style="color: #28c840;"></i>'; | ||
doneButton.classList.add("done-button"); |
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.
deleteButton.classList.add('delete-button'); | ||
|
||
//공백 여부에 따른 처리 | ||
if(!inputField.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.
공백 여부 체크를 함수 상단에 두고, 빈 값일 경우 early return 처리 해준다면, 불필요한 노드 생성들을 막는데에 도움이 될 거 같아요~ 그리고 if문이 참일 때 실행하는 실행문에 중괄호가 생략되어있는데 추가되는게 좋을 거 같습니다!
} | ||
|
||
.done-button, | ||
.delete-button{ |
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.
보통 button은 global css에서 공통으로 cursor:pointer를 많이 적용해주는 거 같아요~!
TodoButton.addEventListener('click', addTodo); //+ 버튼 클릭시 todo에 list를 추가 | ||
|
||
function addTodo(event){ | ||
event.preventDefault(); //event에 대한 기본 동작 실행 방지 |
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.
그래서 저라면 div가 아니라 form 태그로 만들었을 거 같아요~! 버튼의 기본 type이 submit이라 그러면 인풋에 커서를 두고 enter를 치거나, 버튼을 click했을 때의 이벤트를 따로 처리해주지 않아도 form에 이벤트 핸들러를 submit으로 바로 붙여버리면 되거든요. 이때는 event.preventDefault()가 필요하겠죠?
@@ -1 +1,125 @@ | |||
//CEOS 19기 프론트엔드 파이팅🔥 ദ്ദി˶ˊᵕˋ˵) | |||
const inputField = document.querySelector('.todo-input'); //todo 입력창 | |||
const TodoButton = document.querySelector('.todo-input-button'); //+추가 버튼 |
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.
이 변수만 카멜 케이스가 아니네요!
사실 제가 처음에 구상했던 디자인은 다음과 같은데요, figma를 활용해서 직접 디자인 하고 구현해 나가면서 구상한 대로만 나와주길 기대했었지만 생각보다 쉽지 않더라구요🥲 하지만 저는 원래 css와 디자인적인 요소를 많이 다루었다면 이번 기회를 통해 vanilla-JS로 기능 구현을 하면서 좀 더 기능적인 요소에 재미를 느꼈습니다. 또한 React에 익숙해져서인지 vanilla-JS 만으로 기능을 구현하는 것이 어색했지만, 리액트가 이런 문법을 기반으로 만들어졌구나 하는 기초를 다질 수 있는 기회가되었습니다. 로컬 스토리지와 삭제 버튼 애니메이션까지 완성 할 수 있었다면 얼마나 좋았을까 하는 아쉬움이 많이 남아 미션제출 이후에도 시간이 남는다면 좀더 완성해 보고 싶습니다. 간단한 todo 리스트이지만, 사용자 입장에서 디자인을 구상하다보니 overflow 처리도 깔끔하게 하면 어땠을까 하는 생각도 드네요. 여전히 깔끔한 코드 작성에는 익숙하지 않아 많은 분들의 코드도 참고해 보았고, css도 주먹구구식으로 작성하지 않으려 노력해 보았지만, 여전히 갈 길이 멀다는 것을 다시 한 번 느끼면서, 다음 주차 미션도 열심히 해 보겠습니다!
[배포 링크]
vanilla-todo-dahn12
[Key questions]
DOM은 무엇인가요?
DOM은 문서의 구조화된 표현(structured representation)을 제공하며 프로그래밍 언어가 DOM 구조에 접근할 수 있는 방법을 제공하여 그들이 문서 구조, 스타일, 내용 등을 변경할 수 있게 돕는다. DOM 은 nodes와 objects로 문서를 표현한다. 이들은 웹 페이지를 스크립트 또는 프로그래밍 언어들에서 사용될 수 있게 연결시켜주는 역할을 담당한다. 자바스크립트의 경우에는 각 태그를 노드로 하여 렌더트리를 만든다.
HTML (tag) Element를 JavaScript로 생성하는 방법은 어떤 것이 있고, 어떤 방법이 가장 적합할까요?
insertAdjacentHTML, createElement가 있다. insertAdjacentHTML는 가독성은 좋지만 보안 이슈가있고, createElement는 HTML element를 생성할때마다 dom tree 자료구조를 탐색해야하기 때문에 js의 성능을 저하시키지만, 이번 과제를 통해 구현할 때는 appendChild()를 통해 부모자식 관계를 인지하기가 더 쉬웠고, JS 가독성도 좋다고 느껴졌다.
Semantic tag에는 어떤 것이 있으며, 이를 사용하는 이유는 무엇일까요?
비교적 익숙하지 않은 태그를 설명해보면,
section : 문서의 부분을 의미하는 태그로, section 안에 section을 넣을 수 도 있고, article을 활용해 내용을 넣을 수도 있다.
nav : 웹사이트의 메뉴, 탭, 탐색경로 등 탐색 링크가 포함된 페이지 부분을 정의 한다.
article : 독립젹인 글을 다루는 데 사용하는 태그입니다. 독립적으로 배포하거나 재사용 할 수 있는 독립형 콘텐츠를 정의한다.
시멘틱 태그는 다음 3가지의 이점이 있다.
Flexbox Layout은 무엇이며, 어떻게 사용하나요?
flexbox는 행과 열 형태로 항목 무리를 배치하는 일차원 레이아웃 메서드이다. 가로 세로 축을 정할 수 있고, 유연하기 때문에 반응형 웹 제작에 용이하다. align-items는 교차축을 기준으로 요소를 배치하고, justify-content는 기본축을 기준으로 요소를 배치한다.
JavaScript가 다른 언어들에 비해 주목할 만한 점에는 어떤 것들이 있나요?
자바스크립트는 인터프리터 언어이기 때문에 컴파일이 필요한 다른 언어에 비해 시간이 적게 소요된다. 또한 서버로딩이 브라우저 자체에서 데이터 유효성 검사를 할 수있다는 이점이 있다. 그외에도 프론트 언어로서 풍부하 인터페이스가 있다는 점이 이점이다.
코드에서 주석을 다는 바람직한 방법은 무엇일까요?
제 3자가 코드를 읽었을때에도 막힘 없이 이해 할 수 있도록 달아야한다.따라서 정보를 제공하고, 의도를 설명하며 나아가서는 결과에 대한 경고까지 다른 사람이 내 코드를 실행했을 때 문제가 없도록 해야한다.