[3기 김휘겸] TodoList with CRUD #41

breakstorm · 2020-09-07T19:11:31Z

🚀 기본 요구사항

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를 반영하기. 따라서 새로고침하여도 저장된 데이터를 확인할 수 있어야 함

후기.
기본 기능들만 급하게 구현해서 제출을 합니다.

문제점.
코드에 대해서 테스트 미진행.
함수가 private으로 선언되어 있지 않다. (모듈화 및 클래스화 되어있지 않다.)

JunilHwang

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

몇 가지 리뷰 남겼으니 확인해보시면 좋을 것 같아요!

리뷰에 대한 요약은 다음과 같습니다.

변수는 일단 const로 선언한다고 생각해주세요.
- 변수에 할당한 값이 변경되는 경우는 생각보다 많지 않습니다.
- 객체나 배열 또한 값이 직접적으로 변경되는 것이 아니기 때문에 const로 관리해야합니다.
state/setState/render를 적극적으로 이용해보세요!
- 이번 미션을 소개하는 페이지에 상세하게 나와있습니다.
- 프론트엔드 상태관리
- TECH CONCERT: FRONT END 2019 - 데이터 상태 관리. 그것을 알려주마
if/else 등의 사용은 최대한 줄여주세요!
- else 분기문을 만들지 마라, 함수가 한 가지 일을 하게 만들어라

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

JunilHwang · 2020-09-08T07:27:29Z

README.md

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


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

이렇게 표기해야 마크다운에서 정상적으로 보인답니다!

예시:

- [X] todo list에 todoItem을 키보드로 입력하여 추가하기 - [O] todo list에 todoItem을 키보드로 입력하여 추가하기 - [o] todo list에 todoItem을 키보드로 입력하여 추가하기

todo list에 todoItem을 키보드로 입력하여 추가하기

[O] todo list에 todoItem을 키보드로 입력하여 추가하기

[o] todo list에 todoItem을 키보드로 입력하여 추가하기

JunilHwang · 2020-09-08T07:30:01Z

js/app.js

+import todoItem from './todoItem.js';
+import filterItem from './filterItem.js';
+let ID = 0;
+let DATA = []; // {id, context, complete}


/** * @typedef {object} TodoItem * @property {string} id : 아이디 값 * @property {string} context : 내용 * @property {boolean} completed : 완료여부 */ /** @type TodoItem[] **/ const DATA = [];

아예 이렇게 jsdoc을 이용하여 타입을 지정할 수 있습니다!
이럴 경우 Intellij나 vscode 등에서 저절로 타입을 보여준답니다.

https://code.visualstudio.com/docs/languages/javascript#_jsdoc-support
이런 기능이 있는줄 몰랐는데 감사합니다 (감동)

혹시 const로 선언한 DATA가 이벤트 처리시 발생하는 값을 재할당 하는데 error가 발생을 해서 let으로 변경을 하였습니다. 혹시 이를 해결할 방법이 있을까요?

아래와 같은 방식으로 현재 프로그램 내에서 DATA를 사용했습니다.

const foo = ['bar']; foo = ['foo']; Uncaught TypeError: Assignment to constant variable.

앗.. 이럴 경우엔 let을 사용해야합니다.

JunilHwang · 2020-09-08T07:31:20Z

js/app.js

+		inputEvent(data)
+		render()


inputEvent에서 render를 처리해주는건 어떨까요?

해당 방법 대로 수정을 해보겠습니다.
(위에 내용처럼 작성한 이유는 종속적인 호출(함수안에서 다음함수를 호출)을 적게 하는 것이 프로그램이 덜 복잡하지 않을까 생각을 하였는데, 객관적으로 더 낳아 보이는 것이 없는것 같습니다 ㅜㅜ)

JunilHwang · 2020-09-08T07:32:01Z

js/app.js

+        let data = {
+            id: ID++,
+            context: valueTrim,
+            complete: false
+		}
+		inputEvent(data)
+		render()


Suggested change

let data = {

id: ID++,

context: valueTrim,

complete: false

}

inputEvent(data)

