Skip to content

[클린코드 7기 이상혁] 점심 뭐 먹지 미션 STEP 3 #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: sanghyuk-2i
Choose a base branch
from

Conversation

sanghyuk-2i
Copy link

@sanghyuk-2i sanghyuk-2i commented Mar 6, 2025

안녕하세요!!
영화 리뷰 미션과 병행하면서 진행하니까 좀 늦어지고 있는데..
리뷰 잘 부탁드리겠습니다! 🙏

해결하려는 문제 & 해결 방법

기능 요구사항들을 모두 수행해야 한다.

  • 자주 가는 음식점을 추가하고 목록으로 확인할 수 있다.
    • 음식점 목록에서 자주 가는 음식점을 추가할 수 있다.
    • 음식점 상세 정보에서 자주 가는 음식점으로 추가할 수 있다.
    • 자주 가는 음식점 탭에서 추가한 음식점 목록을 확인할 수 있다.

고민한 부분

Tabs을 공통 컴포넌트로 만들어서 재사용성을 높이려고 노력했습니다.

리뷰 요청 사항 & 질문

  1. 상태 관리 관련 내용
export const restaurantStore = createObserver(
  {
    // Domains
    category: sessionStorageUtil.get(
      'category',
      RESTAURANT_CATEGORIES[0].value,
    ),
    ...

    // Tabs
    activeTab: sessionStorageUtil.get('activeTab', 'ALL_TAB'),
    ...
);

이렇게 식당 관련 Store에 탭 관련 상태값을 넣었는데, 나중에는 탭 전용 Store를 만들어서 따로 관리하는게 맞을까요?

구현 예시

배포 사이트

- Tab 컴포넌트 생성
- 음식점 상세 정보에 즐겨찾기 여부 표시 추가
- Tabs 이벤트 추가 및 activeTab 상태 추가
- Tab 상태값에 따른 식당 목록 필터화 처리
- 요구사항 확인 및 체크
- 자주 가는 음식점 기능 동작 테스트를 위한 E2E 테스트 코드 추가
@igy95 igy95 self-requested a review March 8, 2025 07:46
@igy95
Copy link

igy95 commented Mar 8, 2025

이렇게 식당 관련 Store에 탭 관련 상태값을 넣었는데, 나중에는 탭 전용 Store를 만들어서 따로 관리하는게 맞을까요?

  1. 전역 상태와 지역 상태를 가름하는 기준은 무엇이라고 생각하시나요?
  2. store에 넣어야 되는 상태와 넣지 않아야하는 상태는 어떻게 나눌 수 있다고 생각하시나요?
  3. 탭 전용 store를 따로 만들어야겠다는 생각이 든 이유는 무엇인가요?

Copy link

@igy95 igy95 left a comment

Choose a reason for hiding this comment

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

안녕하세요 상혁님 ~ 3단계는 diff가 많지 않아 코멘트 달 게 별로 없어보이네요! 다만 상태의 성격을 어떻게 구분해야할지 아직 감이 잘오질 않으시는 듯 하여 이 부분에 대해 논의 이어가보면 좋을 것 같습니다 :)

Copy link

Choose a reason for hiding this comment

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

컴포넌트에 스타일을 모두 inline으로 작성하신 이유가 궁금하네요 !

Comment on lines +11 to +26
checked
? Icon({
name: 'favorite-icon-filled',
size: 'md',
removeBackground: true,
})
: ''
}
${
!checked
? Icon({
name: 'favorite-icon-lined',
size: 'md',
removeBackground: true,
})
: ''
Copy link

Choose a reason for hiding this comment

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

삼항 연산자 하나로 처리할 수 있지 않을까요?

const json = JSON.stringify(props);

const isChecked = favorites.some((favoriteId) => favoriteId === id);
Copy link

Choose a reason for hiding this comment

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

isCheckedrestaurantStore에서 제공할 수 있지 않을까요? 이걸 다른 곳에서도 쓰는 듯 보여서요.

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