Skip to content

Conversation

@StellaKim1230
Copy link
Collaborator

@StellaKim1230 StellaKim1230 commented Dec 25, 2019

#7 [mission002]

  • 1. 불러오기
  • 2. 추가하기
  • 3. 삭제하기
  • 4. 토글
  • 5. localStorage에 데이터 넣기.

궁금한점.

코드 시도 후 아래와 같은 에러가 나왔는데, 이유를 모르겠습니다. 같이 고민해보아요!

  1. fetchTodoData 안에 있는 this.setState is not function 이라고 나옴
function App() {
  const fetchTodoData = async () => {
    const data = await apiHandler({ url: 'http://todo-api.roto.codes' })
    this.setState(data)
  }
  fetchTodoData()
  this.setState = (data) => {
    this.todoList.setState(data)
  }
}
  1. this.todoList.setState(data) 부분에서 this.todoList 가 undefined 라고 에러남
function App() {
  const fetchTodoData = async () => {
    const data = await apiHandler({ url: 'http://todo-api.roto.codes' })
    setState(data)
  }
  fetchTodoData()
  this.todoList = new TodoList({
    data: this.data,
  })
  function setState(data) {
    this.todoList.setState(data)
  }
}
  1. fetchTodoData 안에 있는 setState(data) 에서 Cannot access ‘setState’ before initialization 이라고 나옴
function App() {
  const fetchTodoData = async () => {
    const data = await apiHandler({ url: 'http://todo-api.roto.codes' })
    setState(data)
  }
  fetchTodoData()
  const setState = (data) => {
    this.todoList.setState(data)
  }
}

kim Stella and others added 9 commits December 23, 2019 14:08
1. call todo api.
2. add todo api.
3. remove todo api connect.
4. add toggle api connect
5. add all todo count.
1. 전체보기, 헤야할 일, 완료한 일 클릭시 데이터, 총 데이터 수 보여주기.
localStorage에 데이터 넣기.
@StellaKim1230 StellaKim1230 changed the title [mission002] todolist 비동기/온오프라인 고려하여 업데이트하기 [mission002] TODOS 시스템에 비동기/온오프라인을 고려하여 업데이트하기 Dec 26, 2019
}

onKeyDown = async (e) => {
if (e.key === ENTER) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

e.key를 썼을 때, input value가 한글일 경우 리스트에 두 개씩 추가되는 버그가.. 있더라구요.
이건 e.key 대신 e.keyCode를 쓰면 해결되는데, keyCode가 표준이 아니라서ㅜㅜ..
다른 분들은 어찌하셨는지 궁금하네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

동준님이 해결해주셧녜요 ㅠㅠ

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 e.code일때 두번 입력되어서 e.key로 변경하고 작동도 정상이라 해결했다고 생각했는데 e.key도 동일한 증상이 있나요?

@StellaKim1230 혹시 동준님이 어떻게 해결하셨는지 알려주실 수 있나요??

e.key를 썼을 때, input value가 한글일 경우 리스트에 두 개씩 추가되는 버그가.. 있더라구요.
이건 e.key 대신 e.keyCode를 쓰면 해결되는데, keyCode가 표준이 아니라서ㅜㅜ..
다른 분들은 어찌하셨는지 궁금하네요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@StellaKim1230 동준님 PR #15 리뷰내용 중 엔터 연속입력시 처리방법 말씀하시는 거였군요. e.key랑은 다른 문제였네요. 😓

@@ -0,0 +1,22 @@
const username = 'kimjieun'

export const apiHandler = async ({ url, method, body, customUrl }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

apiHander 함수를 만들어서 하나로 쓰는 게 인상깊습니다.
중복코드가 많이 줄어들 것 같아요.

@@ -0,0 +1 @@
export const hostUrl = 'http://todo-api.roto.codes'
Copy link
Collaborator

Choose a reason for hiding this comment

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

hostUrl만 따로 url.js로 관리하시는 이유가 있을까요?
username은 api.js에서 관리하고, hostUrl은 url.js에서 관리하는 기준이 궁금합니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 실수한 부분입니다ㅎㅎ 따로 관리하는게 맞을것 같아요 ㅎㅎ

}

onKeyDown = async (e) => {
if (e.key === ENTER) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Enter 키를 눌렀을 때 -> input value가 비어있으면, 값이 추가되지 않는 조건을 넣어주면 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오! 꼼꼼한 리뷰 감사합니다!

}
}

this.fetchTodoData()
Copy link
Collaborator

Choose a reason for hiding this comment

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

input에 값을 칠 때마다 get api가 불러와지네요.
Enter키를 눌러 값이 추가되었을 때만 불러와도 될 것 같습니다.

Copy link
Collaborator

@s280493 s280493 left a comment

Choose a reason for hiding this comment

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

올려주신 궁금한 점 1, 2, 3이 제 컴퓨터에서는 재현이 안 되더라구요!
아무 콘솔 오류도 없어요.

지은님 컴퓨터에서는 어떻게 보이는지 궁금하네요~

this.fetchTodoData()
}

onTodoCheck = (todoStatus) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

해야할 일, 완료한 일 필터를 눌렀을 때 필터클래스에 selected를 넣어서 필터주위에 빨간 테두리가 유지되어있으면 더 좋을 것 같아요.

<link rel="stylesheet" href="css/style.css" />
</head>
<body>
<div id="app">
Copy link
Collaborator

@ganeodolu ganeodolu Jan 6, 2020

Choose a reason for hiding this comment

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

<div id="app"><div id="app" class="todoapp">로 변경하면 기존 css가 잘 적용될 것 같아요.

Copy link
Collaborator

