[10기 Tami] TodoList with CRUD #217

ink-0 · 2021-07-12T10:33:35Z

📄 회고

기타 사정으로 인해 요구사항을 거의 완료하지 못했습니다. 😢
이번주 내내 스피드를 내서 이번주 미션과 저번주 미션을 이행할 예정입니다

📌 진행 상황

html 모듈화
체크 박스 구현 중에 상태관리에 있어 헷갈리는 부분이 있습니다.

🎯 요구사항

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의 상태값을 확인하여, 해야할 일과, 완료한 일을 클릭하면 해당 상태의 아이템만 보여주기

-ink-0/js-todo-list-step1/#1

- ink-0/js-todo-list-step1/#1

ink-0/js-todo-list-step1/#1

[Week1] TodoList UI 모듈화

-ink-0/js-todo-list-step1/#3

Week1/3/check box

renee13wi

Tami님, 1주차 과제 너무 수고많으셨습니다 😄
1주차에 PR 올리셨단 것 만으로도 대단하십니다! 😄

남은 과제도 함께 열심히 해봅시다! 화이팅 !!

renee13wi · 2021-07-12T11:33:09Z

src/js/TodoApp.js

+      const prevIdx = this.state.todoes[this.state.todoes.length - 1].idx;
+
+      const newTodo = {
+        idx: prevIdx + 1,
+        content: contents,
+        state: '',
+        edit: '',
+      };
+      this.state.todoes.push(newTodo);
+
+      this.setState({
+        ...this.state,
+      });
+    },


이 부분은 따로 함수를 만들어 주는게 좋을 것 같아요! :)
ex) handleAdd

네이밍까지 감사합니다 ㅎㅎ

renee13wi · 2021-07-12T11:36:24Z

src/js/TodoApp.js

