Skip to content

[10기 박은혜] TodoList with CRUD #221

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

Open
wants to merge 4 commits into
base: main
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
28 changes: 27 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,32 @@ <h1>TODOS</h1>
/>
<main>
<input class="toggle-all" type="checkbox" />
<ul id="todo-list" class="todo-list"></ul>
<ul id="todo-list" class="todo-list">
<!-- <li>
<div class="view">
<input class="toggle" type="checkbox"/>
<label class="label">새로운 타이틀</label>
<button class="destroy"></button>
</div>
<input class="edit" value="새로운 타이틀" />
</li>
<li class="editing">
<div class="view">
<input class="toggle" type="checkbox" />
<label class="label">완료된 타이틀</label>
<button class="destroy"></button>
</div>
<input class="edit" value="완료된 타이틀" />
</li>
<li class="completed">
<div class="view">
<input class="toggle" type="checkbox" checked/>
<label class="label">완료된 타이틀</label>
<button class="destroy"></button>
</div>
<input class="edit" value="완료된 타이틀" />
</li> -->
</ul>
<div class="count-container">
<span class="todo-count">총 <strong>0</strong> 개</span>
<ul class="filters">
Expand All @@ -34,5 +59,6 @@ <h1>TODOS</h1>
</div>
</main>
</div>
<script type="module" src="./src/js/index.js"></script>
</body>
</html>
67 changes: 67 additions & 0 deletions src/js/component/TodoApp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import TodoInput from '../component/TodoInput.js'
import TodoList from '../component/TodoList.js'
import TodoCount from '../component/TodoCount.js'

import { storage } from '../util/Storage.js'

function TodoApp () {
this.TODOS_KEY = "todos"
this.todos = (localStorage.getItem(this.TODOS_KEY)) ? storage.get(this.TODOS_KEY) : []

this.handleAddTodo = (todoItem) => {
const newTodos = this.todos.slice()
newTodos.push(todoItem)
this.setState(newTodos)
storage.set(this.TODOS_KEY, this.todos)
}

this.handleToggleTodo = (target) => {
const liItem = target.closest("li")
this.todos.map(item => {
if(item.id == target.id) {
if(item.status == "completed") {
item.status = "active"
liItem.classList.remove("completed")
} else if(item.status == "active") {
item.status = "completed"
liItem.classList.add("completed")
}

Choose a reason for hiding this comment

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

이부분은 if(item.id===target.id)
item.status ==="completed"? item.status="active' : item.status="completed" 이렇게 줄여볼 수 있을 것 같습니다. 그리고 현재는 리턴식에 대입문이 쓰여서 리턴하려는 값이 명확하지 않습니다. 리턴을 생략할 수 있습니다.

Copy link
Author

Choose a reason for hiding this comment

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

아,, 그렇네요!! 좋은 리뷰 감사합니다 👍

}
})
storage.set(this.TODOS_KEY, this.todos)
}

this.handleRemoveTodo = (target) => {
this.todos = this.todos.filter(item => {
if(item.id !== target.id) {
return item
}
Copy link

@nerdyinu nerdyinu Jul 12, 2021

Choose a reason for hiding this comment

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

filter()메서드는 매개변수 안에 조건식에 해당하는 배열을 리턴하기 때문에
filter(item=> item.id!==target.id) 이렇게 쓰시면 됩니다.
https://ko.javascript.info/array-methods 여길 참고해 보세요!

})
this.setState(this.todos)
storage.set(this.TODOS_KEY, this.todos)
}

this.render = () => {
this.todoInput = new TodoInput({
onAddTodo: this.handleAddTodo.bind(this)
})
this.todoCount = new TodoCount({
todos: this.todos
})
this.todoList = new TodoList({
todos: this.todos,
onToggleTodo: this.handleToggleTodo.bind(this),
onRemoveTodo: this.handleRemoveTodo.bind(this)
})
Copy link
Author

Choose a reason for hiding this comment

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

this bind를 왜 하는지?

}

this.setState = (nextState) => {
this.todos = nextState
this.todoList.setState(nextState)
this.todoCount.setState(nextState)
}

this.render()
}

export default TodoApp
26 changes: 26 additions & 0 deletions src/js/component/TodoCount.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import $ from '../util/QuerySelector.js'

function TodoCount({
todos
}) {
this.todos = todos
this.todoCountGroup = $('.todo-count')
this.todoCount = this.todoCountGroup.querySelector("strong")

this.handleCountTodo = () => {
this.todoCount.textContent = this.todos.length
}

this.render = () => {
this.handleCountTodo()
}

this.setState = (nextState) => {
this.todos = nextState
this.render()
}

this.render()
}

export default TodoCount
30 changes: 30 additions & 0 deletions src/js/component/TodoInput.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import $ from '../util/QuerySelector.js'

function TodoInput({
onAddTodo
}) {
this.onAddTodo = onAddTodo

this.newTodo = $('.new-todo')

this.handleAddTodo = () => {
this.newTodo.addEventListener("keyup", (e) => {
if(e.keyCode === 13) {
const todoItem = {
id: String(Date.now()),
todo : this.newTodo.value,
status: "active"
}
this.onAddTodo(todoItem)
}
})
}

this.render = () => {
this.handleAddTodo()
}

this.render()
}

