Conversation
everything-flows
left a comment
There was a problem hiding this comment.
2주차 과제 너무 고생하셨습니다!!🥰🥰🥰
따로 상태 관리 라이브러리를 쓸 수 없어서 props drilling때문에 많이 고생하셨죠?😭😭😭
그런데 배포된 링크가 권한 문제로 열리지 않는 것 같은데, 한번 확인해주실 수 있나요?
| ...react.configs.recommended.rules, | ||
| ...react.configs['jsx-runtime'].rules, | ||
| ...reactHooks.configs.recommended.rules, | ||
| 'react/jsx-no-target-blank': 'off', |
There was a problem hiding this comment.
제가 보기에는 이 프로젝트에서 react/jsx-no-target-blank 규칙을 끌 필요는 없을 것 같은데, 원래 사용하시던 규칙들을 가져오신건가요?!
| 'react-refresh/only-export-components': [ | ||
| 'warn', | ||
| { allowConstantExport: true }, | ||
| ], |
There was a problem hiding this comment.
저도 react-refresh라는 eslint 규칙이 있다는 걸 배웠습니다..! 그런데 이쪽도 이번 프로젝트에서는 설정할 필요 없는 것 같아요!
추가로, 지금처럼 상수들을 export할 때 jsx 파일이 아니라 color.js같이 js 파일에서 export하신다면 문제가 발생하지 않으므로 삭제하셔도 될 것 같아요!!
| @@ -0,0 +1,43 @@ | |||
| { | |||
There was a problem hiding this comment.
추가로 Vite를 이용해서 프로젝트를 잘 세팅해주신 것 같아요!!👍👍
그런데 혹시 따로 Vite를 사용하신 이유가 있을까요?
또, 프로젝트 안에 따로 week2-ceos 폴더를 따로 만들고 프로젝트를 세팅하셨는데 root 폴더에 바로 세팅하지 않으신 이유가 있을까용?! 지금대로면 root에서 npm start를 하면 빈 페이지가 실행되고, 원하는 페이지를 열려면 week2-ceos로 이동해서 npm run dev 로 실행시켜야 할 것 같은데, 처음 프로젝트를 보는 사람이면 실행법을 헷갈릴 수 있다고 생각했습니다! 만약 week2-ceos 파일들만 사용하면 root에 있는 src 등의 파일들은 다 지워도 될 것 같아요 ;)
There was a problem hiding this comment.
아 일단 vite 를 사용한 이유는,
일단 yarn + vite 를 쓰는게 습관이 된 이유도 있고, cra가 vite보다 상대적으로 느려서요!
사실 이번 과제와 같이 작은 플젝은 별로 상관은 없지만, 그래도 이왕이면 빠르고 최신거?를 사용하자!는 마음으로 진행했습니다!
그리구 원래 과제를 할 때 저만의 폴더를 만들고 시작하는게 맘이 편하더라구요,,ㅎㅎ
| return ( | ||
| <ThemeProvider theme={theme}> | ||
| <MainPage /> | ||
| <GlobalStyle /> |
There was a problem hiding this comment.
globla style을 잘 지정해주신 점 너무 좋아요👍👍
| } | ||
|
|
||
| const DayWrapper = styled.span` | ||
| ${rowFlex} |
There was a problem hiding this comment.
rowFlex를 따로 만들어 두는 방법은 생각 못했는데, 정말 좋은 것 같아요!
| const filtered = todoList.filter((list) => list.day === clickedDay); | ||
| if (filtered.length > 0) { | ||
| setFilteredTodoList(filtered[0].todos); | ||
| } else { | ||
| setFilteredTodoList([]); | ||
| } |
There was a problem hiding this comment.
이 부분도 위에서 언급드린 것처럼 find를 이용하실 수 있을 것 같아요!
| export default function TodosHeader(props) { | ||
| const { todoList, clickedDay } = props; | ||
| const [filterTodoList, setFilteredTodoList] = useState([]); | ||
| const [isDoneList, setIsDoneList] = useState([]); |
There was a problem hiding this comment.
아래 코드를 읽어 보니 isDoneList는 완료된 할 일의 갯수를 나타내는 것 같네요! 그런데 초기화는 빈 배열로 되고 있어서 코드를 읽는 데 혼란을 줄 수 있을 것 같아요. 이 부분 네이밍+초기값을 수정하시면 좋을 것 같아요!!
| {filterTodoList.length > 0 && | ||
| filterTodoList.map((content) => { |
There was a problem hiding this comment.
filterTodoList의 길이를 체크하고 map을 사용하는 방법이 좋네용!
저는 이런 경우에 filterTodoList?.map((content) => ...처럼 쓰거나, filterTodoList가 항상 배열임이 보장되면 그냥 filterTodoList.map((content) => ...처럼도 쓰는데 괜찮은 것 같아요! 혜인님은 어떻게 생각하시나요?
There was a problem hiding this comment.
저는 뭔가 에러를 조기에 잡을 수 없다는 생각이 들어서 길이 같이 바로 보이는 걸로 체크하고 코딩하는게 습관화된 것 같아요,,!!!
| @@ -0,0 +1,116 @@ | |||
| import { create } from "zustand"; | |||
There was a problem hiding this comment.
앗 zustand 쓰신 부분이 이부분이군요..! 결국 사용하지 않게 된 것 같아서 이번 코드리뷰에서는 건너뛸게용..! 다음 코드 리뷰에서 뵈어요🙇🙇
| :root { | ||
| font-family: Inter, system-ui, Avenir, Helvetica, Arial, sans-serif; | ||
| line-height: 1.5; | ||
| font-weight: 400; | ||
|
|
||
| color-scheme: light dark; | ||
| color: rgba(255, 255, 255, 0.87); | ||
| background-color: #242424; | ||
|
|
||
| font-synthesis: none; | ||
| text-rendering: optimizeLegibility; | ||
| -webkit-font-smoothing: antialiased; | ||
| -moz-osx-font-smoothing: grayscale; | ||
| } |
yyj0917
left a comment
There was a problem hiding this comment.
더 많은 코드리뷰를 남기고 싶었으나 혜인님 배포 링크가 열리지 않아서,,,코드만 보고 유추하기엔 아직 미숙하여 제대로 된 코드리뷰가 안 된 것 같습니다...! 링크 접속이 가능하게 된다면 다시 코드를 살펴보겠습니다. 그게 아니더라도 정말 배울 점이 많았던 것 같습니다! 고생많으셨습니다 😁
There was a problem hiding this comment.
eslint 설정관련해서 공부가 필요했는데 좋은 래퍼런스가 될 것 같습니다 감사합니다!
There was a problem hiding this comment.
제 컴퓨터에선 진짜 잘 열렸었는데ㅠ 지금 다시 재배포해두었습니다,,ㅠㅠ죄송해요ㅠㅠㅠㅠㅠ
There was a problem hiding this comment.
스타일을 유동적으로 바꿀 수 있게 설정한 것이 디자인 구조를 구성하는 데 있어 더 편리할 것 같습니다. 다양한 방법 배워갑니다!
| import { rowFlex } from "@styles/commonStyle"; | ||
|
|
||
| export default function TodoInput(props) { | ||
| const { handleInput, inputRef } = props; |
There was a problem hiding this comment.
혹시 이 부분에서 setTodoText도 같이 내렸는데 따로 안 받는 이유가 있을까요?!
| <Wrapper> | ||
| <LabelTextWrapper> | ||
| <CheckBoxLabel> | ||
| <CheckBox checked={isDone} onChange={handleCheck} $boxColor={boxColor} type="checkbox" /> |
There was a problem hiding this comment.
$로 스타일 Props를 내려줘서 구분과 동시에 사용할 수 있다는 것을 알게 되었습니다! 감사합니다~
| outline: 4px auto -webkit-focus-ring-color; | ||
| } | ||
|
|
||
| @media (prefers-color-scheme: light) { |
There was a problem hiding this comment.
이런식으로 적용할 수도 있군요! @media에 대해 더 공부해봐야겠습니다
✨ 구현 기능 명세
구현배포링크
🌟 미션 목표
🌠 Key Question
블로그에 정리해두었습니다
💎 필수 요건
🧩 PR Point
💜 Modal
할 일을 입력하는 부분을 추가하는 버튼을 클릭하면, 모달을 띄울 수 있도록 하였습니당,,,
useState로 Modal의 open, closed를 제어하도록 하였습니다.💚 Input
Input value 를
useRef훅을 사용하여, 제어할 수 있도록 코드를 구성했습니다.💛 customHook
💙 colorBox
사용자가 클릭한 색깔을 이용해서 checkbox의 border + backgroundColor 나타나도록
🧡 주간 별 날짜들이 나타나는 컴포넌트
🩷 styled-component
styled-component에서 globalStyle, theme을 만들어 두어서 필요할 때 불러와 사용했습니다.가장 많이 쓰는
flex관련 부분은 commonStyle 에 미리 저장해 두어 불러와 사용했습니다.🔥 Issue Log
블로그에 같이 정리해두었습니다
🥹 느낀 점
일단,,,, 처음에 과제 요건을 제대로 안 보고 zustand로 전역 상태 관리를 해버렸지 뭐에요,,ㅎㅎㅎ 이걸 제출 직전에 발견해서 부랴부랴 customhook으로 바꾸고 props drilling으로 바꾸었습니다. 시간이 나면 context api를 이용해서 리팩토링해보려고 합니다!!
저번에 너무 시간을 못 써서 이번에는 최대한 커밋도 나누고, 디자인도 예쁘게 하려고 노력했습니다! 디자인,,은 사실 제가 자주 쓰는 어플인 TimeBlocks의 할일 부분에서 따왔습니다..! (거의 따라했다,,?도 무방,,ㅎㅎㅎ) react,,, 다시 정말 열심히 공부해야 할 거 같고 별거 아닌 부분에서 에러가 생각보다 많이 터져서 이런 부분도 잘 정리해두어야 할 것 같아요:-)