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주차] 강나연 미션 제출합니다. #8

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
19 changes: 16 additions & 3 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,25 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vanilla Todo</title>
<title>💭 Todo</title>
<link rel="stylesheet" href="style.css" />
</head>

<body>
<div class="container"></div>
<div class="container">
Copy link
Member

Choose a reason for hiding this comment

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

semantic 태그를 잘 활용하시는 것 같아요! 앞으로 개발하실때도 계속해서 태그를 잘 활용하시면 좋을 것 같아요👍

Copy link
Member Author

Choose a reason for hiding this comment

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

항상 div 남발하고 다녔는데 이번에 공부하면서 그러면 안 된다는 걸 알게 됐습니다... 🥲

<h1>Things to do</h1>
<form id="input-form">
<input id="input-text" type="text" placeholder="📍 Enter your to-do" />
Copy link
Member

Choose a reason for hiding this comment

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

이모지들 너무 귀여워요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

감사합니다 ㅎㅅㅎ 열심히 골라 봤어요 🤓

<button id="input-button" onclick="addTodoList()">+</button>
Copy link
Member

Choose a reason for hiding this comment

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

이벤트를 인라인으로 다는건 유지보수가 어려워 안티패턴이라고 합니다~
참고자료도 남깁니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 읽어 보니 정말 유지보수가 힘들 것 같네요,,, 배워 갑니다...!!!

</form>
<section class="todo-section">
<h2>📂 to do <span id="todo-count"></span></h2>
<ul id="todo-list"></ul>
</section>
<section class="done-section">
<h2>🗑 done<span id="done-count"></span></h2>
<ul id="done-list"></ul>
</section>
</div>
</body>
<script src="script.js"></script>
</html>
91 changes: 91 additions & 0 deletions script.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
let itemList = [];

// 로컬 스토리지에서 값을 불러와 화면에 그려 준다
const renderTodoItem = () => {
const savedTodo = localStorage.getItem("itemList");

const todoList = document.getElementById("todo-list");
const doneList = document.getElementById("done-list");

// 덮어쓰지 않도록 초기화
todoList.innerHTML = "";
doneList.innerHTML = "";

// 데이터가 존재하는지 확인
if (savedTodo) {
itemList = JSON.parse(savedTodo);

itemList.forEach((savedItem) => {
Copy link
Member

Choose a reason for hiding this comment

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

고차함수를 적절하게 잘 활용하시는 것 같아요~!

const item = document.createElement("li");

const itemText = document.createElement("span");
itemText.className = "item-text";
itemText.addEventListener("click", toggleItem);
itemText.innerText = savedItem.text;

const deleteButton = document.createElement("button");
deleteButton.className = "delete-button";
deleteButton.addEventListener("click", removeItem);
deleteButton.innerText = "🧹";

item.appendChild(itemText);
item.appendChild(deleteButton);
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

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

appendChild()를 반복해서 사용하고 계신데, 한 번에 여러 Element를 추가할 수 있는 append()는 어떨까요? 두 함수의 차이점을 비교해보는 것도 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

append()를 썼어야 했네요... 감사합니다 감사합니다 🙇🏻‍♀️


if (!savedItem.isDone) {
todoList.appendChild(item);
// deleteButton.appendChild(todoList);
} else {
doneList.appendChild(item);
//deleteButton.appendChild(item);
}
});
}
countItem();
};