export default TodoInput
105 changes: 105 additions & 0 deletions src/js/component/TodoList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import $ from '../util/QuerySelector.js'
import {
STATUS_ALL,
STATUS_ACTIVE,
STATUS_COMPLETED
} from '../util/Constant.js'

function TodoList ({
todos,
onToggleTodo,
onRemoveTodo
}) {
this.todoList = $('.todo-list')
this.filters = $('.filters')

this.onToggleTodo = onToggleTodo
this.onRemoveTodo = onRemoveTodo
this.todos = todos

this.todoTemplate = (item) => {
return `<li id=${item.id} ${item.status === 'completed' ? "class=completed" : ""}>
<div class="view">
<input class="toggle" type="checkbox" id=${item.id} ${item.status === 'completed' ? "class=completed" : "class=checked"} />
<label class="label">${item.todo}</label>
<button class="destroy" id="${item.id}"></button>
</div>
</li>`
}

this.handleMapAllTodo = () => {
this.todos.map(item => {
this.todoList.insertAdjacentHTML('beforeend', this.todoTemplate(item))
})
}

this.handleMapActiveTodo = () => {
const todos = this.todos.filter(item => item.status === 'active')
todos.map(item => {
this.todoList.insertAdjacentHTML('beforeend', this.todoTemplate(item))
})
}

this.handleMapCompletedTodo = () => {
const todos = this.todos.filter(item => item.status === 'completed')
todos.map(item => {
this.todoList.insertAdjacentHTML('beforeend', this.todoTemplate(item))
})
}

Choose a reason for hiding this comment

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

map은 배열을 반환하는 함수라 이렇게 insertAdjacentHTML처럼 쓰는 것은 정확하지 않습니다. 의도하신 부분대로라면 forEach가 맞겠네요.

this.mapTodos = (option = STATUS_ALL) => {
this.todoList.innerHTML = '';

switch(option) {
case STATUS_ALL :
this.handleMapAllTodo()
break;
case STATUS_ACTIVE :
this.handleMapActiveTodo()
break;
case STATUS_COMPLETED :
this.handleMapCompletedTodo()
break;
}

Choose a reason for hiding this comment

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

switch문에는 default값을 선언해주는 것이 좋습니다. 이렇게 switch문을 활용할 수도 있지만 template 자체에 조건문을 포함하여 리턴하면 훨씬 깔끔해진답니다!

` ${filteredItems.map(({ id, content, isComplete }) => `
    <li class="${isComplete ? "completed" :'false'} ${selectedItem===id?"editing":""}" data-id="${id}">
        <div class="view">
            <input class="toggle" type="checkbox" ${isComplete? "checked" : ''}/>
            <label class="label">${content}</label>
            <button class="destroy"></button>
        </div>
        <input class="edit" value="${content}"/>
    </li>`).join(``)}``

이런 방식을 참고해보세요!

}

this.handleBindEvents = () => {
this.filters.addEventListener("click", e => {
if(e.target.nodeName === 'A') {
e.target.closest('ul')
.querySelectorAll('a')
.forEach((target) => target.classList.remove('selected'))
e.target.classList.add('selected')
}
if(e.target.classList.contains("active")) {
this.mapTodos(STATUS_ACTIVE)
} else if(e.target.classList.contains("completed")) {
this.mapTodos(STATUS_COMPLETED)
} else if(e.target.classList.contains("all")) {
this.mapTodos(STATUS_ALL)
}
})

this.todoList.addEventListener("click", e => {
if(e.target.classList.contains("destroy")) {
this.onRemoveTodo(e.target)
} else if(e.target.classList.contains("toggle")) {
this.onToggleTodo(e.target)
}
Copy link
Author

Choose a reason for hiding this comment

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

contains 말고 좀 더 좋은 방법은 없을까요?

})
}

this.render = () => {
this.handleBindEvents()
this.mapTodos()
}

this.setState = (nextState) => {
this.todos = nextState
this.render()
}

this.render()
}

export default TodoList
3 changes: 3 additions & 0 deletions src/js/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import TodoApp from "./component/TodoApp.js";

const todo = new TodoApp()
3 changes: 3 additions & 0 deletions src/js/util/Constant.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const STATUS_ALL = "is-all"
export const STATUS_ACTIVE = "is-active"
export const STATUS_COMPLETED = "is-completed"
3 changes: 3 additions & 0 deletions src/js/util/QuerySelector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const $ = (selector) => document.querySelector(selector)

export default $

Choose a reason for hiding this comment

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

모듈 활용 좋아요 👍

12 changes: 12 additions & 0 deletions src/js/util/Storage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const storage = {
get: (TODOS_DATA) => {
try {
return JSON.parse(localStorage.getItem(TODOS_DATA))
} catch(error) {
console.log(error)
}
},
set: (key, value) => {
localStorage.setItem(key, JSON.stringify(value))
}
}
Copy link

@nerdyinu nerdyinu Jul 12, 2021

Choose a reason for hiding this comment

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

getter와 setter는

get getstorage(todos_data){
}
set setstorage(key,value){
}

이런 식으로 콜론 없이 써야 한답니다. https://ko.javascript.info/property-accessors 여길 참고해보세요!

Copy link
Author

Choose a reason for hiding this comment

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

그렇군요! 좋은 리뷰 감사합니다~~!