[3기 나무] TodoList with CRUD #36

wooooooood · 2020-09-06T13:44:43Z

🚀 기본 요구사항

todo list에 todoItem을 키보드로 입력하여 추가하기
todo list의 체크박스를 클릭하여 complete 상태로 변경. (li tag 에 completed class 추가, input 태그에 checked 속성 추가)
todo list의 x버튼을 이용해서 해당 엘리먼트를 삭제
todo list를 더블클릭했을 때 input 모드로 변경. (li tag 에 editing class 추가) 단 이때 수정을 완료하지 않은 상태에서 esc키를 누르면 수정되지 않은 채로 다시 view 모드로 복귀
todo list의 item갯수를 count한 갯수를 리스트의 하단에 보여주기
todo list의 상태값을 확인하여, 해야할 일과, 완료한 일을 클릭하면 해당 상태의 아이템만 보여주기

🚀🚀 심화 요구사항

localStorage에 데이터를 저장하여, TodoItem의 CRUD를 반영하기. 따라서 새로고침하여도 저장된 데이터를 확인할 수 있어야 함

🤔 고민

data를 컨트롤하는 방법이 맞는지? 상태관리법에 대해 더 알아보기
버퍼기간에 리팩토링하기

🐞 버그

todo item을 필터링할 때 원 배열의 인덱스도 같이 넘기기

심화 요구사항은 세션 전까지 구현해서 추가커밋 하려 합니다!
필터를 구현하고 나니 아이템 삭제/토글 하는 부분에서 버그가 생겼는데^_^ 다른 분들 코드 참고해가며 수정하도록 하겠습니다!

- 파일 끝 endline 추가 https://blog.coderifleman.com/2015/04/04/text-files-end-with-a-newline/ - 불필요 코드 삭제 - let -> const

- 지난 피드백 적용하면서 수정 덜된부분 수정 - TodoCount에서는 count만 받도록 수정

- handle conflict

…t-step1 into wooooooood

* todo list에 todoItem을 키보드로 입력하여 추가하기 및 완료 처리하기 * todo list의 x버튼을 이용해서 해당 엘리먼트를 삭제 * todo item 수정 기능 추가 * todo list의 item갯수를 count한 갯수를 리스트의 하단에 보여주기 * keyCode 메소드 대신 key 메소드 사용 * 리스트 개수 선택자 범위를 더 포괄적으로 처리 * data의 불변성을 지켜주기 위해 push가 아닌 concat 메소드 사용

* all except active and completed * 1st week missions acomplished * 리스트 위치 바꿈 * 1 * destroy has a problem * only without local storage * 로컬스토리지에서 불러냈을 때 체크 표시가 화면에 반영 안 됨 / 이것만 하면 1주차 심화까지 완성 Co-authored-by: eastjun <56821976+EastjunDev@users.noreply.github.com>

into wooooooood

…t-step1 into wooooooood

ganeodolu · 2020-09-06T16:34:05Z

src/app.js

+  this.removeItem = (index) => {
+    this.data.splice(index, 1);
+
+    this.filterItems(this.selected);
+  };


필터를 구현하고 나니 아이템 삭제/토글 하는 부분에서 버그에 대해 고민해보았습니다.
@wooooooood 님께서 적어놓았듯이 해야할 일에 필터가 선택되어 있을 때 보여지는 인덱스와 실제 인덱스가 다르기 때문에 버그가 생기는 것 같습니다.
다른 방법도 많이 있겠지만 저 같은 경우 this.data 객체에 할일의 고유번호인 id 항목을 추가하여 사용하였습니다.
TodoList의 렌더부분에 data-index대신 data-id="${id}"를 사용하여 map에서 나온 인덱스가 아니라 좀 전의 고유아이디를 적용하였는데요.
Todolist.js 29번째줄을 아래와 같이 수정하여 보여지는 순서에 상관없이 할일의 고유아이디를 얻을 수 있습니다.

if (target.classList.contains('destroy')) { const {id} = target.closest('.todo-item').dataset; removeItem(id); }

위에서 얻은 아이디에 해당하는 할일을 삭제하는 기능을 아래와 같이 구현해 보았습니다.

Suggested change

this.removeItem = (index) => {

this.data.splice(index, 1);

this.filterItems(this.selected);

};

this.removeItem = (id) => {

const nextData = this.data.filter((todo) => todo.id.toString() !== id);

this.data = nextData;

this.filterItems(this.selected);

};

