Skip to content

Conversation

@amorfati0310
Copy link
Collaborator

@amorfati0310 amorfati0310 commented Jan 15, 2020

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

다 못하였는데 한달쨰 PR 안 날리고 있어서 부끄러워서 일단 한데 까지만 이라도 날리고
이어서 작업하도록 하겠습니다 ㅜ

아쉬운 점

  1. class를 사용했는데 너무 this bind가 남용되고 있는 것 같습니다.
  2. 상수로 뺄 수 있는 부분 빼지 못한 점
  3. 뷰 모델 연결관계 구조 및 네이밍
    vue 에 Dep와 비슷한 구조와 + extends를 활용하고 dispatch action 등 네이밍만 비슷하게 사용한게 아닌지 어색한 느낌이 강하네요
  4. 웹팩 설정을 하다가 말았네요 다음스텝 넘어가면서 더 추가해보겠습니다 _ㅇ

특히 더 리뷰 받고 싶은 부분

  • 아쉬운 점 3번에서 얘기한 뷰 모델 연결 관계 어색하지 않은지, 수정했으면 좋겠다 하는 부분에 대해서 많은 피드백 받고 싶습니당

ETC

  • EDIT_MODE 에서 focusout event를 감지 못하는데 input event를 제대로 잘 몰라서 삽질을 하고 있는 중입니다 수정중이 발생할 때 inputEl -> focusout 이벤트리스너를 달아주면 될것이라 생각했는데 동작을 안 하네용;ㅁ;
    혹시 아시는 분 도움주실 말 있으면 감사하겠습니다

@amorfati0310 amorfati0310 self-assigned this Jan 16, 2020
@amorfati0310 amorfati0310 changed the title Mission001/dali [mission001] todolist crud Jan 16, 2020
@ganeodolu
Copy link
Collaborator

다시 만나게 되어서 반가워요. 저도 많이 밀리다보면 쫓아가기 버겁고, 그러다가 보면 놓아버리는 경우가 생기더라구요. 계속 함께 발전해 나갔으면 좋겠습니다. 😃
index.html을 열어보면 작동을 하지 않는데 혹시 script로 js파일을 불러오는 부분은 필요 없는지요??
그래서 app.js에서 css도 불러오지를 못하는 것 같아요.

@HoseokNa
Copy link
Collaborator

안녕하세요!
늦었지만 코멘트 달아봅니다.
리뷰어는 아니지만 소감과 몇가지 조심스럽게 피드백 남기겠습니다.

먼저 전체적인 느낌을 말씀드리겠습니다.

  1. 파일이 세부적으로 나누어져 있어서 보기 좋았습니다. (api, component, store, util 등)

  2. dom을 사용하는 함수들을 dom.js로 관리하는게 인상 싶었습니다.

  3. vue를 사용해 보지 않았었는데 어떤 느낌인지 간접적으로 알 수 있어서 좋았습니다.

다음은 피드백 내용인데 아직 많이 부족해서 작은 부분만 해드릴 수 있는 점 양해 부탁드리겠습니다.

  1. vue를 사용하지 않아서 네이밍과 구조에 대해서 피드백 드리기가 조심스럽네요. 이 부분은 다른 분들의 조언을 구하겠습니다.

  2. 전체적으로 공백이 여러개 들어가 있는 파일이 있습니다. 혹은 공백이 없거나. (ex import 다음, export 전) 이 부분은 통일 시키면 보기 더 좋을 것 같습니다.

  3. api의 index.js에는 destructing을 사용 됐는데 다른 곳에도 적용되면 좋을 것 같습니다. 아니면 혹시 다른 이유가 있는 것인지 궁금합니다.
    const id = getClosetLI(target).dataset.id
    { id } = getClosetLI(target).dataset

  4. switch 문에 주석이 달린 부분이 있는데 좀 더 구체적인 증상이 궁금합니다. break도 사용 안했을 때 case를 모두 들어가게 되는 말씀인지 궁금하네요. 문자열 비교이기 때문에 상관 없을 것 같은데 이상한 현상이네요. 증상 재현을 해보려고 했는데 아직 환경 구성에 능숙하지 못해서 구동을 못했습니다.(흑흑)

export const getTasks = async () => {
const { data } = await axios.get(TASKS_URL);
return data;
}; No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

전체적으로 end of line이 없는 것 같습니다. File의 EOL은 POSIX환경에서 정해놓은 일종의 명세이니 지켜주시면 좋습니다. 파일 끝에 개행을 추가해야 하는 이유
루카스가 예전에 달아준 코멘트인데 복붙 해왔습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

전체적으로 end of line이 없는 것 같습니다. File의 EOL은 POSIX환경에서 정해놓은 일종의 명세이니 지켜주시면 좋습니다. 파일 끝에 개행을 추가해야 하는 이유
루카스가 예전에 달아준 코멘트인데 복붙 해왔습니다!

마지막줄에 엔터키가 필요하다는 의미로 보면 될까요? 제 파일을 확인해보니 대부분 안되어있었네요. ㅜ.ㅜ

}
handleClicked({target}){
const id = getClosetLI(target).dataset.id
// 의문점 switch case return 안하니까 둘다 탄다 ?? 왜 ?
Copy link
Collaborator

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.

공부하다가 혹시 이 내용일 수도 있어서 남겨 봅니다.

아래의 switch 문처럼 break가 없을 경우
toggle, destroy 둘 다 출력이 됩니다.

let className = "toggle";

switch (className) {
	case "toggle":
    console.log("toggle");
	case "destroy":
    console.log("destroy");
}
// toggle
// destroy

switch 문이 case 표현식의 매칭이 끝나고 break 없는 코드 블록의 내용이 다 실행되면
다음 case의 표현식을 매칭하지 않고 바로 코드 블록의 내용이 실행 되는 것 같습니다.
잘못알고 있었네요.
그리고 이건 you don't know js 에 나온 switch 내용이니 참고해보시면 좋을 것 같습니다.
switch

@amorfati0310
Copy link
Collaborator Author

다시 만나게 되어서 반가워요. 저도 많이 밀리다보면 쫓아가기 버겁고, 그러다가 보면 놓아버리는 경우가 생기더라구요. 계속 함께 발전해 나갔으면 좋겠습니다. 😃
격려 너무 감사합니다 ㅜ .. 요즘 멘탈 번 아웃이 왔었나보네요 ...
index.html을 열어보면 작동을 하지 않는데 혹시 script로 js파일을 불러오는 부분은 필요 없는지요??

  • 아 실행이 되려면 package.json에 webpack 을 실행시켜야 보이는데 좀 더 테스트 환경 개선할 수 있게 적용해볼게요 감사합니다 ~~ :)

그래서 app.js에서 css도 불러오지를 못하는 것 같아요.

@amorfati0310
Copy link
Collaborator Author

안녕하세요!
늦었지만 코멘트 달아봅니다.
리뷰어는 아니지만 소감과 몇가지 조심스럽게 피드백 남기겠습니다.

먼저 전체적인 느낌을 말씀드리겠습니다.

  1. 파일이 세부적으로 나누어져 있어서 보기 좋았습니다. (api, component, store, util 등)
  2. dom을 사용하는 함수들을 dom.js로 관리하는게 인상 싶었습니다.
  3. vue를 사용해 보지 않았었는데 어떤 느낌인지 간접적으로 알 수 있어서 좋았습니다.

좋게봐주셔서 감사합니다 ㅜㅜ vue에 dep만 가지고 온 부분이여서 이상하게 짬뽕시켰을 확률이 높아요

다음은 피드백 내용인데 아직 많이 부족해서 작은 부분만 해드릴 수 있는 점 양해 부탁드리겠습니다.
아닙니다 저도 마찬가지입니다

  1. vue를 사용하지 않아서 네이밍과 구조에 대해서 피드백 드리기가 조심스럽네요. 이 부분은 다른 분들의 조언을 구하겠습니다.
    아닙니다 적극적으로 의견 주시면 감사하겠습니다 ~~ :)
  2. 전체적으로 공백이 여러개 들어가 있는 파일이 있습니다. 혹은 공백이 없거나. (ex import 다음, export 전) 이 부분은 통일 시키면 보기 더 좋을 것 같습니다.
    감사합니다 신경써볼게요
  3. api의 index.js에는 destructing을 사용 됐는데 다른 곳에도 적용되면 좋을 것 같습니다. 아니면 혹시 다른 이유가 있는 것인지 궁금합니다.
    const id = getClosetLI(target).dataset.id
    { id } = getClosetLI(target).dataset
    이게 더 좋을 것 같네요 적용할게요 감사합니다
  4. switch 문에 주석이 달린 부분이 있는데 좀 더 구체적인 증상이 궁금합니다. break도 사용 안했을 때 case를 모두 들어가게 되는 말씀인지 궁금하네요. 문자열 비교이기 때문에 상관 없을 것 같은데 이상한 현상이네요. 증상 재현을 해보려고 했는데 아직 환경 구성에 능숙하지 못해서 구동을 못했습니다.(흑흑)
    다시 한 번 살펴볼게요 감사합니다 ~~

@amorfati0310
Copy link
Collaborator Author

일요일에 마저 안 된 부분 진행하고 피드백 요청 드릴게유 ~~

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.

3 participants