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

[라빈] TodoList 상태관리 하기 미션 제출합니다. #10

Open
wants to merge 10 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
8 changes: 8 additions & 0 deletions .idea/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions .idea/misc.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions .idea/modules.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions .idea/todolist.iml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions .idea/vcs.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 1 addition & 25 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,6 @@ <h1>TODOS</h1>
<div class="main">
<input class="toggle-all" type="checkbox">
<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">
<label class="label">완료된 타이틀</label>
<button class="destroy"></button>
</div>
<input class="edit" value="완료된 타이틀">
</li>
</ul>
</div>
<div class="count-container">
Expand All @@ -56,6 +32,6 @@ <h1>TODOS</h1>
</ul>
</div>
</section>
<script src="js/app.js"></script>
<script type="module" src="./js/module/TodoApp.js"></script>
</body>
</html>
126 changes: 126 additions & 0 deletions js/module/TodoApp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { STATUS, EVENT_TYPE, KEY_TYPE, TAG_TYPE } from "../util/constants.js";
import { TodoInput } from './TodoInput.js';
import { TodoItem } from './TodoItem.js';
import { TodoList } from './TodoList.js';
import { TodoCount } from './TodoCount.js';

function TodoApp() {
this.$todoList = document.querySelector("#todo-list");
this.$todoInput = document.querySelector("#new-todo-title");
this.$todoCount = document.querySelector("body > section > div.count-container > span > strong");

this.todoItems = [];
this.todoList = new TodoList(this.$todoList);

let todoItemIndex = this.todoItems.length;

new TodoCount(event => {
Copy link
Contributor

Choose a reason for hiding this comment

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

TodoCount에서 TodoApp에 종속적인 this.setState만 파라미터로 넘겨주고, 나머지 구체적인 구현부는 TodoCount.js안에서 구현해주면 TodoApp이 더욱 깔끔해질 것 같은데 라빈의 생각은 어떤가요? 🤔

event.preventDefault();
const $target = event.target;
const tag = $target.getAttribute("href");
Copy link
Contributor

Choose a reason for hiding this comment

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

href의 값을 비교하는 것보다는 해당 타겟의 명확한 class를 이용해 비교하는게 더 일반적이에요 😄

if (tag === TAG_TYPE.ALL) {
this.setState(this.todoItems);
return;
}
if (tag === TAG_TYPE.ACTIVE) {
const activeItems = this.todoItems.filter(item => item.status === STATUS.TODO);
this.setState(activeItems);
return;
}
if (tag === TAG_TYPE.COMPLETED) {
const completedItems = this.todoItems.filter(item => item.status === STATUS.COMPLETED);
this.setState(completedItems);
}
});

new TodoInput({
onAdd: contents => {
const newTodoItem = new TodoItem(todoItemIndex++, contents, STATUS.TODO);
this.todoItems.push(newTodoItem);
this.setState(this.todoItems);
}
}, this.$todoInput);

this.setState = updatedItems => {
const showItems = updatedItems;
this.todoList.setState(showItems);
this.$todoCount.innerText = showItems.length;
};

const completedTodoItem = event => {
event.preventDefault();
const $target = event.target;
const isCompletedButton = $target.classList.contains("toggle");
if (isCompletedButton) {
const todoItemId = Number.parseInt($target.closest("div").dataset.itemId);
Copy link
Contributor

Choose a reason for hiding this comment

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

서비스가 업데이트 되면서 div태그가 추가된다면, 원치 않는 에러를 발생시킬 수도 있을 것 같아요~! 명확한 의미를 담는 class를 이용하여 비교하는건 어떨까요?

const targetItem = this.todoItems.find(item => item.id === todoItemId);
targetItem.status = targetItem.status === STATUS.TODO ? STATUS.COMPLETED : STATUS.TODO;
this.todoList.setState(this.todoItems);
}
};

const removeTodoItem = event => {
event.preventDefault();
const $target = event.target;
const isDeleteButton = $target.classList.contains("destroy");
if (isDeleteButton) {
removeTargetTodoItem($target);
}
}

const removeTargetTodoItem = ($target) => {
const todoItemId = Number.parseInt($target.closest("div").dataset.itemId);
const targetItem = this.todoItems.find(item => item.id === todoItemId);
const isConfirmDelete = confirm(`${targetItem.content}를 삭제하시겠습니까?`);
if (isConfirmDelete) {
const targetItemIndex = this.todoItems.indexOf(targetItem);
this.todoItems.splice(targetItemIndex, 1)
this.setState(this.todoItems);
}
}

const setEditMode = event => {
event.preventDefault();
const $target = event.target;
const isDeleteButton = $target.classList.contains("destroy");
const isCompletedButton = $target.classList.contains("toggle");
if (!isDeleteButton && !isCompletedButton) {
const todoItemId = Number.parseInt($target.closest("div").dataset.itemId);
const targetItem = this.todoItems.find(item => item.id === todoItemId);
targetItem.status = STATUS.EDIT;
this.todoList.setState(this.todoItems);
document.querySelector("#todo-list > li.editing > input").addEventListener(EVENT_TYPE.KEY_DOWN, editMode);
}
}

const editMode = event => {
const $target = event.target;
const todoItemId = Number.parseInt($target.previousSibling.previousSibling.dataset.itemId);
const targetItem = this.todoItems.find(item => item.id === todoItemId);
if (event.target.value !== 0 && event.key === KEY_TYPE.ESC) {
event.preventDefault();
targetItem.status = STATUS.TODO;
this.todoList.setState(this.todoItems);
return;
}
if ($target.value.trim() !== 0 && event.key === KEY_TYPE.ENTER) {
const updateItem = new TodoItem(targetItem.id, $target.value, STATUS.TODO);
const targetItemIndex = this.todoItems.indexOf(targetItem);
this.todoItems.splice(targetItemIndex, 1, updateItem);
this.todoList.setState(this.todoItems);
}
}

const initEventListener = () => {
this.$todoList.addEventListener(EVENT_TYPE.CLICK, completedTodoItem);
this.$todoList.addEventListener(EVENT_TYPE.CLICK, removeTodoItem);
this.$todoList.addEventListener(EVENT_TYPE.DOUBLE_CLICK, setEditMode);
}

this.init = () => {
initEventListener();
}
}