임시로 삭제부분만 예시를 들었는데 토글도 비슷한 방식으로 적용하면 되지 않을까 생각합니다. 🐸

아하! index를 재활용(?)하고 싶다는 생각밖에 없었는데 말씀해주신 id 방식을 적용하면 해결될것같습니당! 예시까지 작성해주셔서 감사드립니다~👍✨

JunilHwang

안녕하세요 나무님! 같은 기수로 참여중인 황준일입니다.

제가 남긴 리뷰에 대해 종합해보자면 다음과 같습니다.

컴포넌트의 코드 스타일이 이도 저도 아닌 상태입니다. 그래서 명확한 방향으로 작성하면 좋을 것 같아요!
- ES5 Class: prototype 사용
- ES6 Class: Class 문법 사용
- Functional 사용
function 내부에서 this에다가 method를 사용하는건 좋지 않은 방법입니다.
- 이벤트와 관련된 메소드가 아니라면 prototype을 통하여 정의해주세요.
Map을 정확하게 사용해주세요. 지금은 forEach 처럼 사용하고 있습니다.
일부 코드에서 DOM을 기준으로 State를 변경하고 있습니다. State를 기준으로 DOM을 변경해주세요.
state/setState/render 등을 명확하게 분리하여 사용해주세요!
- 지금은 render update 및 각각의 변수명 등으로 흩어져있습니다. 이 상태에서 기능이 더 추가되면 본인이 작성한 코드도 무슨 내용인지 못알아볼 수 있어요!
- state 성격을 띄는 변수는 state 객체 안에 담아주세요.
- state를 변경할땐 꼭 setState를 사용해주세요.
- setState에서만 render를 실행해주세요.

추가로 나무님께서 읽어보시면 좋을 것 같은 글입니다. 같은 기수로 참여중인 @eyabc 님이 작성한 것인데 도움 되는 내용이 많이 있습니다!

추가로 class에 대한 이해가 깊어졌을 때 [es2015+] class문은 특별할까? 이 글도 읽어보시면 좋을 것 같습니다!

아 마지막으로 상태관리에 대한 내용인데 https://www.youtube.com/watch?v=o4meZ7MRd5o 이 동영상도 추천드려요!

제가 같은 팀원은 아니기 때문에 Aprrove나 Request Changes가 아닌 Comment로 남겨놓겠습니다!

JunilHwang · 2020-09-08T04:16:33Z

src/TodoCount.js

+export default function TodoCount($todoCount, totalCount) {
+  this.$todoCount = $todoCount;
+  this.totalCount = totalCount;
+
+  this.render = (totalCount) => {
+    this.totalCount = totalCount;
+    this.$todoCount.innerHTML = `총 <strong>${this.totalCount}</strong> 개`;
+  };
+
+  this.render(this.totalCount);
+}


음 지금 이 코드는 매우 애매한 기준으로 작성되었습니다.
무언가 컨셉(?)을 잡고 개선하면 좋을 것 같아요!

ES5의 class

export default function TodoCount($todoCount, totalCount) { this.$todoCount = $todoCount; this.totalCount = totalCount; this.render(this.totalCount); } // prototype을 사용하면 된답니다. TodoCount.prototype.render = function (totalCount) { this.totalCount = totalCount; this.$todoCount.innerHTML = `총 <strong>${this.totalCount}</strong> 개`; }

ES6의 class

export default class TodoCount { $totalCount; totalCount; constructor ($totalCount, totalCount) { this.$totalCount = $totalCount; this.render(totalCount); } render (totalCount) { this.totalCount = totalCount; this.$todoCount.innerHTML = `총 <strong>${totalCount}</strong> 개`; } }

함수형

export default function TotalCount ($totalCount, totalCount) { const render = totalCount => { $todoCount.innerHTML = `총 <strong>${totalCount}</strong> 개`; } render(totalCount); return { render }; } // 사용 const totalCount = TotalCount($totalCount, totalCount); totalCount.render(10);

안녕하세요! 상세한 리뷰 정말 감사드립니다 ㅠㅠ! 각각의 동작에 대해 더 명확히 구분을 지어야겠네요..! 첨부해주신 링크와 리뷰 차근차근 따라가겠습니다!~👍

JunilHwang · 2020-09-08T04:17:36Z

src/TodoFilter.js

