Skip to content
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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,46 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vanilla Todo</title>
<link rel="stylesheet" href="style.css" />
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.1.0/css/all.min.css" integrity="sha512-10/jx2EXwxxWqCLX/hHth/vu2KY3jCF70dCQB8TSgNjbCVAC/8vai53GfMDrO2Emgwccf2pJqxct9ehpzG+MTw==" crossorigin="anonymous" referrerpolicy="no-referrer" />


</head>

<body>
<div class="container"></div>
<main class="container">
<div class = "logo">to do list</div>
<!-- 날짜 표기 -->
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

html에도 주석을 남기는 거 좋은거 같아요!

<article class="date">
<h1 id = 'date'></h1>
<h3 id = 'week'></h3>
</article>

<!-- list 표시되는 구간 -->
<article class="show-lists">
<section class="todo-list-container">
<!-- list 입력 필드 -->
<div class="todo-list-input">
<input class="todo-input" placeholder="To do"/>
<button class="todo-input-button">+</button>
</div>
<div class="todo-list-wrapper">
<ul class="todo-lists"></ul>
</div>
<div class="show-todo-number">
</div>
</section>

<!-- done list 표시 -->
<section class="done-list-container">
<div class="done-title">Done</div>
<div class="done-list-wrapper">
<ul class="done-lists"></ul>
</div>
<div class="show-done-number">
</div>
</section>
</article>
</main>
<script src="./script.js"></script>
</body>
<script src="script.js"></script>
</html>
126 changes: 125 additions & 1 deletion script.js
Original file line number Diff line number Diff line change
@@ -1 +1,125 @@
//CEOS 19기 프론트엔드 파이팅🔥 ദ്ദി˶ˊᵕˋ˵)
const inputField = document.querySelector('.todo-input'); //todo 입력창
const TodoButton = document.querySelector('.todo-input-button'); //+추가 버튼
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 변수만 카멜 케이스가 아니네요!

const todoLists = document.querySelector('.todo-lists'); //todo list목록
const doneLists = document.querySelector('.done-lists'); //done list목록
const date = document.querySelector('h1#date'); //날짜 표기
const week = document.querySelector('h3#week'); //요일 표기
const showTodoNumber = document.querySelector('.show-todo-number'); //todo list div 개수
const showDoneNumber = document.querySelector('.show-done-number'); //done list div 개수