+    todoCount.setState(this.state.todoes);
+  };
+
+  new TodoInput({


const todoList = new TodoList({ $app, initialState: this.state.todoes, }); const todoCount = new TodoCount({ $app, initialState: this.state.count, });

TodoList, TodoCount와 다르게 인스턴스를 생성 할 때 const에 할당을 안해준 이유가 있을까요?!

일단은 할당한 후에 따로 setState할 필요가 없다고 생각해 그렇게 했는데요, 앞으로 추가 기능을 구현하면서 어떻게 될지는 모르겠네요 ..^^

renee13wi · 2021-07-12T11:37:22Z

src/js/TodoCount.js

+
+  this.render = () => {
+    const todoCountTemplate = `
+      <span class="todo-count">총 <strong>${this.state}</strong> 개</span><ul class="filters">


<ul class="filters">
가독성을 위해 개행해주는게 좋을 것 같아요! 😄

꼼꼼한 리뷰 감사합니다

jeleedev

안녕하세요 이지은입니다.
코드리뷰가 처음이라 다른사람 코드보는게 익숙하지가 않네요 😂
기능구현은 아직 다 안됐어도 컴포넌트가 잘 나눠져있어서 리뷰하면서 많이 배웠습니다

jeleedev · 2021-07-12T11:30:09Z

index.js

@@ -0,0 +1,2 @@
+import TodoApp from './src/js/TodoApp.js';
+new TodoApp(document.querySelector('.App'));


main 전체를 넘겨주는 건 생각 못해봤는데 HTML이 한결 더 깔끔해지네요! 하나 배워갑니다 👍

jeleedev · 2021-07-12T11:46:04Z

src/js/TodoItem.js

+    this.state = nextState;
+    this.render();
+  };
+  this.render = () => {


컴포넌트마다 render를 따로 둔 이유가 있으신가요? 그렇게는 아직 못해봐서 ㅎㅎㅎ 궁금하네요

사실 제가 다른 상태관리 블로그를 많이 참고해서 만드느라 비슷해 졌는데 eazisilver 님의 코드도 한번 보고 다른점을 찾아보면 좋을 것 같습니다 ㅎㅎ

nerdyinu

잘 읽었습니다! 전체적으로 이해하기 쉽게 잘 짜여져 있는 것 같은데 templated을 각 컴포넌트에서 따로따로 생성하는 방식보다는 TodoApp에서 각 컴포넌트의 target에 해당하는 엘리먼트를 template으로 render하고 각 컴포넌트 객체 생성과 함께 target값을 넘겨주는 방식은 어떨까요? 코드 추상화에 신경을 좀 더 쓰신 다면 더 좋은 코드가 나올 것 같아요!

nerdyinu · 2021-07-12T12:09:10Z

src/js/TodoCount.js

+  this.state = initialState;
+
+  this.$target = document.createElement('div');
+  this.$target.className = 'count-container';


이렇게 새로 createElement를 하는 방식보단 TodoApp의 template에서 하위 component들에 해당하는 target을 배분해주는 방식은 어떨까요?

nerdyinu · 2021-07-12T12:25:36Z

src/js/TodoApp.js

+  const todoCount = new TodoCount({
+    $app,
+    initialState: this.state.count,
+  });


TodoList나 TodoCount에 사용하실 함수들도 OnAdd처럼 객체에 화살표 함수 형식으로 넘겨주거나, 아니면 bind(this)를 해서 넘겨주면 객체에서 state 충돌없이 사용가능해요!

-ink-0/js-todo-list-step1/#3

[week1] checkBox 구현

- ink-0/js-todo-list-step1/#6

[week1] TodoList Delete 구현

- ink-0/js-todo-list-step1/#8

Week1/8/todo edit

- 상태 추가 filterTodoes, isFilter, todoesCount - component 네이밍 변경 TodoCount -> TodoFilter - ink-0/js-todo-list-step1/#10

- const 생성: 메세지, class data 분리 - 객체 리터럴 적용 - ink-0/js-todo-list-step1/#10

Week1/10/todo filter

- 그외 const 대치 안한 것 수정 - ink-0/js-todo-list-step1/#12

-ink-0/js-todo-list-step1/#12

Week1/12/local storage

-ink-0/js-todo-list-step1/#14

[week2] GET userList

] -ink-0/js-todo-list-step1/#17

Week2/17/user list UI

ink-0

1차 TodoList 본인 리뷰

ink-0 · 2021-07-26T11:29:56Z

src/js/TodoApp.js

+  const initialData = {
+    todoes: [
+      {
+        idx: 0,
+        content: 'hiEvery One',
+        state: '',
+      },
+      {
+        idx: 1,
+        content: 'Im Tami',
+        state: '',
+      },
+    ],
+    todoesFiltered: [],
+    filterState: FILTER_TYPES.ALL,
+    todoesCount: '0',
+    users: [],
+  };


초기 데이터를 따로 보관하는 것이 좋았을 것이다.

ink-0 · 2021-07-26T11:31:30Z

src/js/TodoApp.js

+    this.state['todoes'] = resetTodoes;
+    localStorage.clear();
+    localStorage.setItem('state', JSON.stringify({ ...this.state }));
+    this.state = JSON.parse(localStorage.getItem('state'));
+    this.setState({
+      ...this.state,
+    });
+  };


생성 삭제 시 clear 후 set하는 방식밖에 없었는지 고민할 필요가 있다.

ink-0 · 2021-07-26T11:32:16Z

src/js/TodoApp.js

+  const filterTodo = (filterType) => {
+    const { todoes } = this.state;
+    if (filterType === FILTER_TYPES.ALL) {
+      this.setState({
+        ...this.state,
+        filterState: FILTER_TYPES.ALL,
+        todoesCount: todoes.length,
+      });
+    } else if (filterType === FILTER_TYPES.COMPLETE) {
+      const completedTodoes = todoes.filter(
+        (todo) => todo.state === FILTER_TYPES.COMPLETE
+      );
+
+      this.setState({
+        ...this.state,
+        todoesFiltered: completedTodoes,
+        filterState: FILTER_TYPES.COMPLETE,
+        todoesCount: completedTodoes.length,
+      });
+    } else if (filterType === FILTER_TYPES.ACTIVE) {
+      const activeTodoes = todoes.filter(
+        (todo) => todo.state !== FILTER_TYPES.COMPLETE
+      );
+      this.setState({
+        ...this.state,
+        todoesFiltered: activeTodoes,
+        filterState: FILTER_TYPES.ACTIVE,
+        todoesCount: activeTodoes.length,
+      });


코드 중복이 많다.
setState를 하는 부분에서 중복을 해결할 방안이 필요하다

ink-0 · 2021-07-26T11:32:56Z

src/js/components/TodoList.js

+  $nodeTodoList.addEventListener('click', (e) => {
+    const $node = e.target;
+
+    if ($node.className === TODO_ITEM_CLASS.TOGGLE) {
+      this.toggleTodoItem($node);
+    }
+    if ($node.className === TODO_ITEM_CLASS.DESTROY) {
+      this.deleteTodoItem($node);
+    }
+
+    if ($node.className === TODO_ITEM_CLASS.EDIT) {
+      this.editTodoItem($node);
+    }


이벤트 early return 이 필요하다

ink-0 added 9 commits July 12, 2021 01:23

[week1] TodoUI UI component 분리

e0b9819

-ink-0/js-todo-list-step1/#1

[week1] favicon 변경

0060f4b

-ink-0/js-todo-list-step1/#1

[Week1] TodoInput 기능 구현현

a484359

- ink-0/js-todo-list-step1/#1

TodoCount UI 분리

8836ea4

ink-0/js-todo-list-step1/#1

TodoList UI 수정

c4d0353

ink-0/js-todo-list-step1/#1

Merge pull request #2 from ink-0/week1/1/TodoUI

0386adf

[Week1] TodoList UI 모듈화

[week1] todoes data 변경 (idx,content 추가)

e7ffe4a

-ink-0/js-todo-list-step1/#3

[week1] checkBox 기능생성중

3544be9

-ink-0/js-todo-list-step1/#3

Merge pull request #4 from ink-0/week1/3/checkBox

dc0b8aa

Week1/3/check box

renee13wi reviewed Jul 12, 2021

View reviewed changes

jeleedev reviewed Jul 12, 2021

View reviewed changes

nerdyinu reviewed Jul 12, 2021

View reviewed changes

ink-0 added 17 commits July 13, 2021 03:48

[week1] checkBox 구현

510f9ca

-ink-0/js-todo-list-step1/#3

Merge pull request #5 from ink-0/week1/3/checkBox

d8eb0b6

[week1] checkBox 구현

[week1] TodoList Delete 구현

0b0d2e7

- ink-0/js-todo-list-step1/#6

Merge pull request #7 from ink-0/week1/6/TodoDelete

92fa970

[week1] TodoList Delete 구현

[week1] TodoList Edit 구현

e5fe50f

- ink-0/js-todo-list-step1/#8

[week1] TodoList Edit 구현 일부 수정

56b3cfa

- ink-0/js-todo-list-step1/#8

[week1] TodoList 리팩토링

4fa6f12

- ink-0/js-todo-list-step1/#8

Merge pull request #9 from ink-0/week1/8/TodoEdit

e1c0dbc

Week1/8/todo edit

[week1] TodoFilter 구현

15c4aab

- 상태 추가 filterTodoes, isFilter, todoesCount - component 네이밍 변경 TodoCount -> TodoFilter - ink-0/js-todo-list-step1/#10

[week1] 코드 리팩토링

b4e07a0

- const 생성: 메세지, class data 분리 - 객체 리터럴 적용 - ink-0/js-todo-list-step1/#10

Merge pull request #11 from ink-0/week1/10/TodoFilter

fc05b25

Week1/10/todo filter

[week1] TodoFilter CSS 조건 수정

102b581

- 그외 const 대치 안한 것 수정 - ink-0/js-todo-list-step1/#12

[week1] localStorage 적용

c70cb01

-ink-0/js-todo-list-step1/#12

Merge pull request #13 from ink-0/week1/12/localStorage

cb7745e

Week1/12/local storage

[week2] GET userList

e45ba36

-ink-0/js-todo-list-step1/#14

Merge pull request #16 from ink-0/week2/14/GETuser

527420f

[week2] GET userList

[week2] UserList UI 생성

360338a

] -ink-0/js-todo-list-step1/#17

ink-0 added 2 commits July 19, 2021 19:24

[week2] UUserList 의 user에 클릭이벤트 거는중

90a6d0c

] -ink-0/js-todo-list-step1/#17

Merge pull request #18 from ink-0/week2/17/UserListUI

3654826

Week2/17/user list UI

ink-0 closed this Jul 19, 2021

chore:

a02aa27

ink-0 reopened this Jul 26, 2021

ink-0 commented Jul 26, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10기 Tami] TodoList with CRUD #217

[10기 Tami] TodoList with CRUD #217

ink-0 commented Jul 12, 2021 •

edited

Loading

renee13wi left a comment

renee13wi Jul 12, 2021

ink-0 Jul 12, 2021

renee13wi Jul 12, 2021

ink-0 Jul 12, 2021

renee13wi Jul 12, 2021

ink-0 Jul 12, 2021

jeleedev left a comment

jeleedev Jul 12, 2021

jeleedev Jul 12, 2021

ink-0 Jul 12, 2021

nerdyinu left a comment •

edited

Loading

nerdyinu Jul 12, 2021

nerdyinu Jul 12, 2021 •

edited

Loading

ink-0 left a comment

ink-0 Jul 26, 2021

ink-0 Jul 26, 2021

ink-0 Jul 26, 2021

ink-0 Jul 26, 2021

		@@ -0,0 +1,2 @@
		import TodoApp from './src/js/TodoApp.js';
		new TodoApp(document.querySelector('.App'));

[10기 Tami] TodoList with CRUD #217

Are you sure you want to change the base?

[10기 Tami] TodoList with CRUD #217

Conversation

ink-0 commented Jul 12, 2021 • edited Loading

📄 회고

📌 진행 상황

🎯 요구사항

renee13wi left a 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

jeleedev left a 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

nerdyinu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerdyinu Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

ink-0 left a 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

ink-0 commented Jul 12, 2021 •

edited

Loading

nerdyinu left a comment •

edited

Loading

nerdyinu Jul 12, 2021 •

edited

Loading