@@ -0,0 +1,39 @@
+import {FilterDetails} from './constants.js';
+
+export default function TodoFilter($todoFilter, data, selected, filterItems) {


TodoFilter 또한 앞서 언급한 TodoCount와 마찬가지입니다.

JunilHwang · 2020-09-08T04:24:55Z

src/TodoFilter.js

+    let result = '';
+    FilterDetails.map(({type, text}) => {
+      result += `
+        <li>
+          <a class="${type} ${this.selected == type? 'selected': ''}" href="#${type}">${text}</a>
+        </li>
+        `;
+    });
+
+    this.$todoFilter.innerHTML = result;


map을 forEach 처럼 사용하고 있네요! map은 새로운 값을 반환해줘야합니다.

Suggested change

let result = '';

FilterDetails.map(({type, text}) => {

result += `

<li>

<a class="${type} ${this.selected == type? 'selected': ''}" href="#${type}">${text}</a>

</li>

`;

});

this.$todoFilter.innerHTML = result;

this.$todoFilter.innerHTML = FilterDetails.map(({type, text}) => `

<li>

<a class="${type} ${this.selected == type? 'selected': ''}" href="#${type}">${text}</a>

</li>

`).join('');

즉, 배열의 요소({ type, text })를 string으로 변환하여 새로운 배열을 만든 후에 이어주는거죠!

예를 들자면 다음과 같습니다.

const originArr = [1, 2, 3, 4, 5]; const arr1 = originArr.map(v => v * 10); const arr2 = originArr.map(v => `${v}${v}`).join('/'); console.log(arr1); // [10, 20, 30, 40, 50] console.log(arr2); // "11/22/33/44/55"

제가 맵을 이상하게 사용하고 있었네요.. 핃백감사합니다!!!

JunilHwang · 2020-09-08T04:25:14Z

src/TodoInput.js

@@ -0,0 +1,21 @@
+import {KEY} from './constants.js';
+
+export default function TodoInput($todoInput, addItem) {


TodoCount와 동일합니다.

JunilHwang · 2020-09-08T04:25:34Z

src/TodoInput.js

+
+export default function TodoInput($todoInput, addItem) {
+  if (!$todoInput) {
+    throw new Error('ERROR: Invalid object');


throw만 날릴게 아니라 throw를 catch하여 무언가 더 작업하는게 필요할 것 같아요!

JunilHwang · 2020-09-08T04:40:10Z

src/TodoList.js

+    let result = '';
+    this.data.map(({text, isCompleted}, index) => {
+      result += `
+      <li class="todo-item ${isCompleted? 'completed' : ''}" data-index="${index}">
+        <div class="view">
+          <input class="toggle" type="checkbox" ${isCompleted? 'checked' : ''} />
+          <label class="label">${text}</label>
+          <button class="destroy"></button>
+        </div>
+        <input class="edit" value="${text}" />
+      </li>
+      `;
+    }).join('');
+
+    this.$todoList.innerHTML = result;


Suggested change

let result = '';

this.data.map(({text, isCompleted}, index) => {

result += `

<li class="todo-item ${isCompleted? 'completed' : ''}" data-index="${index}">

<div class="view">

<input class="toggle" type="checkbox" ${isCompleted? 'checked' : ''} />

<label class="label">${text}</label>

<button class="destroy"></button>

</div>

<input class="edit" value="${text}" />

</li>

`;

}).join('');

this.$todoList.innerHTML = result;

this.$todoList.innerHTML = this.data.map(({text, isCompleted}, index) => `

<li class="todo-item ${isCompleted? 'completed' : ''}" data-index="${index}">

<div class="view">

<input class="toggle" type="checkbox" ${isCompleted? 'checked' : ''} />

<label class="label">${text}</label>

<button class="destroy"></button>

</div>

<input class="edit" value="${text}" />

</li>

`).join('');;

이 부분도 앞서 언급한 것 처럼 이렇게 사용할 수 있습니다!

JunilHwang · 2020-09-08T04:43:49Z

src/app.js

+  this.filterItems = (type) => {
+    this.selected = type;
+
+    if (type === FilterOptions.ALL.type) {
+      this.filteredData = this.data;
+    } else if (type ===  FilterOptions.ACTIVE.type) {
+      this.filteredData = this.data.filter((item) => !item.isCompleted);
+    } else if (type ===  FilterOptions.COMPLETED.type) {
+      this.filteredData = this.data.filter((item) => item.isCompleted);
+    }
+    this.todoList.updateItem(this.filteredData);
+    this.todoCount.render(this.filteredData.length);
+  };


Suggested change

this.filterItems = (type) => {

this.selected = type;

if (type === FilterOptions.ALL.type) {

this.filteredData = this.data;

} else if (type === FilterOptions.ACTIVE.type) {

this.filteredData = this.data.filter((item) => !item.isCompleted);

} else if (type === FilterOptions.COMPLETED.type) {

this.filteredData = this.data.filter((item) => item.isCompleted);

}

this.todoList.updateItem(this.filteredData);

this.todoCount.render(this.filteredData.length);

};

this.filterItems = (type) => {

this.selected = type;

this.todoList.updateItem(this.getFilteredItem());

this.todoCount.render(this.getFilteredItem().length);

};

this.getFilteredItem = () =>

this.data.filter(({ isCompleted }) =>

(this.type === FilterOptions.ALL.type) ||

(this.type === FilterOptions.COMPLETED.type && isCompleted) ||

(this.type === FilterOptions.ACTIVE.type && !isCompleted));

이런식으로 분리해주면 좋을 것 같습니다!

와 filter로 이렇게 구현할 수 있다는게 신기하네요,,! 👍

JunilHwang · 2020-09-08T04:44:58Z

src/app.js

+  const $todoCount = document.querySelector('.todo-count');
+  const $todoFilter = document.querySelector('.filters');
+  this.data = [];
+  this.filteredData = [];


filteredData를 상태로 저장하기보단
data와 selected조합하여 filteredData를 뽑아내는 메소드로 만들면 어떨까요?

크 넵 위에 작성해주신 부분 같은데 훨씬 유용한 방법이네요 👍👍!!!

JunilHwang · 2020-09-08T04:48:11Z

src/app.js

+  this.removeItem = (index) => {
+    this.data.splice(index, 1);
+
+    this.filterItems(this.selected);
+  };


Suggested change

this.removeItem = (index) => {

this.data.splice(index, 1);

this.filterItems(this.selected);

};

this.removeItem = filteredIndex => {

const item = this. filteredData[filteredIndex];

const index = this.data.find(v => v === this.get)

this.data.splice(index, 1);

this.filterItems(this.selected);

};

index가 다르더라도 실제 객체는 똑같은걸 공유하기 때문에 이런식으로 작성해주면 된답니다!

아하.. 코드 참고해서 수정하고 있는데 객체 내용이 똑같더라도 원하는 아이템을 잘 삭제해주는게 신기하네요👍

혹시 27번줄의 this.get은 js의 어떤 기능이 있는 코드인가요? 검색해도 나오는 것 같진 않아서 참고로 작성해주신 것이라 생각하고 const index = this.data.findIndex(v => v === item)으로 수정했습니다 ㅎㅎ;

~~isCompleted는 다르지만 내용은 동일한 다른 index값을 삭제할 수도 있지 않을까요??~~

@wooooooood 앗 this.get은 제가 잘못 작성한 것 같아요 ㅠㅠ item이 맞습니다!

this.removeItem = filteredIndex => { const item = this. filteredData[filteredIndex]; const index = this.data.findIndex(v => v == item) this.data.splice(index, 1); this.filterItems(this.selected); };

제가 졸면서 썼느지 .. 잘못된 부분이 몇 개 있네요 ㅠㅠ find말고 findIndex를 사용해야합니다!

남겨주신 리뷰를 잘못 이해했었네요. 정말 고유아이디가 필요없겠어요. 잘 배웠습니다. 👍

JunilHwang · 2020-09-08T04:49:15Z

src/constants.js

+export const FilterDetails = [
+  {type: 'all', text: '전체보기'},
+  {type: 'active', text: '해야할 일'},
+  {type: 'completed', text: '완료한 일'},
+];
+
+export const FilterOptions = {
+  ALL: FilterDetails[0],
+  ACTIVE: FilterDetails[1],
+  COMPLETED: FilterDetails[2],
+};


Suggested change

export const FilterDetails = [

{type: 'all', text: '전체보기'},

{type: 'active', text: '해야할 일'},

{type: 'completed', text: '완료한 일'},

];

export const FilterOptions = {

ALL: FilterDetails[0],

ACTIVE: FilterDetails[1],

COMPLETED: FilterDetails[2],

};

export const FilterOptions = {

ALL: 'all',

ACTIVE: 'active',

COMPLETED: 'completed',

};

export const FilterDetails = [

{type: FilterOptions.ALL, text: '전체보기'},

{type: FilterOptions.ACTIVE, text: '해야할 일'},

{type: FilterOptions.COMPLETED, text: '완료한 일'},

];

거꾸로 해주는게 좋을 것같아요!

헉 작성하면서도 배열 원소 넣는게 마음에 안들었는데 이런 방법이 있었군요!!😂

wooooooood and others added 29 commits June 17, 2020 21:38

[1주차] todo list에 아이템 추가

15d90e2

체크박스로 todo-item의 완료상태 토글

df843a1

x버튼을 누르면 해당 item을 삭제

1f959e5

Enable editing todo-item

81e5e02

todo-item의 갯수를 나타냄

5362e23

feedback

2847329

- 파일 끝 endline 추가 https://blog.coderifleman.com/2015/04/04/text-files-end-with-a-newline/ - 불필요 코드 삭제 - let -> const

feedback (2)

d4ac95a

- 지난 피드백 적용하면서 수정 덜된부분 수정 - TodoCount에서는 count만 받도록 수정

refactor(index.html): update markup

a937d8d

refactor(index.html): remove unused markup

ebb7951

refactor(index.html): update markup

ae4fa72

- handle conflict

refactor(index.html): remove unused markup

e45cfd1

Apply ESLint

adc0dbf

Apply module

55cb299

Merge branch 'wooooooood' of https://github.com/next-step/js-todo-lis…

c44ad08

…t-step1 into wooooooood

bugfix (1)

dfb29c2

bugfix (2) set checked

be9b641

refactor(base): update base sources

ea43302

Update README.md

a0aca9c

[ADD]InputNewTodo

3c0e702

Merge branch 'master' of https://github.com/next-step/js-todo-list-step1

a3885f1

into wooooooood

Merge branch 'wooooooood' of https://github.com/next-step/js-todo-lis…

0db58df

…t-step1 into wooooooood

Initialize 3기

284e24a

Do not use deprecated keycode

846ef5c

refactor

d742e15

Toggle options

c6a160f

add. filter items

d378561

refactor.

408238b

wooooooood added the 🚀 필수미션 label Sep 6, 2020

wooooooood added the 🐞 버그 label Sep 6, 2020

ganeodolu assigned wooooooood Sep 6, 2020

ganeodolu reviewed Sep 6, 2020

View reviewed changes

JunilHwang reviewed Sep 8, 2020

View reviewed changes

eastjun-dev added the TeamC label Sep 8, 2020

wooooooood added 2 commits September 13, 2020 01:00

feedback (1)

b461708

apply. local storage

7d7fb31

eastjun-dev merged commit 4c2b305 into next-step:wooooooood Oct 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3기 나무] TodoList with CRUD #36

[3기 나무] TodoList with CRUD #36

wooooooood commented Sep 6, 2020 •

edited

ganeodolu Sep 6, 2020

wooooooood Sep 8, 2020

JunilHwang left a comment •

edited

JunilHwang Sep 8, 2020 •

edited

wooooooood Sep 8, 2020 •

edited

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

wooooooood Sep 9, 2020

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

wooooooood Sep 9, 2020

JunilHwang Sep 8, 2020

wooooooood Sep 9, 2020 •

edited

JunilHwang Sep 8, 2020

wooooooood Sep 9, 2020

wooooooood Sep 9, 2020

ganeodolu Sep 9, 2020 •

edited

JunilHwang Sep 10, 2020

JunilHwang Sep 10, 2020 •

edited

ganeodolu Sep 10, 2020

JunilHwang Sep 8, 2020

wooooooood Sep 9, 2020

		@@ -0,0 +1,39 @@
		import {FilterDetails} from './constants.js';

		export default function TodoFilter($todoFilter, data, selected, filterItems) {

		@@ -0,0 +1,21 @@
		import {KEY} from './constants.js';

		export default function TodoInput($todoInput, addItem) {

[3기 나무] TodoList with CRUD #36

[3기 나무] TodoList with CRUD #36

Conversation

wooooooood commented Sep 6, 2020 • edited

🚀 기본 요구사항

🚀🚀 심화 요구사항

🤔 고민

🐞 버그

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JunilHwang left a comment • edited

Choose a reason for hiding this comment

JunilHwang Sep 8, 2020 • edited

Choose a reason for hiding this comment

wooooooood Sep 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooooooood Sep 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganeodolu Sep 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JunilHwang Sep 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooooooood commented Sep 6, 2020 •

edited

JunilHwang left a comment •

edited

JunilHwang Sep 8, 2020 •

edited

wooooooood Sep 8, 2020 •

edited

wooooooood Sep 9, 2020 •

edited

ganeodolu Sep 9, 2020 •

edited

JunilHwang Sep 10, 2020 •

edited