const todoApp = new TodoApp();
todoApp.init();
16 changes: 16 additions & 0 deletions js/module/TodoCount.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {EVENT_TYPE} from "../util/constants.js";

export function TodoCount(showItems) {
this.number = document.querySelector("div.count-container > span > strong");
Copy link
Contributor

Choose a reason for hiding this comment

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

this.number는 어디에 쓰는 건가요? 🤔

this.allTodoItems = document.querySelector("div.count-container > ul > li:nth-child(1) > a");
Copy link
Contributor

Choose a reason for hiding this comment

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

element를 찾기 위한 query가 길어진다는 생각이 들어요 😢
html에 class를 해당 element를 쉽게 찾을 수 있게 class를 추가해서 찾으면 더 좋을 것 같아요!

this.todoItems = document.querySelector("div.count-container > ul > li:nth-child(2) > a");
this.doneItems = document.querySelector("div.count-container > ul > li:nth-child(3) > a");

this.initEventListener = () => {
this.allTodoItems.addEventListener(EVENT_TYPE.CLICK, showItems);
this.todoItems.addEventListener(EVENT_TYPE.CLICK, showItems);
this.doneItems.addEventListener(EVENT_TYPE.CLICK, showItems);
}

return this.initEventListener();
}
19 changes: 19 additions & 0 deletions js/module/TodoInput.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { EVENT_TYPE, KEY_TYPE } from "../util/constants.js";

export function TodoInput({ onAdd }, todoInput) {
todoInput.addEventListener(EVENT_TYPE.KEY_DOWN, event => this.addTodoItem(event));

this.isValid = (event, value) => {
return (event.key === KEY_TYPE.ENTER)
Copy link
Contributor

Choose a reason for hiding this comment

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

enter키를 체크하는 로직이 반복되고 있는데요. util성 메서드를 모아놓는 파일을 따로 만들어서 isEnterKey()와 같이 재사용하면 어떨까요~? 특정 키보드 이벤트를 받는지와 같은 부분은 유틸성으로 만들어 놓으면 재사용하기 쉽고 코드도 더 간결해질 것 같아요!

&& value.trim().length !== 0;
}

this.addTodoItem = event => {
const $newTodoTarget = event.target;
if (this.isValid(event, $newTodoTarget.value)) {
event.preventDefault();
onAdd($newTodoTarget.value);
$newTodoTarget.value = "";
}
};
}
7 changes: 7 additions & 0 deletions js/module/TodoItem.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function TodoItem(id, content, status) {
this.id = id;
this.content = content;
this.status = status;

return this;
}
13 changes: 13 additions & 0 deletions js/module/TodoList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { todoItemTemplate } from '../util/templates.js'

export function TodoList(todoList) {
this.setState = updatedTodoItems => {
this.todoItems = updatedTodoItems;
this.render(this.todoItems);
};

this.render = items => {
const template = items.map(todoItemTemplate);
todoList.innerHTML = template.join("");
};
}
23 changes: 23 additions & 0 deletions js/util/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export const STATUS = {
Copy link
Contributor

Choose a reason for hiding this comment

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

디렉토리 경로가 js/util인데요 util의 모음이 담기는 곳이니까 utils가 더 잘어울릴 것 같아요 :)

TODO: "todo",
COMPLETED: "completed",
EDIT: "editing"
};

export const EVENT_TYPE = {
CLICK: "click",
KEY_PRESS: "keypress",
DOUBLE_CLICK: "dblclick",
KEY_DOWN: "keydown"
};

export const KEY_TYPE = {
ESC: "Escape",
ENTER: "Enter"
};

export const TAG_TYPE = {
ALL: "#/",
ACTIVE: "#/active",
COMPLETED: "#/completed"
}
13 changes: 13 additions & 0 deletions js/util/templates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {STATUS} from "./constants.js";

export const todoItemTemplate = item => {
const isChecked = item.status === STATUS.COMPLETED ? "checked" : "";
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

return `<li class="${item.status}">
<div class="view" data-item-id="${item.id}">
<input class="toggle" type="checkbox" ${isChecked}>
<label class="label">${item.content}</label>
<button class="destroy"></button>
</div>
<input class="edit" value="${item.content}">
</li>`
};