// 새로운 할 일을 입력 받을 때 로컬 스토리지에 추가한다
const addTodoList = () => {
event.preventDefault;
Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

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

addTodoList에서 event.preventDefault() 처리가 안되고 계속 새로고침이 일어나는 것 같아요! event를 인자로 받아주시고 호출 인자에 이벤트를 부여해주셔야 할 것 같아요!

Suggested change
const addTodoList = () => {
event.preventDefault;
const addTodoList = (event) => {
event.preventDefault();

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 이거 제출하고 나서야 깨달았네요... 수정하겠습니다!!

const inputObject = {
id: Date.now(),
text: document.getElementById("input-text").value,
isDone: false,
};
if (inputObject.text) {
Copy link
Member

Choose a reason for hiding this comment

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

비어있는 입력값을 예외처리 해주셨는데, 공백이 들어간 값은 예외처리가 안되더라구요. 정규식(\s)이나 다른 함수(trim())를 사용하셔서 공백 처리를 해볼 수 있을 것 같아요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

세상에 이것도 몰랐네요... 알려 주셔서 감사해요... 🥺

// 빈 입력은 받지 않음
itemList = [...itemList, inputObject];
Copy link
Member

Choose a reason for hiding this comment

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

스프레드 함수도 너무 잘 활용하시는 것 같아요!!👍

Copy link
Member Author

Choose a reason for hiding this comment

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

이번에 공부하면서 알게 됐어요!!! 이번 과제로 몰랐던 거 많이 많이 배워 갑니다... 🥰

localStorage.setItem("itemList", JSON.stringify(itemList)); // 로컬 스토리지에 저장
renderTodoItem();
Copy link
Member

Choose a reason for hiding this comment

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

값을 추가한 뒤에 계속해서 renderTodoItem()을 호출하시는데, renderTodoItem() 로직은 모든 데이터를 지우고 화면을 다시 그리는 것 같아요. 계속해서 데이터가 지워졌다가 그려지는 방식은 조금 비효율적일 수 있어서 DOM에 접근해서 Element를 추가하는 방식으로 접근해보면 어떨까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 비효율적이라는 생각을 했는데 해결책을 못 떠올리고 그냥 이렇게 짜버렸네요... 🥲 방향 제시해 주셔서 감사해요 🥺

}
};

// isDone의 상태를 반대로 바꿔 준다
const toggleItem = (e) => {
const itemObject = itemList.find(
(inputObject) => inputObject.text === e.target.innerText
);
itemObject.isDone = !itemObject.isDone;
localStorage.setItem("itemList", JSON.stringify(itemList)); // 로컬 스토리지 갱신
renderTodoItem();
};

// 로컬 스토리지에서 값을 삭제한다
const removeItem = (e) => {
const filteredList = itemList.filter(

Choose a reason for hiding this comment

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

사소한거긴 한데 여기서 filteredList 대신에 itemList라는 명칭을 그대로 쓰면 아래 count에서 filter 하지 않아도 되지 않을까 싶습니다 ..!!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 여기서 사용한 filter는 제가 삭제하고자 하는 todo 객체의 text를 찾아 삭제하기 위해 사용했고 밑의 filter는 각각 객체 속 isDone의 true, false 값을 카운트 하기 위해 사용한 건데 혹시 제가 잘못 이해했을까요??.............

(inputObject) =>
inputObject.text !== e.target.parentNode.innerText.slice(0, -2)
);
Comment on lines +74 to +77
Copy link
Member

Choose a reason for hiding this comment

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

원하는 아이템의 text가 일치하는 경우 삭제해주는 방식을 사용하셨는데, 이렇게 되면 같은 텍스트를 입력한 경우 하나의 아이템을 삭제해도 두 개가 동시에 삭제됩니다! id 등 고유한 값으로 삭제하는게 좋을 것 같아요~~

Copy link
Member Author

Choose a reason for hiding this comment

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

헉 그렇네요...!! 수정하겠습니다!

localStorage.setItem("itemList", JSON.stringify(filteredList)); // 로컬 스토리지 갱신
renderTodoItem();
};

// todo, done의 수 카운트
const countItem = () => {
const todoCount = document.getElementById("todo-count");
todoCount.innerText = ` (${itemList.filter((item) => !item.isDone).length})`;

Choose a reason for hiding this comment

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

여기서용!


const doneCount = document.getElementById("done-count");
doneCount.innerText = ` (${itemList.filter((item) => item.isDone).length})`;
};

window.onload = renderTodoItem();
84 changes: 84 additions & 0 deletions style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
@font-face {
font-family: "Pretendard-Regular";
src: url("https://cdn.jsdelivr.net/gh/Project-Noonnu/noonfonts_2107@1.1/Pretendard-Regular.woff")
format("woff");
font-weight: 800;
font-style: normal;
}

body {
height: 100vh;
background: linear-gradient(to left, rgb(221, 202, 209), rgb(210, 173, 190));
display: flex;
align-items: center;
justify-content: center;
font-family: "Pretendard-Regular";
}

.container {
background-color: rgba(255, 255, 255, 0.8);
width: 22rem;
height: 40rem;
border-radius: 20px;
box-shadow: 0px 0px 20px rgb(125, 123, 125);
display: flex;
flex-direction: column;
}

h1,
h2 {
margin-left: 5%;
margin-bottom: 0;
}

button {
border: 0;
background: none;
font-size: 20px;
cursor: pointer;
}
/*
li {
cursor: pointer;
}

button {
cursor: pointer;
} */

/* input-form */

#input-form {
display: flex;
align-items: center;
justify-content: center;
padding-top: 1rem;
padding-bottom: 1rem;
}

#input-text {
width: 80%;
height: 30px;
font-size: 18px;
border: 0;
border-radius: 15px;
outline: none;
background-color: rgba(213, 213, 213, 0.5);
}

/* todo-section, done-section */

.todo-section,
.done-section {
flex: 0.5;
border-top: 1px solid rgb(213, 213, 213);
}

#todo-list,
#done-list {
cursor: pointer;
list-style: none;
padding-left: 1rem;
margin: 0;
overflow: auto;
Comment on lines +71 to +83
Copy link
Member

Choose a reason for hiding this comment

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

공통된 style이 적용되는 id, class는 공통된 하나의 속성으로 묶어줘도 좋을 것 같아요! Element가 많아질수록 css 파일 내에서 id 선언이 늘어나고 유지보수가 어려워질 수 있으니까요!

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇네요... 수정하겠습니다!! 감사해요 🥺

}