Skip to content

[Step2] cactus - Props와 State - #4

Open
ehlung wants to merge 20 commits into
cactus-step1from
cactus-step2
Open

[Step2] cactus - Props와 State#4
ehlung wants to merge 20 commits into
cactus-step1from
cactus-step2

Conversation

@ehlung

@ehlung ehlung commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

개인 목표 달성 여부

  • Props의 개념과 데이터 흐름 이해
  • State와 useState 사용법 이해
  • 목록 렌더링과 Key의 역할 이해

리뷰어에게

  • filteredRestaurantsApp.jsx에서 직접 계산하여 내려주는 방식으로 구현했는데, 이 로직을 별도의 함수나 커스텀 Hook으로 분리하는 것이 더 나은지 의견을 듣고 싶습니다.
  • 카테고리별 이미지를 컴포넌트 외부에 객체(categoryImages)로 선언하여 관리했는데, 더 적절한 방식이 있는지 궁금합니다.

@ehlung
ehlung requested a review from meteorqz6 June 3, 2026 10:29
@ehlung
ehlung changed the base branch from main to step2 June 3, 2026 10:32
@ehlung
ehlung changed the base branch from step2 to cactus-step1 June 3, 2026 10:33

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[제안]
상수여서 src/constants/restaurants.js 가 더 적절할 것 같습니다!

Comment thread src/RestaurantList.jsx Outdated
Comment on lines +3 to +10
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",
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[제안]
이 부분도 따로 src/constants/categoryImages.js 로 분리하면 어떨까요?

Comment thread src/App.jsx Outdated
<main>
<CategoryFilter />
<RestaurantList />
<CategoryFilter category={category} onChangeCategory={setCategory} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[논의]
CategoryFilter한테 setCategory를 넘겨주는 것이 제가 구현한 방식과 차이가 있습니다. 저는 이벤트 핸들러를 App.jsx에 위치시켜서 이벤트 핸들러 자체를 CategortFilter로 넘겨줬습니다. 이 두 가지 방식에 대해서 같이 논의해보면 좋을 것 같아요!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

미션 요구사항 예시가 세터 함수를 직접 넘기는 방식으로 돼 있어서 처음엔 별 고민 없이 그대로 구현했는데, 유성님 코드 보면서 다시 생각해보게 됐습니다. setState를 직접 넘기면 자식이 부모 상태를 직접 제어하는 형태가 돼서, 나중에 카테고리 변경 시 side effect가 생기면 처리할 곳이 애매해질 것 같더라고요. 저도 유성님 방식이 맞다고 생각해서 App.jsx에 핸들러를 두는 방식으로 수정했습니다. 핸들러를 부모에 두면 로직 확장도 쉽고 책임 분리도 명확해지는 것 같습니다!

ehlung and others added 10 commits June 8, 2026 06:08
- 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>
@ehlung

ehlung commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

두 가지 모두 좋은 제안 감사합니다! 말씀해주신 것처럼 restaurants도 데이터라기보단 상수에 가깝다고 생각해서 src/constants/로 이동했고, categoryImages도 같은 이유로 src/constants/categoryImages.js로 분리했습니다.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d6ee957b-4c2d-4f13-b9a5-b8a4df80c0b0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cactus-step2

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread src/constants/categoryImages.js Outdated
Comment thread src/constants/restaurants.js Outdated
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.

2 participants