Skip to content

[10기 김승훈] TodoList with CRUD #220

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.

Sign up for GitHub

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 5 commits into
base: main
Choose a base branch
from
Open

[10기 김승훈] TodoList with CRUD #220

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d725a7e
etc : commit 템플릿 설정
rlatmdgns Jul 7, 2021
7b64bde
feat : todoItem 입력하여 추가
rlatmdgns Jul 11, 2021
d2517bf
feat : item 삭제 기능 추가
rlatmdgns Jul 11, 2021
decc2f1
feat : count 추가
rlatmdgns Jul 12, 2021
2e89fca
feat : 체크박스를 클릭하여 complete 상태로 변경
rlatmdgns Jul 12, 2021
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
21 changes: 12 additions & 9 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class App {
this.todoInput.setEvent({
onAdd: text => {
Copy link

@na-reum na-reum Jul 12, 2021

Choose a reason for hiding this comment

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

setEvent에 들어가는 함수들을 따로 빼서 리팩토링하면 더 보기 깔끔할것같아요!

const todo = {
id: new Date(),
id: Date.now(),
text,
completed:false,
}
Expand All @@ -35,18 +35,21 @@ export default class App {
this.todoList.render(this.todoItems)
this.todoList.setEvent({
Copy link

@hyoungnam hyoungnam Jul 12, 2021

Choose a reason for hiding this comment

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

직접 접근해서 전달하는것보다 위의 new TodoList의 파라미터로 메소드들을 넘기는 것이 더 명확할꺼같습니다.

onDelete: (event) => {
Copy link
Author

@rlatmdgns rlatmdgns Jul 26, 2021

Choose a reason for hiding this comment

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

이벤트를 각 컴포넌트에서 처리할 수 있는 건 컴포넌트에서 처리

const item = event.target;
if (!item.classList.contains('destroy')) return;
const id = Number(item.closest('li').dataset.id);
const itemIdx = this.todoItems.findIndex((i) => i.id === id);
const target = event.target;
if (!target.classList.contains('destroy')) return;
const id = Number(target.closest('li').dataset.id);
const itemIdx = this.todoItems.findIndex((item) => item.id === id);
this.todoItems.splice(itemIdx, 1);
Copy link

@hyoungnam hyoungnam Jul 12, 2021

Choose a reason for hiding this comment

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

slice대신 splice를 사용하신 이유가 있나요?

Copy link
Author

@rlatmdgns rlatmdgns Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

배열 자체를 수정하려고 하다가 splice 를 사용하였는데... 생각을 해보니 filter , slice가 더 적합할 것 같습니다.

this.setState(this.todoItems);
},
onCompleted : (event) =>{
const item = event.target;
if (!item.classList.contains('toggle')) return;
const itemIdx = this.todoItems.findIndex((i) => i.id === id);
console.log(itemIdx)
const target = event.target;
if (!target.classList.contains('toggle')) return;
Copy link

@hyoungnam hyoungnam Jul 12, 2021

Choose a reason for hiding this comment

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

이벤트위임 방식 좋아요!

const id = Number(target.closest('li').dataset.id);
console.log(id)
const item = this.todoItems.find((todoItem) => todoItem.id === id);
item.completed = !item.completed;
Copy link

@HongJungKim-dev HongJungKim-dev Jul 12, 2021

Choose a reason for hiding this comment

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

completed를 true false로 관리하는것 직관적이고 좋은것 같습니다.

this.setState(this.todoItems);
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/TodoList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ export default class TodoList {
render(todoItems){
const todoItemTemplate = (todo) => {
return `
<li data-id="${todo.id}" class="${todo.complete ? 'completed' : ''}">
<li data-id="${todo.id}" class="${todo.completed ? 'completed' : ''}">
<div class="view">
<input class="toggle" type="checkbox" ${todo.complete ? 'checked' : ''} />
<input class="toggle" type="checkbox" ${todo.completed ? 'checked' : ''} />
<label class="label">${todo.text}</label>
<button class="destroy"></button>
</div>
Expand Down