//lists 개수 세기
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 : )`
}
Comment on lines +12 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 태그를 세는 게 더 직관적인 코드가 될 거 같아요~!

todoListsNumber();
Comment on lines +12 to +19

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todoList의 개수를 세서 업데이트 하는 함수이니까 함수명을 updateListCounts 와 같은식으로 직관적으로 알아보기 편할 것 같아요!



//+ 버튼 클릭시 todo에 list를 추가
TodoButton.addEventListener('click', addTodo);
function addTodo(event){
event.preventDefault(); //event에 대한 기본 동작 실행 방지
Copy link

@songess songess Mar 16, 2024

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형식으로 제출되었을때 자동으로 실행되는 새로고침을 막기 위해서니깐요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 헷갈렸던 부분인데 깔끔하게 정리해 주셔서 감사합니다!

Copy link

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()가 필요하겠죠?


//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");
Comment on lines +27 to +44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 생성한 노드가 ul 태그 안으로 들어가게 되면 ul 태그의 직계 자손 태그가 li 태그가 아니라, div 태그가 되어버리는데요! 이게 좀 어색하게 느껴집니다. ul 태그 하위 요소로 바로 li 태그가 존재하는 게 적절한 거 같아요!

스크린샷 2024-03-17 오전 1 06 02

HTML5 Standard ul 태그



//공백 여부에 따른 처리
if(!inputField.value.trim())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공백여부 체크까지! 좋은거 같아요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공백여부 체크 너무 좋습니다~~ 사소하지만 공백여부를 함수 초기에 해주면 중간에 불필요한 요소를 만들어두지 않을 것 같아요!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공백 여부 체크를 함수 상단에 두고, 빈 값일 경우 early return 처리 해준다면, 불필요한 노드 생성들을 막는데에 도움이 될 거 같아요~ 그리고 if문이 참일 때 실행하는 실행문에 중괄호가 생략되어있는데 추가되는게 좋을 거 같습니다!

alert("할일을 추가해 보세요!");
else{
todoLi.innerText = inputField.value;
todoDiv.appendChild(todoLi);
todoDiv.appendChild(deleteButton);
todoDiv.appendChild(doneButton);
Comment on lines +52 to +54

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가 더 유연해서 참고해주시면 될 것 같습니다~~ 찾아본 블로그글 하나 두고 갑니다!

todoLists.appendChild(todoDiv);
todoListsNumber();
}

//input field 초기화
inputField.value = '';
}

//enter키 입력시 + 버튼 누른 효과
inputField.addEventListener("keyup", function (event) {
if (event.keyCode === 13) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keyCodedeprecated 되어서 key 속성을 사용하는 건 어떨까요 ??

Suggested change
if (event.keyCode === 13) {
if (event.key === "Enter") {

event.preventDefault();
TodoButton.click();
}
});
Comment on lines +64 to +69
Copy link

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;문을 추가해서 해결 할 수 있어요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keypress도 deprecated되어서 isComposing 체크해주면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 제출하고 발견한 오류라 당황 했었는데 해결방안까지! 정말 감사합니다! 최고👍



//todoLists의 버튼 클릭 함수
todoLists.addEventListener('click', deleteDoneTodo);
function deleteDoneTodo(e){
const item = e.target;

//delete 버튼 클릭시 삭제
if(item.classList[0] === "delete-button"){

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')을 사용하는 것을 권장합니다!

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();
}
}
Comment on lines +72 to +103

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이벤트 위임이 더 좋나 직접 각 버튼에 리스너 추가하는게 더 좋은가 생각해보니 동적으로 많이 생성되는 요소에는 위임 방식이 더 좋은 것 같아요! 각각 장단점이 있으니 블로그글 한번 읽으셔도 좋을 것 같습니다!
이외에도 삭제하는 로직은 동일하니 중복되는 코드를 합쳐도 될 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 이렇게 하면 제 경우 todoDiv에는 두가지 버튼이 있으니 부모 요소에 대해 이벤트 리스너를 부여하고 각 버튼을 따로 지정해서 기능을 주면 되니까 더 직관적인 코드를 짤 수 있을것 같아요! 혹은 말씀하신 대로 삭제하는 로직을 중복되지 않게 관리할 수도 있을거 같구요! 이벤트 리스너를 append한 부모 요소인 todoDiv에 주면 되는걸로 이해했는데 맞나요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞아요! 저도 각각 리스너 주는게 좋나 생각했는데 다희님처럼 작성하는 게 더 좋은 것 같습니다!! 잘 짜신 것 같아요




//날짜 표기 함수
const getDate = () => {
const newDate = new Date();
const year = newDate.getFullYear();
const month = newDate.getMonth() + 1; //js 자체 오류 보정
const day = newDate.getDate();

date.innerText = `${year}년 ${month}월 ${day}일`;
};
//요일 표기 함수
const getWeek = () => {
const daysOfWeek = ["일요일","월요일","화요일","수요일","목요일","금요일","토요일"];
const newDate = new Date();
const newWeek = daysOfWeek[newDate.getDay()];

week.innerText = `${newWeek}`;
};
getDate();
getWeek();
185 changes: 184 additions & 1 deletion style.css
Original file line number Diff line number Diff line change
@@ -1 +1,184 @@
/* 본인의 디자인 감각을 최대한 발휘해주세요! */
body{
width: 100%;
height: 100%;
display: flex;
justify-content: center;
}
Comment on lines +1 to +6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브라우저마다 기본으로 제공하는 스타일이 있어서 width:100%로 주었을 때 하단에 의도치 않는 스크롤 바가 있을 수 있어서 디폴트 스타일에 대한 블로그글 하나 남깁니다~~


.container{
display: flex;
flex-direction: column;
width: 70%;
aspect-ratio: 7 / 4 ;
margin-top: 10vh;
border: 0.05rem solid #CBC0C0;
border-radius: 25px;
box-shadow: 5px 4px 4px #CBC0C0;
padding: 2rem;
}

/* 상단 to do list */
.logo{
text-align: end;
color: #968E8E;
font-weight: 200;
font-size: 1.3rem;
}


/* 날짜 표기 */
.date{
border-bottom: 0.05rem solid #968E8E;
}
.date h1{
margin: 0;
font-size: 3rem;
font-weight: bold;
}
.date h3{
margin: 0.3rem;
font-size: 2rem;
font-weight: normal;
}


/* 하단 list 부분 */
.show-lists{
display: flex;
flex-direction: row;
width: 100%;
flex-grow: 1;
margin-top: 0.5rem;
}


/* todo list */
.todo-list-container{
width: 50%;
display: flex;
flex-direction: column;
align-items: center;
}

.todo-list-input{
width: 90%;
height: 2.5rem;
display: flex;
flex-direction: row;
border-bottom: 0.05rem solid #968E8E;
}

.todo-input{
border: none;
width: 90%;
color: black;
font-size: 1.3rem;
font-weight: normal;
}
::placeholder{
color: black;
font-size: 1.3rem;
font-weight: normal;
}

.todo-input-button{
border: none;
margin-left: auto;
width: 10%;
background-color: #fff;
color: #2F82FE;
font-size: 3rem;
font-weight: normal;
display: flex;
align-items: center;
justify-content: center;
}



/* done list 부분 */
.done-list-container{
width: 50%;
display: flex;
flex-direction: column;
align-items: center;
}
.done-title{
width: 90%;
height: 3.3rem;
display: flex;
align-items: center;
color: black;
font-size: 1.3rem;
font-weight: normal;
}



/* lists가 차지하는 부분 */
.todo-list-wrapper, .done-list-wrapper{
width: 90%;
height: max-content;
}

.todo-list-wrapper,.done-list-wrapper{
flex-grow: 1;
height: 100%;
}

.todo-lists, .done-lists{
display: flex;
flex-direction: column;
margin: 0;
padding: 0;
}


/* list 1개당 */
.todo-div{
background-color: #fff;
list-style-type: none;
width: 100%;
height: 3rem;
border: 0.05rem solid #CBC0C0;
border-radius: 10px;
box-shadow: 0px 4px 4px #CBC0C0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 그림자 디테일 너무 좋아요 떠있는 것 같습니다~~

margin: 0.3rem 0;
display: flex;
flex-direction: row;
}

.todo-li{
display: flex;
align-items: center;
flex-grow: 1;
padding: 0 1rem;
}
Comment on lines +151 to +156

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

설마 이런 사람은 없겠지만 .. 사진처럼 줄바꿈 안되는 경우를 대비해서
image

Suggested change
.todo-li{
display: flex;
align-items: center;
flex-grow: 1;
padding: 0 1rem;
}
.todo-li{
display: flex;
align-items: center;
flex-grow: 1;
padding: 0 1rem;
width:90%;
word-break:break-all;
}

처럼 추가해도 좋을 것 같습니다!
image


.done-button,
.delete-button{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

button등 사용자가 클릭할 수 있는 컴포넌트에 hovercursor: pointer;가 적용되어 있으면 더 좋을 거 같아요!

Copy link

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를 많이 적용해주는 거 같아요~!

border-radius: 10px;
border: 0.05rem solid #fff;
width: 10%;
background-color: #fff;
}

.fa-circle-check,
.fa-circle-minus{
pointer-events: none;
}


.done-div .delete-button{
display: block;
}
.done-div .done-button{
display: none;
}


/* list개수 표기 부분 */
.show-todo-number, .show-done-number{
font-size: 1rem;
color: #968E8E;
}