@YongHoonJJo YongHoonJJo left a comment

Choose a reason for hiding this comment

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

리뷰 늦게 드려 죄송합니다.ㅜ 눈에 보이는 몇 가지에 대해 리뷰 남겨보았습니다..!!
그리고, 질문 1, 2, 3은 해결 되신건가요?
1, 2 는 this 에 해당 프로퍼티들이 없어서 발생한 문제 같아 보여서요..ㅜ

this.$selector.addEventListener('click', (e) => {
if (e.target.className === ACTIVE) this.onTodoCheck(ACTIVE)
if (e.target.className === COMPLETED) this.onTodoCheck(COMPLETED)
if (e.target.className === ALLSELECTED) this.onTodoCheck(ALLSELECTED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

위에서 만족하는 조건식이 있어도 나머지 조건식들에 대한 검사가 이루어질 것 같아요!! switch-case 문이나, this.func() 실행 후 return 을 통해 함수를 확실히 종료해주는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

용훈님 말씀대로 return 이나 swtich case 문으로 바꿔 주는 것이 좋겠네요! 필요없는 로직 검사가 안이루어지니깐 ㅎ 조언 감사합니다

init = () => {
this.$selector.addEventListener('click', (e) => {
if (e.target.className === DESTROY) this.onDeleteTodo(e.target.parentNode.dataset.idx)
if (e.target.className === TOGGLE) this.onToggleTodo(e.target.parentNode.dataset.idx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

const className = e.target.className 과 같이 프로퍼티 캐싱처리 후 위에서 언급했던 것 처럼 switch-case 문이나 if 문 뒤에 return 으로 종료시켜주는 것은 어떨까요?


createLiClassName = (isCompleted) => {
if (!isCompleted) return 'view'
if (isCompleted) return 'completed'
Copy link
Collaborator

Choose a reason for hiding this comment

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

isCompleted 의 경우 T/F 만 존재하기 때문에,

if (!isCompleted) return 'view'
return 'completed'

와 같이 하면, 굳이 return '' 를 넣지 않아도 될거 같아요.

그리고 return isCompleted ? 'completed' : 'view' 와 같이 처리할 수도 있구요!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그렇네요! 항상 삼항연산자는 다른사람이 리뷰해주면 생각나는듯 ㅠㅠ 습관화시켜야겠어요 ㅠ

createTodoListHtmlString = ({ content, isCompleted, _id }) => {
return `<li class=${this.createLiClassName(isCompleted)}>
<div data-idx=${_id} class="view">
<input class="toggle" type="checkbox" ${isCompleted === true && 'checked'}>
Copy link
Collaborator

@YongHoonJJo YongHoonJJo Jan 7, 2020

Choose a reason for hiding this comment

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

isCompoled 가 false 면 <input class="toggle" type="checkbox" false> 이런식으로 남게 되지 않을까요? (전에 제가 이렇게 했었지만요..ㅎ)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅎㅎ 테스트해보겠습니다! ㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

false 안남는것으로 확인했습니다 ㅎㅎ

const todoData = this.data.filter(d => {
if (todoStatus === ACTIVE) return !d.isCompleted
if (todoStatus === COMPLETED) return d.isCompleted
if (todoStatus === ALLSELECTED) return this.data
Copy link
Collaborator

Choose a reason for hiding this comment

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

this.data 는 항상 true 를 리턴해야하기 때문에 이렇게 두신건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

전체 할일 선택시에는 모든 데이터가 나와야 하기 때문에 저렇게 구현 한 것 같아요!

@StellaKim1230
Copy link
Collaborator Author

올려주신 궁금한 점 1, 2, 3이 제 컴퓨터에서는 재현이 안 되더라구요!
아무 콘솔 오류도 없어요.

지은님 컴퓨터에서는 어떻게 보이는지 궁금하네요~

제가 new App()을 안했었던 것 같기도 허고 ㅠㅠ 다시 해봐야겠네요 ㅠㅠ

@StellaKim1230
Copy link
Collaborator Author

리뷰 늦게 드려 죄송합니다.ㅜ 눈에 보이는 몇 가지에 대해 리뷰 남겨보았습니다..!!
그리고, 질문 1, 2, 3은 해결 되신건가요?
1, 2 는 this 에 해당 프로퍼티들이 없어서 발생한 문제 같아 보여서요..ㅜ

아뇨 아직 ㅠ this에 해당 프로퍼티 들이 없다는 말씀은 this.setState() 이 함수자체가 없다는 말씀이신가요??ㅠㅠ
아니면 제가 new App() 을 안해서 발생한 거 같기도 하고 ㅠㅠ 헷갈리네요 항상 ㅠ

@YongHoonJJo
Copy link
Collaborator

리뷰 늦게 드려 죄송합니다.ㅜ 눈에 보이는 몇 가지에 대해 리뷰 남겨보았습니다..!!
그리고, 질문 1, 2, 3은 해결 되신건가요?
1, 2 는 this 에 해당 프로퍼티들이 없어서 발생한 문제 같아 보여서요..ㅜ

아뇨 아직 ㅠ this에 해당 프로퍼티 들이 없다는 말씀은 this.setState() 이 함수자체가 없다는 말씀이신가요??ㅠㅠ
아니면 제가 new App() 을 안해서 발생한 거 같기도 하고 ㅠㅠ 헷갈리네요 항상 ㅠ

new App() 을 안하면 function App() { ... } 은 그냥 함수이기 때문에, this 가 존재하지 않을 것 같아요!!
Obj.method() 에서 this 는 Obj 가 되는데, method 를 호출한 주체를 확인해보셔야 할 것 같아요.!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants