[Study] Feedbacks

[S0YKIM]

1. src/js/index.js 23-26 this.state 에 id 와 counterArray 를 저장해서 상태 관리하는 방법 효율적이고 좋아요!

2. src/js/index.js 69-73 질문: CounterList 에 클릭 이벤트가 발생하면 플러스, 마이너스, 리셋, 리무브 중에 해당하는 함수가 실행되는 것은 이해가 됩니다. 그런데 72-73 줄에서 if (e.target.nodeName !== 'BUTTON') { return; } 를 추가해야 하는 이유가 무엇인지 궁금합니다. 혹시 버튼을 클릭하지 않고 빈 공간을 클릭했는데 그게 패딩된 공간이었을 경우를 대비한 걸까요?

3. src/js/index.js 84 제 경우에는 +, - 등의 각각의 버튼에 전부 addEventListner() 를 추가했는데 아예 이들을 묶은 CounterList 에 한번만 이벤트리스너를 추가하고 contains 메서드로 어떤 버튼을 누른건지 확인하는 방식 깔끔해서 좋아요.

4. src/css/style.css 123 저도 자바스크립트 자체의 버튼을 꾸미기보다 외부 이미지를 끌어와 사용하고 싶었는데 이렇게 하면 되는군요! 이미지를 배경에 삽입하고 컬러를 변경하면 된다는 것 배우고 갑니다.

디자인도 센스 있으시고 코드도 자바스크립트 개념을 잘 활용하셔서 전체적으로 완성도 있는 프로젝트라는 인상을 받았습니다 :)

[MichelleJin12]

1. npm http-server로 올려봤는데 [2021-09-28T15:30:21.291Z] "GET /dist/assets/site.webmanifest" "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/94.0.46nifest" Error (404): "Not found" 요런 에러가 뜬다.
2. 무언가 리소스를 로드하지 못하는 것 같다. 처음 켰을 때, console에 뜨는 에러 site.webmanifest:1 Failed to load resource: the server responded with a status of 404 (Not Found) site.webmanifest:1 Manifest: Line: 1, column: 1, Syntax error. site.webmanifest:1 Failed to load resource: the server responded with a status of 404 (Not Found)
3. 화면을 줄였다 늘였다 했을 때 버튼이 세로 정렬되는 기능이 있으면 좋을거같다.
4. 추가할 때 어떤 식으로든 카운터마다 번호를 넣어서, 삭제할 때 어떤 카운터가 삭제되었는지 알면 좋겠다(현재는 사라지면 다시 정렬되어 어떤 카운터가 사라졌는지 모름)
5. 클릭뿐만 아니라 키보드로도 reset, plus, minus, delete, add 할 수 있으면 좋겠다.
6. 꾸우욱 눌렀을 때 1초마다 계속 증가하거나 감소하는 기능이 있으면 좋겠다.
7. 100단위 넘어가면 버튼이 오른쪽으로 밀리는데 숫자|버튼 정렬되면 좋을거같다.

• 고생하셨씁니닷~!!!!!!!!!!

[hhkim0729]

1. 역시 사랑님... open graph는 회사다닐 때 퍼블리싱 외주 준 곳에서 해준 적밖에 없어서 직접 해본 적이 없는데, 이거까지 신경 쓰시다니 👏 웹팩도 쉽지 않은 길이었지만 결국 다 해결해내시고! 늘 발전하려고 노력하시는 거 배워가요 호호
2. 윈도우에서 npm start는 안 되는군요... webmanifest 오류는 아직 구현하다 마신 것 같아서 패스하겠습니다~!
3. + 버튼만 진하고 나머지는 연한 색인 이유가 궁금해요. 왠지 활성화가 안 된 버튼 느낌이랄까..? (버튼 누르면 쏙 들어가는 거 귀여워서 계속 누르게 돼요 👍)
4. 폰트도 난생 처음 보는 거라 또 배워갑니다...
5. 카운터를 많이 추가하면 위로 밀려서 위에 카운터들이 사라지는 오류가 있어요. (body의 height 값이 고정되어 있어서 그런 것 같네요~!)
6. 카운터 추가하고 스크롤해서 내려가서 또 추가하고 약간 불편한 느낌? 갑자기 든 생각인데 카운터 추가 버튼을 fixed 하거나 할 수 있지 않을까 싶네요.
7. index.js 22번째 라인 function App 이게 결국 클래스의 전신인 것 같아서 클래스로 구현해보시는 것은 어떠신가요? 호호 23번째 라인 this.state 객체를 만들어서 모든 데이터를 관리하는 점이 인상깊네요. 42번째 라인 $CounterList 변수명이 대문자로 시작하는 이유가 있을까요? 68, 110번째 라인 조건이 항상 true인 거 아닌가 싶은데 검사하는 이유는 혹시 모를 상황을 대비해서인가욥..? 81번째 라인 findIndex하고 다시 counter 찾지 않고 그냥 find 메서드로 counter 인스턴스를 찾을 수 있지 않을까요?
8. 전체적으로 잘 작동하고 코드도 깔끔해서 보기가 좋았어요. 이후에 js 파일 분리하는 것도 해보시면 좋을 것 같아요. 고생하셨습니다!

[dopamingo]

따흐흑 쓰다가 날라갔네요 . . . 자동저장 만들어줘 . . .

1. 실시간으로 css가 바뀌는 건 기분탓일까 홀홀 다른 분들도 언급하신대로 Add a new counter 버튼이 상단에 고정되는 편이 나을 거 같습니다

2. 로컬스토리지에 counterArray 로 한 번 더 감싼 이유가 있을까요?

3. index.js :13 이 함수들만 객체로 만들어주신 이유가 궁금합니다. 재사용 목적?

4. index.js :106 true가 어떤 용도인가요? 알고싶다...!

깔끔하게 잘 해주셔서 많이 배웠습니다 고생 많으셨어요!!

[srngch]

@S0YKIM @MichelleJin12 @hhkim0729 @dopamingo 감사합니다!!

답변이 필요한 부분만 커맨트 남겨둡니다.

@S0YKIM

2. 버튼이 아닌 다른 부분을 클릭했을 때 콜백 함수를 빨리 종료시켜 주기 위해서에요! 어차피 코드 아래 부분에서 클래스를 체크해주고 있긴 하지만 필요없는 걸 체크하지 않고 함수가 빨리 종료되면 더 효율적이기 때문에... hhkim0729님 코드 참고해서 업데이트 했습니다.
3. 이벤트 위임!

@MichelleJin12

1. & 2. 둘 다 매니페스토 문제인데 아직 명확하게 알지 못 해서 일단은 readme Todo 목록에 추가해뒀습니다. 4~6. 이런 추가 기능 아이디어 너무 좋아요. 저희 같이 공유해서 업데이트 해도 좋겠네요!
2. 이것도 Todo 였지만 최근 커밋으로 반영 헀습니다☺️

@hhkim0729

1. 웹팩 처음인데 저번에 같이 봐주셔서 감사했습니다!
2. webpack-dev-server를 구현해보고 싶어서 추가해뒀는데 손을 못 댔어요. 일단 제외하는걸로!
3. 헉 비활성화를 생각 못 했네요. 단순하게 증가-진하게, 감소-연하게, 나머지 별로 중요도 낮음-연하게로 했는데.. 수정 완료!
4. 수정 완료!
5. fixed로 수정 완료!
6. index.js
   • 42: 수정 완료!
   • 68, 110: index.html에서 defer 옵션 주기 전에 넣어놨던건데 잊고 있었네요. 수정 완료!
   • 81: 인덱스를 사용해서 배열 요소를 접근, 삭제하는게 편하다고 생각 했어요. 근데 find와 filter를 쓰는게 더 선언형에 가까운 것 같아 수정 했습니다.

@dopamingo

1. fixed로 수정 완료!
2. app의 state를 그대로 저장했어요. 갱신되는 id를 위한 값인데 지금보니까 인스턴스 자체에 시간 기반으로 아이디 값을 만들어주는게 깔끔할 거 같네요.
3. 수정 완료! 아직 어느 경우에 객체, 클래스, 함수를 선택해서 작성해야 할지 확실하지가 않아서 관련해서 더 찾아봐야겠어요. 클래스를 파다 보니까 전부 다 클래스로 만들어야 할 것 같은 느낌😂
4. 작업 중일때 캡쳐링 테스트하려고 addEventListener()의 세 번째 인자로 넣었는데 콜백이 길어져서 당황스럽게 읽히는 코드가 완성되었네요. 수정 완료!