render()

inputEvent({

id: ID++,

context: valueTrim,

complete: false

})

render()

선언하지 말고 바로 넘겨줘도 좋을 것 같습니다!

JunilHwang · 2020-09-08T07:32:49Z

js/app.js

+		target.value = labelElement.innerHTML
+	} else if ( key === 'Enter' && valueTrim) {
+		let index = parent.id.split('-')[1]
+		DATA[index].context = valueTrim


DATA를 직접 다루지 말고 setData 같은 메소드 혹은 함수를 만들어서
setData가 Data를 교체한 다음에 render를 실행하도록 하면 좋을 것 같습니다!

의견 주셔서 감사합니다.

JunilHwang · 2020-09-08T07:33:41Z

js/app.js

+	if (target.className === 'destroy') {
+		DATA = DATA.filter((v) => {
+			return ( `item-${v.id}` === targetId ) ? false : true;
+		})
+		render();
+	}


앞선 리뷰와 동일합니다.

JunilHwang · 2020-09-08T07:36:44Z

js/app.js

+	let filteredData = DATA.filter((v) => {
+		if (HASH === 'completed') {
+			return (v.complete) ? true : false
+		} else if (HASH === 'active') {
+			return !(v.complete) ? true : false
+		} else {
+			return true
+		}
+	})


Suggested change

let filteredData = DATA.filter((v) => {

if (HASH === 'completed') {

return (v.complete) ? true : false

} else if (HASH === 'active') {

return !(v.complete) ? true : false

} else {

return true

}

})

const filteredData = DATA.filter(({ completed }) => (HASH === 'all') ||

(HASH === 'completed' && completed) ||

(HASH === 'active' && !completed));

이런식으로 표현할 수 있습니다.
일단 변수는 무조건 const로 선언한다고 생각해주세요. 보통 변수에 저장한 값이 변하는 경우는 거의 없습니다.
배열이나 객체 또한 변수를 직접적으로 변경하는게 아니라 변수가 바라보고 있는 주소의 값이 변경되는 것이기 때문에 const로 해줘야합니다.

적어주신 방법으로 조건식을 표현하는 것이 더 잘 읽혀서 좋습니다.
좋은 방법 알려주셔서 감사합니다.

JunilHwang · 2020-09-08T07:37:06Z

js/app.js

+		}
+	})
+
+	$todoListElement.innerHTML = filteredData.map( (item) => todoItem(item) ).join('')


Suggested change

$todoListElement.innerHTML = filteredData.map( (item) => todoItem(item) ).join('')

$todoListElement.innerHTML = filteredData.map(todoItem).join('')

이렇게 표현할 수 있습니다

올바른 사용법을 알려 주셔서 감사합니다.

이렇게 표현할 수 있습니다

오 바로 인자로 넘기는군요 새로운 걸 알게됐습니다 감사합니다 ㅎㅎ

JunilHwang · 2020-09-08T07:37:35Z

js/filterItem.js

+    <li>
+        <a class="all${(hash === 'all' ? ' selected' : '')}" href="/#">전체보기</a>
+    </li>
+    <li>
+        <a class="active${(hash === 'active' ? ' selected' : '')}" href="#active">해야할 일</a>
+    </li>
+    <li>
+        <a class="completed${(hash === 'completed' ? ' selected' : '')}" href="#completed">완료한 일</a>
+    </li>


버튼에 배열을 만든 다음에 map으로 만들면 더 좋지 않을까요?

예를 들면 위의 DATA와 같이 FILTER라는 배열을 만들어서 state 관리를 하는 방법을 말씀 하시는 거죠?

말씀 처리 해주셔야 template안에서 연산이 들어가지 않아서 코드가 일관될 것 같습니다.
해당 방법으로 변경을 해보도록 하겠습니다.

JunilHwang · 2020-09-08T07:38:10Z

js/todoItem.js

