[Step2] cactus - Props와 State - #4
Conversation
There was a problem hiding this comment.
[제안]
상수여서 src/constants/restaurants.js 가 더 적절할 것 같습니다!
| const categoryImages = { | ||
| 한식: "../templates/category-korean.png", | ||
| 중식: "../templates/category-chinese.png", | ||
| 일식: "../templates/category-japanese.png", | ||
| 양식: "../templates/category-western.png", | ||
| 아시안: "../templates/category-asian.png", | ||
| 기타: "../templates/category-etc.png", | ||
| }; |
There was a problem hiding this comment.
[제안]
이 부분도 따로 src/constants/categoryImages.js 로 분리하면 어떨까요?
| <main> | ||
| <CategoryFilter /> | ||
| <RestaurantList /> | ||
| <CategoryFilter category={category} onChangeCategory={setCategory} /> |
There was a problem hiding this comment.
[논의]
CategoryFilter한테 setCategory를 넘겨주는 것이 제가 구현한 방식과 차이가 있습니다. 저는 이벤트 핸들러를 App.jsx에 위치시켜서 이벤트 핸들러 자체를 CategortFilter로 넘겨줬습니다. 이 두 가지 방식에 대해서 같이 논의해보면 좋을 것 같아요!
There was a problem hiding this comment.
미션 요구사항 예시가 세터 함수를 직접 넘기는 방식으로 돼 있어서 처음엔 별 고민 없이 그대로 구현했는데, 유성님 코드 보면서 다시 생각해보게 됐습니다. setState를 직접 넘기면 자식이 부모 상태를 직접 제어하는 형태가 돼서, 나중에 카테고리 변경 시 side effect가 생기면 처리할 곳이 애매해질 것 같더라고요. 저도 유성님 방식이 맞다고 생각해서 App.jsx에 핸들러를 두는 방식으로 수정했습니다. 핸들러를 부모에 두면 로직 확장도 쉽고 책임 분리도 명확해지는 것 같습니다!
- src/components/ 하위에 컴포넌트별 폴더 구성 (Header, CategoryFilter, RestaurantList, Modal) - Modal.module.css에서 폼 관련 스타일을 AddRestaurantModal.module.css로 분리 - AddRestaurantModal에서 공유/전용 CSS 모듈 각각 import Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 이미지 파일을 templates/에서 src/assets/로 이동 - src/constants/categoryImages.js로 카테고리-이미지 매핑 객체 분리 - Header, RestaurantList에서 정적 경로 대신 모듈 임포트 방식 적용 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- block__element는 점 표기법, modifier(--)는 대괄호 표기법으로 통일 - modal-backdrop/container/title → modal__backdrop/container/title - button-container → modal__button-container - restaurant-filter-container → restaurant-filter__container - restaurant-list-container → restaurant-list__container - category-icon → restaurant__category-icon - form-item .help-text → form-item__help-text - RestaurantDetailModal template literal 버그 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- setCategory 직접 전달 → handleSelectChange 핸들러 정의 후 전달 - CategoryFilter는 DOM 이벤트를 그대로 상위로 전달하도록 단순화 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- src/constants/categories.js에 CATEGORIES, FILTER_OPTIONS 상수 분리 - CategoryFilter, AddRestaurantModal option 태그 map()으로 전환 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
두 가지 모두 좋은 제안 감사합니다! 말씀해주신 것처럼 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-authored-by: Yooseong Nam <102887277+meteorqz6@users.noreply.github.com>
Co-authored-by: Yooseong Nam <102887277+meteorqz6@users.noreply.github.com>
개인 목표 달성 여부
리뷰어에게
filteredRestaurants를App.jsx에서 직접 계산하여 내려주는 방식으로 구현했는데, 이 로직을 별도의 함수나 커스텀 Hook으로 분리하는 것이 더 나은지 의견을 듣고 싶습니다.categoryImages)로 선언하여 관리했는데, 더 적절한 방식이 있는지 궁금합니다.