+const todoItem = ({id, context, complete}) => {
+    return `
+    <li id="item-${id}" class="${(complete) ? ' completed' : ''}">
+        <input type="text" class="edit" value="${context}">
+        <input type="checkbox" id="toggle-${id}" class="toggle" value="${complete}" ${(complete) ? 'checked' : ''}>
+        <label for="" class="">${context}</label>
+        <button class="destroy"></button>
+    </li>
+    `
+}
+
+export default todoItem;


Suggested change

const todoItem = ({id, context, complete}) => {

return `

<li id="item-${id}" class="${(complete) ? ' completed' : ''}">

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

<input type="checkbox" id="toggle-${id}" class="toggle" value="${complete}" ${(complete) ? 'checked' : ''}>

<label for="" class="">${context}</label>

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

</li>

`

}

export default todoItem;

export default ({id, context, complete}) => `

<li id="item-${id}" class="${(complete) ? ' completed' : ''}">

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

<input type="checkbox" id="toggle-${id}" class="toggle" value="${complete}" ${(complete) ? 'checked' : ''}>

<label for="" class="">${context}</label>

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

</li>

`;

이렇게 표현할 수 있습니다.

breakstorm · 2020-09-10T01:40:21Z

느낀점

리뷰에서 느낀점.
1. Javascript 문법을 명확하게 알고서 사용하고 있지 않다는 것을 알게 되었다.
  - Array.protype.map 사용시 argument 전달 방법
  - javascript 모듈화
  - arrow function 반환값
2. 상태관리 모듈화 (state / setState / render)
  - 현재는 전역변수, 전역 함수로 구현이 되어 있습니다.
3. IF 조건문 표현 방법
  - 단순 if / if else를 사용 하였는데, 1개의 조건문을 더욱 구체적으로 작성해서 1문장으로 작성할 수 있는 것을 알게 되었습니다.
4. VSCode tip
  - JSDoc 기능을 지원하여서 계속 사용하기 좋은 코드를 만들수 있습니다.
개인적으로 느낀점.
1. 요구사항 분석을 명확하지 않고, 시간도 많이 소요가 되었다.

앞으로 진행할 일

미션2 기본요구사항을 위에 피드백 해주신것을 적용해서 빠르게 만들기.
미션1에 피드백 사항을 적용해서 개선 하기.

breakstorm added 11 commits September 1, 2020 21:56

fork Initial commit

7033521

Add 요구사항 분석서 작성 (작성중)

f993128

Update 개발명세서 작성

45a326b

HTML todoItem template 목업 작성

d79015c

입력 이벤트 처리

04b9b0f

이벤트에 대한 가이드 작성

e59fbd9

각 이벤트들 선언

c774641

할일 완료 및 토글

d974f45

할일 삭제, 할일 카운트 표시

5b902c6

필터 기능

3c4f79b

수정 및 수정취소 기능

81dac01

ganeodolu added the 🚀 필수미션 label Sep 8, 2020

ganeodolu assigned breakstorm Sep 8, 2020

JunilHwang reviewed Sep 8, 2020

View reviewed changes

eastjun-dev added the TeamD label Sep 8, 2020

eastjun-dev merged commit 447021f into next-step:breakstorm Oct 21, 2020

breakstorm deleted the breakstorm branch October 21, 2020 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3기 김휘겸] TodoList with CRUD #41

[3기 김휘겸] TodoList with CRUD #41

breakstorm commented Sep 7, 2020

JunilHwang left a comment

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

breakstorm Sep 10, 2020

JunilHwang Sep 10, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

JunilHwang Sep 8, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

lsw1164 Sep 11, 2020

JunilHwang Sep 8, 2020

breakstorm Sep 10, 2020

JunilHwang Sep 8, 2020

breakstorm commented Sep 10, 2020 •

edited

	$todoListElement.innerHTML = filteredData.map( (item) => todoItem(item) ).join('')
	$todoListElement.innerHTML = filteredData.map(todoItem).join('')

[3기 김휘겸] TodoList with CRUD #41

[3기 김휘겸] TodoList with CRUD #41

Conversation

breakstorm commented Sep 7, 2020

JunilHwang 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breakstorm commented Sep 10, 2020 • edited

breakstorm commented Sep 10, 2020 •

edited