refactor: 마이페이지 url 분리 리펙토링 #224
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (17)
src/stories/organisms/SearchTalkPickListSection.stories.tsx (1)
40-40: 정렬 속성의 타입 안전성이 개선되었습니다!문자열에서 객체 구조로의 변경은 좋은 개선입니다. 다만, 이 타입을 재사용할 수 있도록 별도로 정의하는 것을 고려해보세요.
다음과 같이 타입을 분리하는 것을 제안합니다:
export interface SortConfig { fileId: string; order: 'asc' | 'desc'; }src/components/organisms/SideBar/SideBar.tsx (2)
64-64: 에러 처리 개선이 필요합니다현재 마지막에
null을 반환하고 있는데, 이는 사용자에게 아무런 피드백을 제공하지 않습니다. 에러 바운더리나 폴백 UI를 통해 더 나은 사용자 경험을 제공하는 것이 좋겠습니다.다음과 같은 방식으로 개선할 수 있습니다:
- return null; + return ( + <div css={S.sidebarContainer(false)}> + <p>사용자 정보를 불러오는 중 문제가 발생했습니다.</p> + </div> + );
43-43: 상호작용 prop 로직을 더 명시적으로 표현하면 좋겠습니다현재 삼항 연산자를 인라인으로 사용하고 있는데, 이 로직을 더 명확하게 표현할 수 있습니다.
다음과 같이 변수로 분리하여 가독성을 높일 수 있습니다:
+ const profileInteraction = profileImageUrl ? 'custom' : 'default'; <ProfileIcon - interaction={profileImageUrl ? 'custom' : 'default'} + interaction={profileInteraction} imgUrl={profileImageUrl} size="large" />src/components/organisms/MyContentList/MyContentList.tsx (1)
9-13: 타입 유효성 검사 추가 필요인터페이스 속성들이 기본 타입으로 단순화되었지만, 일부 속성에 대한 유효성 검사가 필요해 보입니다:
commentCount와bookmarks는 음수가 될 수 없습니다title은 빈 문자열이 되면 안됩니다다음과 같은 유효성 검사 추가를 제안드립니다:
export interface MyContentItem { id: number; editedAt: string; title: string; commentCount: NonNegativeNumber; bookmarks: NonNegativeNumber; showBookmark?: boolean; bookmarked?: boolean; } type NonNegativeNumber = number & { __brand: 'NonNegativeNumber' }; function assertNonNegative(num: number): asserts num is NonNegativeNumber { if (num < 0) throw new Error('Number must be non-negative'); }src/hooks/api/search/useTalkPickResultQuery.ts (1)
16-20: 쿼리 로직 중복이 있습니다.
useGameResultQuery와useTalkPickResultQuery가 매우 유사한 패턴을 가지고 있습니다. 공통 로직을 추출하여 재사용할 수 있습니다.다음과 같은 리팩토링을 제안합니다:
// src/hooks/api/search/useSearchQuery.ts import { SortOption } from '@/types/sort'; interface SearchQueryParams { query: string; page: number; size: number; sort: SortOption; } export const createSearchQuery = <T>({ queryKey, fetchFn }: { queryKey: string; fetchFn: (query: string, page: number, size: number, sort: string) => Promise<T>; }) => { return ({ query, page, size, sort }: SearchQueryParams) => { const sortParam = `${sort.fileId},${sort.order}`; return useQuery<T>({ queryKey: [queryKey, query, page, size, sortParam], queryFn: () => fetchFn(query, page, size, sortParam), }); }; };이를 각 쿼리 훅에서 다음과 같이 사용할 수 있습니다:
export const useGameResultQuery = createSearchQuery({ queryKey: 'gameResults', fetchFn: getGameResults, }); export const useTalkPickResultQuery = createSearchQuery({ queryKey: 'talkPickResults', fetchFn: getTalkPickResults, });src/components/molecules/MyContentBox/MyContentBox.style.ts (1)
18-22: 접근성 개선이 잘 적용되었습니다키보드 포커스 스타일이 적절하게 추가되었습니다. 하지만 다른 컴포넌트들과의 일관성을 위해 outline 두께를 2px로 수정하는 것을 고려해보세요.
&:focus-visible { - outline: 1px solid ${color.BK}; + outline: 2px solid ${color.BK}; outline-offset: 1px; }src/components/organisms/InfoList/InfoList.tsx (2)
23-34: 성능 최적화가 잘 되었습니다만, 타입 안전성을 더 높일 수 있습니다.
useMemo를 사용한 최적화가 잘 되었습니다. 다만,Record타입에 대한 타입 안전성을 더 높일 수 있습니다.다음과 같이 수정해보세요:
- const groupedItems = useMemo(() => { + const groupedItems = useMemo<Record<string, InfoItem[]>>(() => {
36-38: 라우트 경로를 상수로 분리하는 것이 좋습니다.하드코딩된 라우트 경로를 상수로 분리하면 유지보수성이 향상됩니다.
다음과 같이 수정하는 것을 추천드립니다:
+ const TALKPICK_ROUTE = '/talkpick'; const handleItemClick = (id: number) => { - navigate(`/talkpick/${id}`); + navigate(`${TALKPICK_ROUTE}/${id}`); };src/components/molecules/ContentsButton/ContentsButton.style.ts (1)
61-64: 접근성 개선이 잘 되었습니다! 🎯키보드 네비게이션을 위한 focus-visible 스타일 추가는 매우 좋은 개선입니다. 다만, 아웃라인 색상을 더 눈에 띄게 만들면 좋을 것 같습니다.
다음과 같이 수정하는 것을 고려해보세요:
&:focus-visible { - outline: 1px solid ${color.BK}; + outline: 2px solid ${color.MAIN}; outline-offset: 1px; }src/components/molecules/SearchGameList/SearchGameList.tsx (1)
23-27: 게임 상세 페이지로의 네비게이션 구현이 깔끔합니다.useNavigate를 사용한 페이지 이동 구현이 적절합니다. 다만, 라우트 경로가 하드코딩되어 있어 향후 유지보수를 위해 상수로 분리하는 것을 고려해보세요.
다음과 같이 상수로 분리하는 것을 제안합니다:
+const ROUTES = { + BALANCE_GAME: (id: number) => `/balancegame/${id}`, +} as const; const handleItemClick = (gameId: number) => { - navigate(`/balancegame/${gameId}`); + navigate(ROUTES.BALANCE_GAME(gameId)); };src/stories/organisms/SearchGameListSection.stories.tsx (1)
29-34: 샘플 데이터의 다양성 개선이 필요합니다.현재 샘플 데이터가 단순한 패턴을 따르고 있습니다. 더 현실적인 테스트를 위해 다양한 케이스를 포함하는 것이 좋겠습니다.
다음과 같이 다양한 케이스를 추가하는 것을 제안합니다:
const gameListSample = [ { id: 1, title: "강아지 vs 고양이", mainTag: "동물", subTag: "반려동물", images: [SampleFirst, SampleSecond], }, { id: 2, title: "여름 vs 겨울", mainTag: "계절", subTag: "선호도", images: [SampleFirst, SampleSecond], }, // ... 더 다양한 케이스 추가 ];src/stories/atoms/ToggleGroup.stories.tsx (1)
39-39: argTypes 매핑이 개선되었습니다.
fileId를 기준으로 한 옵션 매핑이 적절합니다. 하지만order옵션도 제어할 수 있도록 하면 더 좋을 것 같습니다.argTypes: { selectedValue: { - options: toggleOneTwo.map((item) => item.value.fileId), + options: { + fileId: toggleOneTwo.map((item) => item.value.fileId), + order: ['asc', 'desc'] + }, control: { type: 'object' }, }, },src/stories/organisms/TalkPickListSection.stories.tsx (1)
61-61: 스토리북 설정이 컴포넌트 변경사항과 일치합니다.
selectedValue의 기본값이 새로운 타입 구조를 정확히 반영하고 있습니다.다만, 다양한 정렬 상태를 테스트할 수 있도록 추가 스토리를 작성하는 것을 권장드립니다.
예시:
export const SortByCreatedAt: Story = { args: { ...Default.args, selectedValue: { fileId: 'createdAt', order: 'desc' } } }; export const SortByViewsAscending: Story = { args: { ...Default.args, selectedValue: { fileId: 'views', order: 'asc' } } };src/components/organisms/MyBalanceGameList/MyBalanceGameList.tsx (2)
26-37: 그룹화 로직 최적화가 필요합니다.
editedAt을 기준으로 한 그룹화 로직이 잘 구현되어 있지만, 다음과 같은 개선사항을 제안드립니다:
- 빈
editedAt값에 대한 처리- 불필요한 객체 스프레드 연산자 사용 제거
const groupedItems = useMemo(() => { return items.reduce( (acc, { editedAt, ...rest }) => { + if (!editedAt) return acc; if (!acc[editedAt]) { acc[editedAt] = []; } - acc[editedAt].push({ editedAt, ...rest }); + acc[editedAt].push(rest); return acc; }, {} as Record<string, MyBalanceGameItem[]>, ); }, [items]);
49-73: 컴포넌트 렌더링 최적화가 필요합니다.
handleItemClick함수가 매 렌더링마다 새로 생성되어 성능에 영향을 줄 수 있습니다.- onClick={() => handleItemClick(gameId)} + onClick={handleItemClick}그리고
handleItemClick을useCallback으로 메모이제이션하는 것을 추천드립니다:const handleItemClick = useCallback((gameId: number) => { navigate(`/balancegame/${gameId}`); }, [navigate]);src/components/organisms/BalanceGameList/BalanceGameList.tsx (1)
56-59: 컴포넌트 간 상호작용이 개선되었습니다.
ToggleGroup과ContentsButton의 이벤트 핸들링이 적절히 구현되어 있습니다. 다만, 다음 사항을 고려해보시기 바랍니다:
- 에러 메시지를 alert 대신 더 사용자 친화적인 방식으로 표시
- 네비게이션 실패 시의 예외 처리 추가
Also applies to: 73-73
src/pages/TalkPickPage/TalkPickPage.tsx (1)
83-85: 댓글 목록 선택 로직이 명확해졌습니다.
fileId를 기준으로 한 조건문으로 변경되어 코드의 의도가 더 명확해졌습니다. 다만, 매직 스트링 사용을 피하기 위해 다음과 같은 개선을 제안합니다:+ const SORT_OPTIONS = { + VIEWS: 'views', + CREATED_AT: 'createdAt' + } as const; - selectedValue.fileId === 'views' ? bestComments : comments + selectedValue.fileId === SORT_OPTIONS.VIEWS ? bestComments : comments
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (43)
src/components/atoms/SelectGroup/SelectGroup.tsx(1 hunks)src/components/atoms/ToggleGroup/ToggleGroup.tsx(2 hunks)src/components/molecules/ContentsButton/ContentsButton.style.ts(1 hunks)src/components/molecules/ContentsButton/ContentsButton.tsx(2 hunks)src/components/molecules/InfoBox/InfoBox.style.ts(2 hunks)src/components/molecules/InfoBox/InfoBox.tsx(1 hunks)src/components/molecules/MyContentBox/MyContentBox.style.ts(2 hunks)src/components/molecules/MyContentBox/MyContentBox.tsx(1 hunks)src/components/molecules/OptionSelector/OptionSelector.tsx(2 hunks)src/components/molecules/SearchGameList/SearchGameList.tsx(3 hunks)src/components/organisms/BalanceGameList/BalanceGameList.tsx(3 hunks)src/components/organisms/CommentsSection/CommentsSection.tsx(2 hunks)src/components/organisms/InfoList/InfoList.tsx(1 hunks)src/components/organisms/MyBalanceGameList/MyBalanceGameList.tsx(1 hunks)src/components/organisms/MyContentList/MyContentList.tsx(1 hunks)src/components/organisms/OptionBar/OptionBar.style.ts(1 hunks)src/components/organisms/OptionBar/OptionBar.tsx(1 hunks)src/components/organisms/SearchGameListSection/SearchGameListSection.tsx(1 hunks)src/components/organisms/SearchTalkPickListSection/SearchTalkPickListSection.tsx(1 hunks)src/components/organisms/SideBar/SideBar.tsx(2 hunks)src/components/organisms/TalkPickListSection/TalkPickListSection.tsx(1 hunks)src/hooks/api/mypages/useMyInfoQuery.ts(1 hunks)src/hooks/api/mypages/useObserver.ts(2 hunks)src/hooks/api/search/useGameResultQuery.ts(1 hunks)src/hooks/api/search/useTalkPickResultQuery.ts(1 hunks)src/hooks/search/useSort.ts(1 hunks)src/pages/LandingPage/LandingPage.tsx(1 hunks)src/pages/MyPage/MyPage.tsx(5 hunks)src/pages/SearchResultsPage/SearchGamePage.tsx(1 hunks)src/pages/SearchResultsPage/SearchResultsPage.tsx(1 hunks)src/pages/SearchResultsPage/SearchTalkPickPage.tsx(1 hunks)src/pages/TalkPickPage/TalkPickPage.tsx(3 hunks)src/pages/TalkPickPlacePage/TalkPickPlacePage.tsx(2 hunks)src/stories/atoms/ToggleGroup.stories.tsx(4 hunks)src/stories/molecules/SearchGameList.stories.tsx(1 hunks)src/stories/organisms/BalanceGameList.stories.tsx(3 hunks)src/stories/organisms/CommentsSection.stories.tsx(2 hunks)src/stories/organisms/InfoList.stories.tsx(4 hunks)src/stories/organisms/SearchGameListSection.stories.tsx(2 hunks)src/stories/organisms/SearchTalkPickListSection.stories.tsx(2 hunks)src/stories/organisms/TalkPickListSection.stories.tsx(1 hunks)src/types/search.ts(1 hunks)src/utils/validateUrlParam.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/organisms/OptionBar/OptionBar.style.ts
- src/components/molecules/MyContentBox/MyContentBox.tsx
- src/hooks/api/mypages/useMyInfoQuery.ts
- src/stories/molecules/SearchGameList.stories.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/organisms/MyBalanceGameList/MyBalanceGameList.tsx (1)
Learnt from: WonJuneKim
PR: JECT-Study/PICK-O-Client#224
File: src/components/organisms/MyBalanceGameList/MyBalanceGameList.tsx:26-37
Timestamp: 2024-12-05T12:45:30.571Z
Learning: `editedAt` 필드는 이미 `YYYY.MM.DD` 형식의 문자열로 제공된다.
🪛 eslint
src/hooks/api/mypages/useObserver.ts
[error] 20-20: Expected 'undefined' and instead saw 'void'.
(no-void)
🔇 Additional comments (68)
src/hooks/api/mypages/useObserver.ts (2)
7-7: 타입 안전성 개선을 위한 좋은 변경입니다!
Promise<any> 대신 Promise<unknown>을 사용함으로써 타입 안전성이 향상되었습니다. unknown 타입은 값을 사용하기 전에 타입 체크를 강제하므로 런타임 오류를 방지하는데 도움이 됩니다.
20-20: Promise 처리 방식이 개선되었습니다.
void 연산자를 사용하여 Promise 반환값을 명시적으로 무시하는 것은 좋은 방법입니다. 이는:
- 의도적으로 Promise 결과를 무시한다는 것을 명확히 표현
- React useEffect 내에서 발생할 수 있는 unhandled Promise rejection 경고를 방지
- React 커뮤니티에서 권장되는 패턴
ESLint 경고는 이 경우 무시해도 됩니다.
🧰 Tools
🪛 eslint
[error] 20-20: Expected 'undefined' and instead saw 'void'.
(no-void)
src/stories/organisms/SearchTalkPickListSection.stories.tsx (2)
13-16: 스토리북 액션 핸들러가 적절히 구현되었습니다!
페이지 변경과 정렬 변경에 대한 액션 핸들러가 명확하게 정의되어 있어 컴포넌트의 이벤트를 효과적으로 문서화하고 있습니다.
46-58: 상태 관리와 이벤트 처리가 잘 구현되었습니다!
정렬 상태와 핸들러가 타입 안전하게 구현되어 있으며, 상태 업데이트와 콜백 호출이 적절히 이루어지고 있습니다.
src/components/organisms/SideBar/SideBar.tsx (2)
8-20: 타입 정의가 잘 구조화되어 있습니다! 👍
로딩 상태와 로드된 상태를 명확하게 구분하여 타입을 정의한 것이 좋습니다. discriminated union 패턴을 사용하여 타입 안전성을 보장하고 있습니다.
22-26: 타입 가드 구현이 깔끔합니다! 👍
isLoadedSideBarProps 타입 가드를 통해 런타임에서 타입 안전성을 보장하고 있습니다. 이전 리뷰에서 제안된 타입 단언 대신 타입 가드를 사용한 것이 매우 적절합니다.
src/components/atoms/SelectGroup/SelectGroup.tsx (2)
4-13: 타입 안전성이 크게 개선되었습니다.
제네릭 타입과 튜플 타입을 사용하여 다음과 같은 이점이 있습니다:
- 정확히 2개의 아이템만 허용하도록 강제
- 다양한 값 타입 지원
- 컴파일 타임에 타입 체크 가능
15-36: 컴포넌트 구현이 개선되었습니다.
- 접근성 속성이 적절히 추가되었습니다
- 제네릭 구현이 올바르게 적용되었습니다
String(value)를 사용하여 key 값 변환이 안전하게 처리되었습니다
src/components/molecules/OptionSelector/OptionSelector.tsx (3)
6-9: 옵션 타입이 더 명확해졌습니다.
options 타입이 { label: string; value: string }[]로 변경되어 데이터 구조가 더 명확해졌습니다.
15-17: 상태 초기화가 안전하게 처리되었습니다.
널 병합 연산자(??)를 사용하여 이전 리뷰에서 지적된 타입 안전성 문제가 해결되었습니다.
19-23: 선택된 옵션 동기화 로직이 추가되었습니다.
useEffect를 사용하여 selectedOption prop 변경 시 내부 상태가 적절히 업데이트됩니다.
src/components/organisms/OptionBar/OptionBar.tsx (3)
24-30: SelectGroup 아이템이 타입 안전하게 정의되었습니다.
튜플 타입과 제네릭을 활용하여 selectGroupItems가 타입 안전하게 구현되었습니다. 이전 리뷰에서 지적된 타입 캐스팅 문제가 해결되었습니다.
9-15: Props 인터페이스가 개선되었습니다.
상태 관리가 제거되고 제어 컴포넌트 패턴이 적용되어 컴포넌트의 예측 가능성이 향상되었습니다.
36-43: 콜백 처리가 단순화되었습니다.
타입 캐스팅 없이 콜백이 직접 전달되도록 개선되었습니다.
src/components/organisms/MyContentList/MyContentList.tsx (2)
36-38: URL 경로 상수화 필요
하드코딩된 URL 경로를 상수로 분리하면 유지보수성이 향상될 것 같습니다.
41-70: 접근성이 개선된 컴포넌트 구조 👍
시맨틱 HTML 요소와 ARIA 레이블을 적절히 사용하여 접근성이 잘 개선되었습니다.
src/hooks/search/useSort.ts (2)
3-6: 타입 안전성이 향상된 인터페이스 구현입니다.
SortOption 인터페이스가 명확하게 정의되어 있으며, order 타입을 'asc' | 'desc'로 제한한 것이 좋습니다.
8-14: 기본값 설정이 적절합니다.
views 기준 내림차순 정렬을 기본값으로 설정한 것이 일반적인 사용 사례에 부합합니다. 상태 관리도 타입 안전하게 구현되었습니다.
src/hooks/api/search/useGameResultQuery.ts (1)
16-20: 쿼리 키와 파라미터 구성이 개선되었습니다.
정렬 파라미터 구성이 명확하고, 쿼리 키에 모든 필요한 값이 포함되어 있습니다.
src/hooks/api/search/useTalkPickResultQuery.ts (1)
5-8: 동일한 인터페이스 중복 문제가 있습니다.
앞서 언급한 것처럼 SortOption 인터페이스를 공통 위치로 이동해야 합니다.
src/pages/SearchResultsPage/SearchGamePage.tsx (1)
16-19: 기본 정렬 설정이 명시적으로 추가되었습니다.
초기 정렬 설정을 조회수(views) 기준 내림차순으로 명시적으로 지정한 것은 좋은 개선사항입니다. 이는 코드의 의도를 더 명확하게 만들어줍니다.
사용자 경험 관점에서 이 기본 정렬 방식이 적절한지 확인이 필요합니다. 다음 스크립트로 다른 페이지들의 정렬 설정을 확인해보세요:
✅ Verification successful
조회수 기준 내림차순 정렬이 검색 결과 페이지들에서 일관되게 적용되어 있습니다.
검색 결과를 보여주는 모든 페이지(SearchGamePage, SearchTalkPickPage, SearchResultsPage)에서 동일한 정렬 설정(조회수 내림차순)을 사용하고 있어 일관된 사용자 경험을 제공하고 있습니다. 스토리북의 예제에서도 같은 정렬 설정이 적용되어 있어 의도된 구현임이 확인됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 다른 페이지들의 useSort 사용 패턴을 확인합니다.
# useSort 훅의 사용 패턴을 검색
ast-grep --pattern 'useSort({
$$$
})'
# 정렬 관련 상수나 설정을 검색
rg -g '*.ts' -g '*.tsx' 'sort.*views.*desc'
Length of output: 1126
src/pages/SearchResultsPage/SearchTalkPickPage.tsx (1)
16-19: 기본 정렬 설정이 적절한지 확인이 필요합니다.
useSort 훅에 조회수 기준 내림차순 정렬을 기본값으로 설정한 것은 사용자 경험 측면에서 적절해 보입니다. 다만, 다른 검색 페이지들과의 일관성을 확인해 주시기 바랍니다.
다른 검색 페이지들의 정렬 설정을 확인하기 위해 다음 스크립트를 실행해 주세요:
✅ Verification successful
정렬 설정이 일관되게 적용되어 있습니다.
검증 결과, 모든 검색 관련 페이지에서 조회수(views) 기준 내림차순(desc) 정렬이 기본값으로 일관되게 적용되어 있습니다:
SearchTalkPickPage.tsx와SearchGamePage.tsx는 동일한 설정 사용SearchResultsPage.tsx도 같은 기본값 사용 (field: 'views', order: 'desc')useSort훅 자체도 이와 동일한 기본값을 가지도록 구현됨
따라서 현재 변경사항은 검색 기능의 일관성을 잘 유지하고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 다른 검색 페이지들의 useSort 훅 사용 패턴을 확인합니다.
# useSort 훅이 사용된 모든 파일을 검색
ast-grep --pattern 'const { $_, $_ } = useSort($$$)'
# SearchPage 컴포넌트들에서 useSort 사용을 확인
rg -A 5 "useSort" "src/**/Search*Page.tsx"
Length of output: 762
Script:
#!/bin/bash
# Let's search for all Search*Page files first to understand the structure
fd "Search.*Page\.tsx$"
# Then let's check the implementation of useSort hook
fd "useSort\.(ts|tsx)$"
# Let's also check for any other components using useSort
rg "useSort" --type ts --type tsx
Length of output: 337
Script:
#!/bin/bash
# Let's check the content of other search pages and useSort implementation
cat src/pages/SearchResultPage/SearchResultPage.tsx
cat src/pages/SearchResultsPage/SearchResultsPage.tsx
cat src/hooks/search/useSort.ts
Length of output: 5601
src/components/molecules/MyContentBox/MyContentBox.style.ts (1)
41-41: LGTM!
텍스트 정렬이 적절하게 적용되었습니다.
src/components/molecules/InfoBox/InfoBox.tsx (1)
11-11: LGTM!
onClick prop이 적절하게 추가되었습니다.
src/components/molecules/InfoBox/InfoBox.style.ts (1)
41-41: LGTM!
텍스트 정렬이 적절하게 적용되었습니다.
src/components/organisms/InfoList/InfoList.tsx (2)
1-3: 타입 정의와 import 구문이 개선되었습니다! ✨
인터페이스에서 직접적인 타입을 사용하도록 변경한 것이 좋습니다. 이는 타입 안전성을 향상시키고 코드의 가독성을 높입니다.
Also applies to: 9-13
46-66: InfoBox 구현이 깔끔합니다! 👍
필요한 모든 props가 잘 전달되었고, onClick 핸들러도 적절하게 구현되었습니다.
src/pages/SearchResultsPage/SearchResultsPage.tsx (1)
11-11: 훅 임포트 구문이 적절히 추가되었습니다.
코드 구조와 임포트 순서가 잘 정리되어 있습니다.
src/types/search.ts (1)
4-4: GameItem 인터페이스에 id 필드 추가가 적절합니다!
게임 아이템의 고유 식별자로 사용될 id 필드가 추가되었습니다. 이는 특정 게임으로의 네비게이션을 가능하게 하는 PR의 목적에 부합합니다.
src/components/molecules/SearchGameList/SearchGameList.tsx (2)
8-15: GameItem 타입 정의가 개선되었습니다.
ContentsButtonProps에서 필요한 속성만을 선택적으로 가져오고, id 필드를 추가한 것이 적절합니다.
41-41: 이벤트 핸들러 연결이 적절합니다.
onClick 이벤트에서 game.id를 전달하는 방식이 타입 안전성을 보장합니다.
src/stories/organisms/SearchGameListSection.stories.tsx (2)
13-23: 스토리 설정이 개선되었습니다.
argTypes와 MemoryRouter 데코레이터 추가로 스토리북 테스트 환경이 향상되었습니다.
43-43: 정렬 설정이 명확해졌습니다.
sort 객체의 구조가 명확하게 정의되어 있어 정렬 동작을 이해하기 쉽습니다.
src/components/atoms/ToggleGroup/ToggleGroup.tsx (4)
12-15: 타입 정의가 개선되었습니다!
value 타입을 문자열에서 객체로 변경한 것은 좋은 개선입니다. 이는 다음과 같은 이점을 제공합니다:
- 타입 안전성 향상
- 정렬 필드와 방향의 명확한 구분
- 런타임 에러 방지
20-21: Props 타입이 일관되게 업데이트되었습니다.
ToggleGroupProps 인터페이스가 새로운 value 타입과 일치하도록 잘 수정되었습니다.
25-26: 기본값이 적절하게 설정되었습니다.
defaultItems의 값들이 새로운 타입 구조에 맞게 잘 업데이트되었습니다.
37-51: 구현이 새로운 타입과 잘 통합되었습니다.
컴포넌트 구현이 새로운 value 구조를 잘 반영하고 있습니다. 특히:
- 키 생성 로직이 적절히 수정됨
- 선택 상태 비교가 정확하게 구현됨
src/components/molecules/ContentsButton/ContentsButton.tsx (2)
16-16: 필수 onClick 핸들러 추가
onClick 핸들러를 필수 prop으로 추가한 것은 컴포넌트의 상호작용성을 명확히 하는 좋은 변경입니다.
32-44: 접근성이 크게 개선되었습니다!
다음과 같은 접근성 개선사항이 잘 구현되었습니다:
- role="button" 속성 추가
- tabIndex 설정
- 키보드 이벤트 처리 (Enter와 Space)
- preventDefault를 통한 적절한 이벤트 처리
특히 키보드 사용자를 위한 상호작용이 잘 고려되었습니다.
src/stories/atoms/ToggleGroup.stories.tsx (2)
12-12: 스토리 예제가 새로운 타입과 일치하도록 잘 수정되었습니다.
모든 토글 아이템의 value 속성이 새로운 객체 구조를 정확하게 반영하고 있습니다.
Also applies to: 16-16, 23-23, 27-27
60-62: 상태 관리와 기본값이 일관되게 설정되었습니다.
selectedValue 상태와 기본값들이 새로운 타입 구조에 맞게 잘 업데이트되었습니다.
Also applies to: 76-76, 78-78
src/pages/TalkPickPlacePage/TalkPickPlacePage.tsx (2)
10-16: 상태 관리 구조 개선이 확인되었습니다.
selectedValue 상태의 타입을 객체로 변경한 것은 정렬 기능의 유연성과 타입 안전성을 향상시킵니다.
36-36: 정렬 파라미터 구성 방식 검증이 필요합니다.
백엔드 API가 fileId,order 형식의 정렬 문자열을 정상적으로 처리할 수 있는지 확인이 필요합니다.
src/components/organisms/TalkPickListSection/TalkPickListSection.tsx (1)
16-19: 타입 정의가 명확하게 개선되었습니다.
정렬 값의 타입을 객체로 변경하고 order를 'asc' | 'desc'로 제한한 것은 타입 안전성을 크게 향상시킵니다.
src/components/organisms/MyBalanceGameList/MyBalanceGameList.tsx (2)
7-16: 인터페이스 타입 정의가 개선되었습니다.
타입 안전성이 향상되었으며, 특히 size 속성의 리터럴 타입 정의가 명확해졌습니다.
39-41: 네비게이션 로직이 적절히 구현되었습니다.
useNavigate 훅을 사용한 페이지 이동 처리가 깔끔하게 구현되었습니다.
src/components/organisms/SearchGameListSection/SearchGameListSection.tsx (1)
17-17: 정렬 관련 타입 정의가 개선되었습니다.
sort와 onSortChange 프로퍼티의 타입이 더 명확하게 정의되었습니다. 오름차순/내림차순을 명시적으로 처리할 수 있게 되었습니다.
Also applies to: 19-19
src/pages/LandingPage/LandingPage.tsx (3)
16-19: 상태 관리가 개선되었습니다.
selectedValue 상태의 타입이 더 명확하게 정의되어 정렬 방향을 명시적으로 처리할 수 있게 되었습니다.
24-25: 에러 처리와 로딩 상태 표시가 필요합니다.
useBestGameList와 useLatestGameList 훅의 에러 처리가 누락되어 있습니다.
27-32: 컨텐츠 메모이제이션이 적절히 구현되었습니다.
useMemo를 사용한 컨텐츠 계산 최적화가 잘 구현되었습니다. 의존성 배열도 적절히 설정되어 있습니다.
src/components/organisms/SearchTalkPickListSection/SearchTalkPickListSection.tsx (1)
16-16: 정렬 관련 타입 정의가 개선되었습니다.
기존의 문자열 타입에서 구조화된 객체 타입으로 변경함으로써 다음과 같은 이점이 있습니다:
- 정렬 방향과 대상 필드를 명확하게 구분
- 타입 안전성 향상
- 다른 컴포넌트들과의 일관성 유지
Also applies to: 18-18
src/components/organisms/BalanceGameList/BalanceGameList.tsx (2)
33-39: 게임 ID 유효성 검사가 구현되었습니다.
이전 리뷰 의견이 반영되어 게임 ID의 유효성을 검사하는 로직이 추가되었습니다.
45-50: 토글 변경 핸들러의 타입 안전성이 향상되었습니다.
handleToggleChange 함수가 명확한 타입 정의와 함께 추가되어 상태 관리의 안정성이 향상되었습니다.
src/pages/TalkPickPage/TalkPickPage.tsx (1)
22-25: 상태 관리 구조가 개선되었습니다.
selectedValue 상태가 더 명확한 객체 구조로 변경되었으며, 토글 아이템들도 이에 맞게 업데이트되었습니다.
Also applies to: 42-42, 46-46
src/stories/organisms/CommentsSection.stories.tsx (2)
64-64: 토글 아이템의 값 구조가 개선되었습니다.
값 구조를 객체로 변경함으로써 정렬 방식과 필드를 명확하게 구분할 수 있게 되었습니다.
Also applies to: 68-68
82-82: selectedValue의 타입이 일관되게 적용되었습니다.
toggleItem의 value 구조 변경에 맞춰 selectedValue도 동일한 구조로 업데이트되었습니다.
src/components/organisms/CommentsSection/CommentsSection.tsx (2)
25-28: 타입 정의가 더 명확해졌습니다.
selectedValue와 setToggleValue의 타입이 구체적으로 정의되어 타입 안전성이 향상되었습니다.
64-66: 댓글 생성 후 정렬 로직이 개선되었습니다.
views 기준 정렬에서 새 댓글 작성 시 자동으로 최신순으로 전환되도록 하는 사용자 경험이 개선되었습니다.
src/stories/organisms/BalanceGameList.stories.tsx (4)
20-23: 선택 옵션이 더 명확하게 구조화되었습니다.
selectedValue의 옵션이 객체 구조로 변경되어 정렬 기준과 방향이 명확해졌습니다.
57-77: 상태 관리가 개선되었습니다.
useState를 사용한 상태 관리와 MemoryRouter 추가로 스토리의 동작이 실제 사용 환경과 더 유사해졌습니다.
90-137: 다양한 시나리오 테스트가 가능해졌습니다.
단일 항목, 다수 항목, 빈 목록 등 다양한 상황을 테스트할 수 있는 시나리오가 추가되었습니다.
141-155: 이전의 접근성 개선사항이 유지되었습니다.
aria-label과 적절한 헤딩 레벨 사용 등 이전 리뷰의 접근성 개선사항이 잘 유지되고 있습니다.
src/stories/organisms/InfoList.stories.tsx (2)
51-103: 테스트 데이터 구조 개선 필요
이전 리뷰에서 지적된 문제가 여전히 존재합니다:
- 하드코딩된 날짜 값
- 반복되는 데이터 구조
더 나은 테스트 데이터 관리를 위해 다음과 같은 개선을 제안드립니다:
const createMockItem = (id: number, date: string, title: string, comment: string) => ({
id,
editedAt: date,
title,
prefix: '내 댓글',
commentContent: comment,
commentCount: 172,
bookmarks: 172,
});
const MOCK_DATES = {
TODAY: '2024.08.06',
YESTERDAY: '2024.08.05',
TWO_DAYS_AGO: '2024.08.04',
} as const;
const groupedData = [
{
date: MOCK_DATES.TODAY,
items: [
createMockItem(22, MOCK_DATES.TODAY, '매달 아르바이트 음료 500 대기업 VS 주4일 일급 250 칼퇴근 중소', '나는 바닥 또 닦고 운동하는게 꿈이라구^^'),
// ... 추가 아이템
],
},
// ... 추가 날짜 그룹
];105-125: 스토리 구현이 잘 되었습니다! 👍
다음과 같은 점들이 잘 구현되었습니다:
- MemoryRouter를 통한 라우팅 컨텍스트 제공
- Default와 All 스토리의 명확한 구분
- 재사용 가능한 CSS 스타일 활용
src/pages/MyPage/MyPage.tsx (3)
32-44: URL 파라미터 유효성 검증 추가 완료
validateUrlParam 함수를 사용하여 group과 option 파라미터의 유효성을 검증하는 로직이 추가되었습니다. 이전에 지적되었던 유효성 검증 부족 문제가 해결되었습니다.
101-110: 🛠️ Refactor suggestion
switch 문에서 매직 스트링 사용
selectedOption에 대한 switch 문에서 문자열 리터럴을 직접 사용하고 있습니다. 상수로 분리하여 사용하는 것이 코드의 유지보수성과 가독성을 높일 수 있습니다.
다음과 같이 상수를 정의하여 사용하는 것을 제안드립니다:
+ const OPTION_TYPES = {
+ BOOKMARKS: 'bookmarks',
+ VOTES: 'votes',
+ COMMENTS: 'comments',
+ WRITTEN: 'written',
+ } as const;
switch (selectedOption) {
- case 'bookmarks':
+ case OPTION_TYPES.BOOKMARKS:
return queries.myBookmarks.myBookmarks;
- case 'votes':
+ case OPTION_TYPES.VOTES:
return queries.myVotes.myVote;
// 나머지 case도 동일하게 적용
}101-110: 🛠️ Refactor suggestion
쿼리 결과 접근 시 속성명 일관성 확인 필요
queries 객체에서 쿼리 결과를 참조할 때 속성명이 일관되게 사용되고 있는지 확인해야 합니다. 예를 들어, queries.myBookmarks.myBookmarks 대신 queries.myBookmarks.data를 사용하는 것이 적절할 수 있습니다.
다음과 같이 속성명을 수정하는 것을 제안합니다:
case 'bookmarks':
- return queries.myBookmarks.myBookmarks;
+ return queries.myBookmarks.data;
case 'votes':
- return queries.myVotes.myVote;
+ return queries.myVotes.data;
case 'comments':
- return queries.myComments.myComments;
+ return queries.myComments.data;
case 'written':
- return queries.myWrittens.myWritten;
+ return queries.myWrittens.data;src/utils/validateUrlParam.ts (1)
1-16: 유효한 URL 파라미터 검증 함수 추가
URL 파라미터의 유효성을 검증하고 기본값을 반환하는 validateUrlParam 함수의 구현이 적절합니다. 타입스크립트 제네릭을 활용하여 타입 안정성을 높인 점이 좋습니다.
| const groupedItems = useMemo(() => { | ||
| return items.reduce<Record<string, MyContentItem[]>>( | ||
| (acc, { editedAt, ...rest }) => { | ||
| if (!acc[editedAt]) { | ||
| acc[editedAt] = []; | ||
| } | ||
| acc[editedAt].push({ editedAt, ...rest }); | ||
| return acc; | ||
| }, | ||
| {}, | ||
| ); | ||
| }, [items]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
성능 최적화 제안
현재 reduce 연산에서 매 반복마다 새로운 객체를 생성하고 있습니다. 큰 데이터셋에서는 성능 저하가 발생할 수 있습니다.
다음과 같은 최적화를 제안드립니다:
const groupedItems = useMemo(() => {
const groups: Record<string, MyContentItem[]> = {};
for (const item of items) {
const { editedAt, ...rest } = item;
if (!groups[editedAt]) {
groups[editedAt] = [];
}
groups[editedAt].push({ editedAt, ...rest });
}
return groups;
}, [items]);| interface SortOption { | ||
| fileId: string; | ||
| order: 'asc' | 'desc'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
인터페이스 중복 문제가 있습니다.
SortOption 인터페이스가 useSort.ts와 중복되어 있습니다. 타입 정의를 공통 위치로 이동하는 것이 좋습니다.
다음과 같이 타입을 분리하는 것을 제안합니다:
// src/types/sort.ts
export interface SortOption {
fileId: string;
order: 'asc' | 'desc';
}그리고 각 파일에서 import하여 사용하세요:
import { SortOption } from '@/types/sort';| <div | ||
| role="button" | ||
| css={S.infoContainer} | ||
| onClick={onClick} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| onClick(); | ||
| } | ||
| }} | ||
| tabIndex={0} | ||
| > |
There was a problem hiding this comment.
시맨틱 마크업 개선이 필요합니다
현재 구현은 div 요소에 role="button"을 추가하여 버튼 역할을 하도록 했지만, 네이티브 button 요소를 사용하는 것이 더 좋은 접근성을 제공합니다.
다음과 같이 수정을 제안합니다:
- <div
- role="button"
+ <button
+ type="button"
css={S.infoContainer}
onClick={onClick}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
onClick();
}
}}
- tabIndex={0}
>추가로 스타일 파일에 다음 속성들을 추가해주세요:
export const infoContainer = css`
/* 기존 스타일 */
border: none;
background: none;
cursor: pointer;
`;| &:focus-visible { | ||
| outline: 1px solid ${color.BK}; | ||
| outline-offset: 1px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
포커스 스타일의 일관성 개선이 필요합니다
다른 컴포넌트들과의 일관성을 위해 outline 스타일을 통일하는 것이 좋습니다.
&:focus-visible {
- outline: 1px solid ${color.BK};
+ outline: 2px solid ${color.BK};
outline-offset: 1px;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &:focus-visible { | |
| outline: 1px solid ${color.BK}; | |
| outline-offset: 1px; | |
| } | |
| &:focus-visible { | |
| outline: 2px solid ${color.BK}; | |
| outline-offset: 1px; | |
| } |
There was a problem hiding this comment.
현재 ContentsButton, InfoBox, MyContentsBox 모두
&:focus-visible {
outline: 1px solid ${color.BK};
outline-offset: 1px;
}이 스타일을 사용하고 있는데 2px를 사용하고 있는 컴포넌트가 있어?
There was a problem hiding this comment.
@WonJuneKim, 확인해보니 현재 ContentsButton, InfoBox, MyContentsBox 모두 outline: 1px solid 스타일을 사용하고 있으며, 코드베이스 내에 outline: 2px solid를 사용하는 컴포넌트는 없습니다. 혼선을 드려 죄송합니다. 기존 스타일을 유지하시면 될 것 같습니다.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 코드베이스에서 'outline: 2px solid' 사용처 검색
rg 'outline: 2px solid'
Length of output: 25
| import * as S from './SearchResultsPage.style'; | ||
|
|
||
| const SearchResultsPage = () => { | ||
| const { query, handleSearch } = useSearchQuery(); | ||
| const { selectedTag, handleTagClick } = useTagFilter('all'); | ||
| const { sort } = useSort({ field: 'views', order: 'desc' }); |
There was a problem hiding this comment.
💡 Codebase verification
정렬 기능 구현 관련 검토 의견입니다.
코드베이스 분석 결과, 다음과 같은 사항들이 확인되었습니다:
-
'views' DESC를 기본 정렬 옵션으로 사용하는 것은 일반적인 검색 결과 표시 방식으로 적절해 보입니다. 조회수 기준 내림차순 정렬은 사용자들에게 인기 있는 콘텐츠를 우선적으로 보여줄 수 있어 UX 관점에서 합리적인 선택입니다.
-
하지만 정렬 옵션이 변경될 때 페이지네이션이 초기화되지 않는 것이 확인되었습니다.
useSort훅의handleSortChange함수에서 페이지 상태를 리셋하는 로직이 없어, 사용자가 정렬 옵션을 변경할 때 예상치 못한 결과를 볼 수 있습니다.
src/hooks/search/useSort.ts에서 정렬 변경 시 페이지 초기화 로직 추가가 필요합니다.- 또는 상위 컴포넌트에서 정렬 변경 시 페이지네이션 리셋을 처리하는 방안을 고려해볼 수 있습니다.
🔗 Analysis chain
정렬 로직 개선이 잘 이루어졌습니다만, 몇 가지 고려사항이 있습니다.
정적인 'all' 값에서 동적인 정렬 방식으로 개선된 것은 좋은 변화입니다. 하지만 다음 사항들을 검토해 주시기 바랍니다:
- 초기 정렬 옵션이 'views' DESC로 고정되어 있는데, 이것이 사용자 경험 관점에서 최적의 기본값인지 검토가 필요합니다.
- 정렬 옵션 변경 시 페이지네이션이 초기화되는지 확인이 필요합니다.
다음 스크립트로 정렬 관련 구현을 확인해보겠습니다:
Let me gather more information about the pagination reset logic when sort changes:
Let me try one more verification to check the pagination reset logic:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 정렬 관련 구현 확인
# useSort 훅의 구현 확인
ast-grep --pattern 'export const useSort = $_'
# 페이지네이션 리셋 로직 확인
rg -A 5 'useSort|usePagination'
Length of output: 5161
Script:
#!/bin/bash
# Check sort change handler implementation and pagination reset
ast-grep --pattern 'const handleSortChange = $_'
# Check if pagination is reset in sort change handler
rg -A 10 'handleSortChange.*=.*{'
Length of output: 980
Script:
#!/bin/bash
# Check sort and pagination interaction in components
rg -A 5 'handleSortChange.*=' --type typescript
# Check if there are any direct connections between sort and pagination
rg -A 5 'setPage.*0' --type typescript
Length of output: 160
Script:
#!/bin/bash
# Check sort and pagination interaction in components
rg -A 5 'handleSortChange.*='
# Check if there are any direct connections between sort and pagination
rg -A 5 'setPage'
Length of output: 2358
| @@ -1,12 +1,12 @@ | |||
| import React from 'react'; | |||
| import type { Meta, StoryObj } from '@storybook/react'; | |||
| import InfoList from '@/components/organisms/InfoList/InfoList'; | |||
| import { MemoryRouter } from 'react-router-dom'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
메타 설정의 중복 데이터 제거 필요
현재 구현에서 meta.args의 items 데이터가 groupedData와 중복되어 있습니다. 스토리북 구성을 더 효율적으로 관리하기 위해 다음과 같은 개선을 제안드립니다:
const meta = {
title: 'organisms/InfoList',
component: InfoList,
argTypes: {
items: {
control: { type: 'object' },
},
},
- args: {
- items: [
- {
- id: 1,
- editedAt: '2024.08.06',
- // ... 중복 데이터
- },
- // ... 더 많은 중복 데이터
- ],
- },
} satisfies Meta<typeof InfoList>;이렇게 하면:
- 데이터 관리가 단순화됩니다
- 유지보수가 용이해집니다
- 데이터 일관성이 보장됩니다
Also applies to: 15-45
| selectedValue?: string; | ||
| onSelect?: (value: string) => void; | ||
| export interface SelectGroupProps<T> { | ||
| items: [SelectGroupItem<T>, SelectGroupItem<T>]; |
There was a problem hiding this comment.
튜플 타입을 사용하여 고정된 개수를 사용하는 SelectGroup에 적합한 타입인거 같네요🧐
| css={[ | ||
| toggleButtonItemStyling, | ||
| idx === 0 ? firstItemRadius : secondItemRadius, | ||
| item.value === selectedValue && selectedStyling, | ||
| selectedValue && | ||
| value.fileId === selectedValue.fileId && | ||
| value.order === selectedValue.order && | ||
| selectedStyling, | ||
| ]} |
There was a problem hiding this comment.
조건이 조금 복잡한거 같은데 함수로 조건별 스타일을 관리하도록 분리하는건 어떨까요?
There was a problem hiding this comment.
9ec9e0f 에서 처리하였고, 해당 함수는 defaultItems와 같이 컴포넌트 외부에 처리했어요..!
| selectedValue?: { fileId: string; order: 'asc' | 'desc' }; | ||
| onClick?: (value: { fileId: string; order: 'asc' | 'desc' }) => void; |
There was a problem hiding this comment.
지금도 좋지만, 반복되는 공통 타입을 분리하는건 어떨까요?
| selectedValue?: { fileId: string; order: 'asc' | 'desc' }; | |
| onClick?: (value: { fileId: string; order: 'asc' | 'desc' }) => void; | |
| export type ToggleGroupValue = { | |
| fileId: string; | |
| order: 'asc' | 'desc'; | |
| }; | |
| export type ToggleGroupItem = { | |
| label: string; | |
| value: ToggleGroupValue; | |
| }; | |
| export interface ToggleGroupProps { | |
| items?: ToggleGroupItem[]; | |
| selectedValue?: ToggleGroupValue; | |
| onClick?: (value: ToggleGroupValue) => void; | |
| } |
| <div | ||
| role="button" | ||
| css={S.infoContainer} | ||
| onClick={onClick} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| onClick(); | ||
| } | ||
| }} | ||
| tabIndex={0} | ||
| > |
There was a problem hiding this comment.
리뷰 요구 사항으로 저의 의견을 말씀드리자면, button 태그로 바꾸는 것이 접근성에 대한 문제 해결하기엔 적합한거 같습니다🙂
어쨋든 세 가지의 컴포넌트들이 클릭을 통해 사용자 액션을 수행하기 때문에 의미적으로도 button 태그로 변경하는것이 좋다구 생각함다..!
There was a problem hiding this comment.
5903475 에서 처리하였습니다! 좋은 의견 감사드려요 ㅠㅠㅠ
|
|
||
| const handleItemClick = (gameId: number) => { | ||
| if (!gameId || gameId <= 0) { | ||
| alert('유효하지 않은 게임 ID입니다.'); |
There was a problem hiding this comment.
에러 메세지에 대한 상수처리해주면 좋을거 같아요!
There was a problem hiding this comment.
e6196a3 에서 처리하였습니다 ㅎㅎ
| const handleItemClick = (gameId: number) => { | ||
| if (!gameId || gameId <= 0) { | ||
| alert('유효하지 않은 게임 ID입니다.'); | ||
| return; | ||
| } | ||
| navigate(`/balancegame/${gameId}`); | ||
| }; | ||
|
|
||
| const handleLoadMore = () => { | ||
| setVisibleItems((prev) => Math.min(prev + 6, contents.length)); | ||
| }; | ||
|
|
||
| const handleToggleChange = (value: { | ||
| fileId: string; | ||
| order: 'asc' | 'desc'; | ||
| }) => { | ||
| setSelectedValue(value); | ||
| }; |
There was a problem hiding this comment.
상태 변경 함수들에 대해 useCallback을 사용해서 핸들러를 최적화해보는 건 어떨까요? 렌더링 간 참조 동일성을 유지해 불필요한 리렌더링을 방지하고 성능 최적화에 좋을 것 같습니당
There was a problem hiding this comment.
e8ac821 에서 반영하였습니다!
자식 컴포넌트에 까지 핸들러를 주입해주니 콜백 처리가 정말 알맞는군요 ㅎㅎ
| React.SetStateAction<{ fileId: string; order: 'asc' | 'desc' }> | ||
| >; |
There was a problem hiding this comment.
이렇게 상태 업데이트 함수에 대한 자세한 타입 정의를 처음 보는거 같네용👍
src/pages/MyPage/MyPage.tsx
Outdated
| const [searchParams, setSearchParams] = useSearchParams(); | ||
| const isValidOptionKey = (value: string): boolean => { | ||
| return Object.values(OptionKeys).includes(value as OptionKeys); | ||
| }; | ||
| const initialGroup = validateUrlParam( | ||
| searchParams.get('group'), | ||
| OptionKeys.TALK_PICK, | ||
| isValidOptionKey, | ||
| ); | ||
| const initialOption = | ||
| searchParams.get('option') ?? optionSets[initialGroup][0].value; | ||
| const [selectedGroup, setSelectedGroup] = useState<OptionKeys>(initialGroup); | ||
| const [selectedOption, setSelectedOption] = useState<string>(initialOption); | ||
|
|
||
| const handleGroupSelect = (group: OptionKeys) => { | ||
| const firstOption = optionSets[group][0].value; | ||
| setSelectedGroup(group); | ||
| setSelectedOption(firstOption); | ||
| setSearchParams({ group }); | ||
| }; | ||
|
|
||
| const handleOptionSelect = (option: string) => { | ||
| setSelectedOption(option); | ||
| setSearchParams({ group: selectedGroup, option }); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| const group = validateUrlParam( | ||
| searchParams.get('group'), | ||
| OptionKeys.TALK_PICK, | ||
| isValidOptionKey, | ||
| ); | ||
| const option = searchParams.get('option') ?? optionSets[group][0].value; | ||
| setSelectedGroup(group); | ||
| setSelectedOption(option); | ||
| }, [searchParams]); |
There was a problem hiding this comment.
URL 파라미터 처리 및 상태 관리를 커스텀 훅으로 분리하는건 어떨까용 가독성에 더 좋을거 같아요!
There was a problem hiding this comment.
408e594 에서 처리하였습니다! 페이지는 순수 데이터 패칭을 다루고 훅으로 url을 다루니 관심사 분리가 잘 된 거 같아요 ㅎㅎ
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (17)
src/components/molecules/ContentsButton/ContentsButton.tsx (1)
Line range hint
32-60: 버튼 내부 구조에 대한 추가 접근성 개선이 필요합니다.이미지와 텍스트에 대한 스크린 리더 지원을 개선하기 위해 다음 사항들을 고려해주세요:
- 이미지 대체 텍스트를 더 구체적으로 작성
- 버튼의 전체 목적을 설명하는 aria-label 추가
다음과 같이 수정하는 것을 제안합니다:
<button type="button" css={S.cardWrapper(size)} onClick={onClick} + aria-label={`${title} ${mainTag} ${subTag} 게임으로 이동`} {...attributes} > <div css={S.imageContainer(size)}> <div css={S.imageWrapper}> - <img src={images[0]} alt="option A" css={S.image} /> + <img src={images[0]} alt={`첫 번째 선택지 이미지`} css={S.image} /> </div> <div css={S.imageWrapper}> - <img src={images[1]} alt="option B" css={S.image} /> + <img src={images[1]} alt={`두 번째 선택지 이미지`} css={S.image} /> </div>src/components/molecules/InfoBox/InfoBox.tsx (2)
11-11: Props 문서화가 필요합니다새로 추가된
onClickprop에 대한 JSDoc 문서화를 추가하면 컴포넌트의 사용성과 유지보수성이 향상될 것 같습니다.다음과 같이 문서화를 추가해보세요:
export interface InfoBoxProps { title: string; prefix: string; commentContent: string; commentCount: number; bookmarks: number; + /** + * InfoBox 클릭 시 실행될 콜백 함수 + * @example onClick={() => navigate(`/detail/${id}`)} + */ onClick: () => void; }
Line range hint
22-32: 접근성 개선이 더 필요합니다버튼 요소로 변경한 것은 좋은 개선이지만, 스크린 리더 사용자를 위한 추가적인 접근성 개선이 필요합니다.
다음과 같은 개선사항을 제안드립니다:
<button type="button" css={S.infoContainer} onClick={onClick} + aria-label={`${title} ${prefix} ${commentContent}`} >그리고
InfoBox.style.ts에 포커스 스타일을 추가해주세요:export const infoContainer = css` /* 기존 스타일 */ &:focus-visible { outline: 2px solid #007AFF; outline-offset: 2px; border-radius: 4px; } `;src/pages/LandingPage/LandingPage.tsx (2)
18-21: 상수 사용을 고려해보세요
selectedValue의 초기값을 상수로 분리하면 코드의 재사용성과 유지보수성이 향상될 것 같습니다.다음과 같이 변경하는 것을 제안합니다:
+const INITIAL_TOGGLE_VALUE: ToggleGroupValue = { + field: 'views', + order: 'desc', +}; -const [selectedValue, setSelectedValue] = useState<ToggleGroupValue>({ - field: 'views', - order: 'desc', -}); +const [selectedValue, setSelectedValue] = useState<ToggleGroupValue>(INITIAL_TOGGLE_VALUE);
29-34: useMemo 구현을 더 간단하게 만들 수 있습니다현재 구현이 효율적이지만, 더 간단하게 작성할 수 있습니다.
다음과 같이 변경하는 것을 제안합니다:
const contents = useMemo(() => { - if (selectedValue.field === 'views') { - return bestGames || []; - } - return latestGames || []; + return selectedValue.field === 'views' ? bestGames || [] : latestGames || []; }, [selectedValue, bestGames, latestGames]);src/components/atoms/ToggleGroup/ToggleGroup.tsx (2)
16-24: isSelected 함수 개선 제안
isSelected함수에서 타입 정의를 직접 작성하는 대신ToggleGroupValue타입을 재사용하면 좋을 것 같습니다.다음과 같이 수정을 제안드립니다:
-const isSelected = ( - selectedValue: { field: string; order: 'asc' | 'desc' } | undefined, - value: { field: string; order: 'asc' | 'desc' }, +const isSelected = ( + selectedValue: ToggleGroupValue | undefined, + value: ToggleGroupValue, ): boolean => {
33-44: 배열 검증 로직 단순화 제안
defaultItems가 이미 정의되어 있고 항상 2개의 항목을 가지므로,Array.isArray(items) && items.length === 2검증은 불필요할 수 있습니다.다음과 같이 단순화하는 것을 고려해보세요:
- {Array.isArray(items) && - items.length === 2 && - items.map(({ label, value }, idx) => ( + {items.map(({ label, value }, idx) => (src/pages/TalkPickPage/TalkPickPage.tsx (2)
83-85: 조건부 렌더링 가독성 개선 제안현재 코드는 간단하지만, 비즈니스 로직의 의도가 명확하지 않을 수 있습니다. 상수나 함수로 분리하여 의도를 더 명확하게 표현하면 좋을 것 같습니다.
다음과 같은 개선을 제안드립니다:
+const isTrendView = (value: ToggleGroupValue) => value.field === 'views'; <CommentsSection talkPickId={id} talkPickWriter={talkPick?.writer ?? ''} - commentList={selectedValue.field === 'views' ? bestComments : comments} + commentList={isTrendView(selectedValue) ? bestComments : comments} ... />
22-25: 초기 상태 값 관리 개선 제안현재 초기 상태가 하드코딩되어 있어
toggleItem의 값과 동기화가 필요할 수 있습니다.toggleItem의 첫 번째 항목을 초기값으로 사용하면 더 유지보수하기 좋을 것 같습니다.다음과 같은 방식을 고려해보세요:
+const toggleItem: ToggleGroupItem[] = [ + { + label: '인기순', + value: { field: 'views', order: 'desc' }, + }, + { + label: '최신순', + value: { field: 'createdAt', order: 'desc' }, + }, +]; -const [selectedValue, setSelectedValue] = useState<ToggleGroupValue>({ - field: 'views', - order: 'desc', -}); +const [selectedValue, setSelectedValue] = useState<ToggleGroupValue>( + toggleItem[0].value +);src/components/organisms/BalanceGameList/BalanceGameList.tsx (2)
33-42: 게임 ID 검증이 적절히 구현되었습니다.유효하지 않은 게임 ID에 대한 처리가 잘 구현되어 있습니다. useCallback을 사용한 성능 최적화도 적절합니다.
하지만 에러 메시지 표시 방식을 개선할 수 있습니다:
- alert(ERROR.GAME.NOT_EXIST); + // Toast 컴포넌트를 사용하여 더 나은 사용자 경험 제공 + showToast(ERROR.GAME.NOT_EXIST);
Line range hint
59-77: 컴포넌트 렌더링 로직이 개선되었습니다.하지만 접근성 개선이 필요해 보입니다:
- <div css={S.containerStyle}> + <section css={S.containerStyle} aria-label="밸런스 게임 목록"> <div css={S.titleWrapStyle}> - <div>주제별 밸런스 게임</div> + <h2>주제별 밸런스 게임</h2>src/stories/organisms/BalanceGameList.stories.tsx (1)
91-138: 시나리오 구조가 체계적으로 구성되었습니다.다양한 케이스를 테스트할 수 있도록 시나리오가 잘 구성되어 있습니다. 하지만 시나리오 타입 정의가 필요해 보입니다:
interface Scenario { title: string; contents: GameContent[]; } const scenarios: Scenario[] = [ // ... existing scenarios ];src/pages/TalkPickPlacePage/TalkPickPlacePage.tsx (1)
34-34: 정렬 파라미터 처리 개선이 필요합니다.현재 구현은 기본적인 기능은 충족하지만, 다음과 같은 개선사항을 제안드립니다:
- selectedValue의 필드가 undefined일 경우에 대한 방어로직 추가
- 백엔드 API의 정렬 파라미터 형식과 일치하는지 검증
다음과 같이 개선해보시는 건 어떨까요?
- sort: `${selectedValue.field},${selectedValue.order}`, + sort: selectedValue?.field && selectedValue?.order + ? `${selectedValue.field},${selectedValue.order}` + : 'views,desc',src/stories/organisms/SearchGameListSection.stories.tsx (2)
13-23: 스토리북 설정이 개선되었습니다!페이지 변경과 정렬 변경에 대한 액션 추적이 추가되어 컴포넌트 동작 테스트가 용이해졌습니다. 다만, 액션 메시지에 좀 더 구체적인 정보를 포함하면 좋을 것 같습니다.
다음과 같이 액션 메시지를 더 상세하게 작성하는 것을 고려해보세요:
- onPageChange: { action: '페이지 변경' }, - onSortChange: { action: '정렬 변경' }, + onPageChange: { action: '페이지 변경 (이전: ${from} -> 현재: ${to})' }, + onSortChange: { action: '정렬 변경 (기준: ${field}, 순서: ${order})' },
스토리의 데이터 구조가 컴포넌트와 일치하지 않습니다
검증 결과 다음과 같은 불일치 사항이 발견되었습니다:
- 스토리의
gameList아이템에서 사용하는images배열이 실제 컴포넌트의GameItem타입(optionAImg,optionBImg)과 일치하지 않습니다.- 스토리의
sort구조는 올바르나,field값이 'views'로 되어있어 실제 사용되는 값인지 확인이 필요합니다.수정 필요 사항:
gameListSample데이터의 이미지 구조를optionAImg,optionBImg로 변경해야 합니다.sort.field값이 실제 사용 가능한 값인지 확인이 필요합니다.🔗 Analysis chain
Line range hint
29-45: 데이터 구조가 개선되었습니다!정렬 구조와 이미지 데이터 구조가 더 명확하게 개선되었습니다. 다만, 이 변경사항이 실제 컴포넌트와 일치하는지 확인이 필요합니다.
다음 스크립트로 컴포넌트와 스토리의 타입 일치 여부를 확인해보세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # SearchGameListSection 컴포넌트의 props 타입과 스토리의 데이터 구조가 일치하는지 확인 # 컴포넌트의 props 타입 확인 ast-grep --pattern 'interface $interface { $$$ sort: { field: string, order: string } $$$ }' # 이미지 배열 타입 확인 ast-grep --pattern 'interface $interface { $$$ images: string[] $$$ }'Length of output: 179
Script:
#!/bin/bash # 컴포넌트와 스토리 파일의 타입 정의를 확인하기 위한 검색 # SearchGameListSection 컴포넌트 파일 찾기 fd "SearchGameListSection" --type f --exclude "*.stories.*" # 컴포넌트의 전체 인터페이스 정의 확인 rg "interface.*SearchGameList.*Props" -A 10 # 게임 아이템 인터페이스 정의 확인 rg "interface.*Game.*Item" -A 10 # 정렬 관련 타입 정의 확인 rg "type.*Sort" -A 3Length of output: 4403
Script:
#!/bin/bash # ToggleGroupValue 타입과 실제 사용되는 정렬 구조 확인 # ToggleGroupValue 타입 정의 검색 rg "type.*ToggleGroupValue" -A 5 # SearchGameListSection에서 sort prop 사용 부분 확인 rg "sort[:\.]" --type ts src/components/organisms/SearchGameListSection/SearchGameListSection.tsx -A 2 -B 2 # 스토리 파일에서 사용된 이미지 import 구문 확인 rg "import.*Sample" src/stories/organisms/SearchGameListSection.stories.tsxLength of output: 693
src/hooks/mypages/useMyPageOptions.ts (2)
6-8: 타입 안전성 개선이 필요합니다
isValidOptionKey함수의 타입 안전성을 개선할 수 있습니다.다음과 같이 수정하는 것을 제안합니다:
-const isValidOptionKey = (value: string): boolean => { +const isValidOptionKey = (value: string): value is OptionKeys => { return Object.values(OptionKeys).includes(value as OptionKeys); };이렇게 하면 타입 가드로 작동하여 더 안전한 타입 추론이 가능해집니다.
55-61: 반환 타입을 명시적으로 정의해주세요반환 객체의 타입이 추론되고 있지만, 명시적인 타입 정의가 있으면 더 좋을 것 같습니다.
다음과 같이 수정하는 것을 제안합니다:
+interface MyPageOptionsReturn { + selectedGroup: OptionKeys; + selectedOption: string; + options: Array<{ value: string; label: string }>; + handleGroupSelect: (group: OptionKeys) => void; + handleOptionSelect: (option: string) => void; +} -return { +return { selectedGroup, selectedOption, options, handleGroupSelect, handleOptionSelect, -}; +} as MyPageOptionsReturn;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (30)
src/components/atoms/ToggleGroup/ToggleGroup.tsx(3 hunks)src/components/molecules/ContentsButton/ContentsButton.style.ts(2 hunks)src/components/molecules/ContentsButton/ContentsButton.tsx(4 hunks)src/components/molecules/InfoBox/InfoBox.style.ts(3 hunks)src/components/molecules/InfoBox/InfoBox.tsx(2 hunks)src/components/molecules/MyContentBox/MyContentBox.style.ts(3 hunks)src/components/molecules/MyContentBox/MyContentBox.tsx(2 hunks)src/components/organisms/BalanceGameList/BalanceGameList.tsx(3 hunks)src/components/organisms/CommentsSection/CommentsSection.tsx(2 hunks)src/components/organisms/SearchGameListSection/SearchGameListSection.tsx(1 hunks)src/components/organisms/SearchTalkPickListSection/SearchTalkPickListSection.tsx(1 hunks)src/components/organisms/TalkPickListSection/TalkPickListSection.tsx(1 hunks)src/constants/message.ts(1 hunks)src/hooks/api/search/useGameResultQuery.ts(1 hunks)src/hooks/api/search/useTalkPickResultQuery.ts(1 hunks)src/hooks/mypages/useMyPageOptions.ts(1 hunks)src/hooks/search/useSort.ts(1 hunks)src/pages/LandingPage/LandingPage.tsx(1 hunks)src/pages/MyPage/MyPage.tsx(7 hunks)src/pages/SearchResultsPage/SearchGamePage.tsx(1 hunks)src/pages/SearchResultsPage/SearchTalkPickPage.tsx(1 hunks)src/pages/TalkPickPage/TalkPickPage.tsx(4 hunks)src/pages/TalkPickPlacePage/TalkPickPlacePage.tsx(2 hunks)src/stories/atoms/ToggleGroup.stories.tsx(4 hunks)src/stories/organisms/BalanceGameList.stories.tsx(3 hunks)src/stories/organisms/CommentsSection.stories.tsx(3 hunks)src/stories/organisms/SearchGameListSection.stories.tsx(2 hunks)src/stories/organisms/SearchTalkPickListSection.stories.tsx(2 hunks)src/stories/organisms/TalkPickListSection.stories.tsx(1 hunks)src/types/toggle.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- src/components/molecules/ContentsButton/ContentsButton.style.ts
- src/components/molecules/MyContentBox/MyContentBox.tsx
- src/stories/organisms/TalkPickListSection.stories.tsx
- src/components/molecules/MyContentBox/MyContentBox.style.ts
- src/hooks/api/search/useTalkPickResultQuery.ts
- src/hooks/api/search/useGameResultQuery.ts
- src/components/molecules/InfoBox/InfoBox.style.ts
- src/pages/SearchResultsPage/SearchTalkPickPage.tsx
- src/pages/SearchResultsPage/SearchGamePage.tsx
- src/stories/atoms/ToggleGroup.stories.tsx
- src/components/organisms/SearchGameListSection/SearchGameListSection.tsx
- src/stories/organisms/CommentsSection.stories.tsx
- src/components/organisms/CommentsSection/CommentsSection.tsx
- src/components/organisms/SearchTalkPickListSection/SearchTalkPickListSection.tsx
- src/components/organisms/TalkPickListSection/TalkPickListSection.tsx
- src/stories/organisms/SearchTalkPickListSection.stories.tsx
- src/pages/MyPage/MyPage.tsx
🔇 Additional comments (18)
src/components/molecules/ContentsButton/ContentsButton.tsx (2)
7-7: 인터페이스 변경사항이 적절히 구현되었습니다.
div에서 button으로의 기본 요소 변경과 필수 onClick 속성 추가는 컴포넌트의 의미론적 구조를 개선했습니다.
Also applies to: 16-16
32-37: 접근성 개선이 성공적으로 구현되었습니다.
이전 리뷰에서 제기된 접근성 문제가 다음과 같이 해결되었습니다:
div대신 시맨틱한button요소 사용type="button"속성 추가- 키보드 접근성 자동 지원 (네이티브 button 요소 사용)
src/components/molecules/InfoBox/InfoBox.tsx (1)
19-19: 구현이 적절해 보입니다
props 구조 분해가 올바르게 구현되었습니다.
src/hooks/search/useSort.ts (4)
2-2: 타입 임포트가 적절히 추가되었습니다.
ToggleGroupValue 타입을 추가함으로써 타입 안전성이 향상되었습니다.
4-6: 기본값 설정이 명확해졌습니다.
defaultSort의 타입과 기본값이 명확하게 구조화되어 있습니다. 'views'와 'desc'를 기본값으로 설정한 것이 적절해 보입니다.
7-7: 상태 관리에 명시적 타입 지정이 추가되었습니다.
useState에 ToggleGroupValue 타입을 명시적으로 지정하여 타입 안전성이 개선되었습니다.
9-9: 이벤트 핸들러의 매개변수 타입이 개선되었습니다.
handleSortChange 함수의 매개변수가 ToggleGroupValue 타입으로 변경되어 타입 안전성이 향상되었습니다.
다만, 다음 사항을 확인해 주시기 바랍니다:
✅ Verification successful
Let me check the actual usage of useSort in those files to verify if they're using the correct type.
useSort 훅의 사용이 모든 컴포넌트에서 올바르게 업데이트되었습니다.
검증 결과, 모든 컴포넌트(SearchTalkPickPage.tsx, SearchResultsPage.tsx, SearchGamePage.tsx)에서 useSort 훅을 객체 형태의 파라미터({ field: 'views', order: 'desc' })로 올바르게 사용하고 있으며, 반환된 handleSortChange 함수도 정상적으로 사용되고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 이 훅을 사용하는 모든 컴포넌트들이 새로운 타입에 맞게 업데이트되었는지 확인
# 이전 문자열 타입 사용 검색
rg -l "useSort\(['\"]"
# 새로운 객체 타입 사용 검색
rg -l "useSort\(\{"
Length of output: 196
Script:
#!/bin/bash
# Check the actual usage of useSort in the files
rg "useSort" src/pages/SearchResultsPage/SearchTalkPickPage.tsx src/pages/SearchResultsPage/SearchResultsPage.tsx src/pages/SearchResultsPage/SearchGamePage.tsx -A 2 -B 2
Length of output: 3124
src/pages/LandingPage/LandingPage.tsx (2)
1-1: 코드 구조가 개선되었습니다!
useEffect를 제거하고 useMemo를 사용하는 것은 좋은 선택입니다. 또한 ToggleGroupValue 타입을 추가하여 타입 안전성이 향상되었습니다.
Also applies to: 11-11
26-27:
에러 처리와 로딩 상태 처리가 여전히 필요합니다
bestGames와 latestGames를 가져오는 과정에서 에러 처리와 로딩 상태 처리가 누락되어 있습니다.
다음과 같은 개선이 필요합니다:
const { bestGames, isLoading: isBestLoading, error: bestError } = useBestGameList(activeTab);
const { latestGames, isLoading: isLatestLoading, error: latestError } = useLatestGameList(activeTab);
// 컴포넌트에서 로딩 상태 처리
{(isBestLoading || isLatestLoading) && <LoadingSpinner />}
// 에러 상태 처리
{(bestError || latestError) && <ErrorMessage />}src/types/toggle.ts (1)
1-15: 타입 정의가 잘 구현되었습니다!
타입 안전성이 향상되었고, 이전 리뷰 피드백이 잘 반영되었습니다. 특히 다음 사항들이 좋습니다:
ToggleGroupValue를 별도 타입으로 분리- order 타입을 'asc' | 'desc'로 명확하게 제한
- 선택적 프로퍼티들의 적절한 사용
src/components/organisms/BalanceGameList/BalanceGameList.tsx (2)
8-10: 타입 정의와 의존성이 개선되었습니다.
타입 안전성이 향상되었으며, 에러 처리를 위한 상수가 적절히 추가되었습니다.
Also applies to: 15-16
48-53: 상태 업데이트 로직이 최적화되었습니다.
useCallback을 사용하여 불필요한 리렌더링을 방지하고 있습니다.
src/constants/message.ts (1)
45-47: 에러 메시지 상수가 적절히 추가되었습니다.
기존 상수 구조를 따르고 있으며, 메시지가 명확합니다.
src/stories/organisms/BalanceGameList.stories.tsx (2)
21-24: 선택 옵션이 타입 안전하게 정의되었습니다.
옵션 구조가 명확하고 타입 안전성이 보장됩니다.
142-156: 접근성이 적절히 개선되었습니다.
시맨틱 마크업과 ARIA 레이블이 잘 적용되어 있습니다.
src/pages/TalkPickPlacePage/TalkPickPlacePage.tsx (2)
7-7: 타입 임포트가 적절히 추가되었습니다!
ToggleGroupValue 타입의 임포트가 깔끔하게 처리되었습니다.
11-14: 상태 타입 변경에 따른 영향도 확인이 필요합니다.
상태 관리가 문자열에서 객체로 변경되었습니다. 이는 타입 안전성을 향상시키는 좋은 변경이지만, 다음 사항들의 확인이 필요합니다:
- TalkPickListSection 컴포넌트가 새로운 타입을 올바르게 처리하는지
- 다른 페이지나 컴포넌트에서 유사한 정렬 로직 사용 시 동일한 패턴을 적용해야 하는지
다음 스크립트로 코드베이스 전체의 영향도를 확인해보겠습니다:
✅ Verification successful
Let me generate additional verification scripts to check the usage of ToggleGroupValue and related sorting logic:
상태 타입 변경은 안전하게 처리되어 있습니다.
확인 결과, 다음과 같은 사항들이 검증되었습니다:
- ToggleGroupValue 타입은 전역적으로 정의되어 있으며, 일관된 방식으로 사용되고 있습니다.
- useSort 훅을 통해 정렬 로직이 중앙 집중화되어 있습니다.
- TalkPickListSection과 SearchTalkPickListSection 컴포넌트 모두 동일한 ToggleGroupValue 타입과 정렬 패턴을 사용하고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 유사한 정렬 로직을 사용하는 다른 컴포넌트들을 확인합니다.
# ToggleGroupValue 타입 사용처 확인
rg -t typescript "ToggleGroupValue"
# 정렬 관련 상태 관리 패턴 확인
ast-grep --pattern 'useState<$_>($$$)'
Length of output: 18573
Script:
#!/bin/bash
# Check ToggleGroupValue type definition and its usage in sorting logic
ast-grep --pattern 'type ToggleGroupValue = $_'
# Check TalkPickListSection component implementation
fd -e tsx -e ts "TalkPickListSection" --exec cat {}
# Check sort-related hooks
fd -e ts "useSort" --exec cat {}
Length of output: 11741
src/stories/organisms/SearchGameListSection.stories.tsx (1)
1-6: 라우팅 설정이 적절히 추가되었습니다!
MemoryRouter를 추가한 것은 스토리북 환경에서 라우팅 기능을 테스트하기 위한 좋은 접근 방식입니다.
| const initialOption = | ||
| searchParams.get('option') ?? optionSets[initialGroup][0].value; |
There was a problem hiding this comment.
잠재적인 런타임 오류 방지가 필요합니다
optionSets[initialGroup][0]에 접근할 때 optionSets[initialGroup]이 빈 배열인 경우 런타임 오류가 발생할 수 있습니다.
다음과 같이 안전하게 수정하는 것을 제안합니다:
-const initialOption =
- searchParams.get('option') ?? optionSets[initialGroup][0].value;
+const initialOption =
+ searchParams.get('option') ??
+ (optionSets[initialGroup]?.[0]?.value ?? OptionKeys.TALK_PICK);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const initialOption = | |
| searchParams.get('option') ?? optionSets[initialGroup][0].value; | |
| const initialOption = | |
| searchParams.get('option') ?? | |
| (optionSets[initialGroup]?.[0]?.value ?? OptionKeys.TALK_PICK); |
| const handleGroupSelect = useCallback( | ||
| (group: OptionKeys) => { | ||
| const firstOption = optionSets[group][0].value; | ||
| setSelectedGroup(group); | ||
| setSelectedOption(firstOption); | ||
| setSearchParams({ group }); | ||
| }, | ||
| [setSearchParams], | ||
| ); | ||
|
|
||
| const handleOptionSelect = useCallback( | ||
| (option: string) => { | ||
| setSelectedOption(option); | ||
| setSearchParams({ group: selectedGroup, option }); | ||
| }, | ||
| [selectedGroup, setSearchParams], | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 로직 추가가 필요합니다
핸들러 함수들에 유효성 검사와 에러 처리가 없습니다. 잘못된 입력이 들어올 경우 사용자에게 적절한 피드백을 제공해야 합니다.
다음과 같이 수정하는 것을 제안합니다:
const handleGroupSelect = useCallback(
(group: OptionKeys) => {
+ if (!isValidOptionKey(group)) {
+ console.error(`Invalid group: ${group}`);
+ return;
+ }
const firstOption = optionSets[group][0].value;
setSelectedGroup(group);
setSelectedOption(firstOption);
setSearchParams({ group });
},
[setSearchParams],
);
const handleOptionSelect = useCallback(
(option: string) => {
+ if (!optionSets[selectedGroup].some(opt => opt.value === option)) {
+ console.error(`Invalid option: ${option} for group: ${selectedGroup}`);
+ return;
+ }
setSelectedOption(option);
setSearchParams({ group: selectedGroup, option });
},
[selectedGroup, setSearchParams],
);Committable suggestion skipped: line range outside the PR's diff.
| useEffect(() => { | ||
| const group = validateUrlParam( | ||
| searchParams.get('group'), | ||
| OptionKeys.TALK_PICK, | ||
| isValidOptionKey, | ||
| ); | ||
| const option = searchParams.get('option') ?? optionSets[group][0].value; | ||
| setSelectedGroup(group); | ||
| setSelectedOption(option); | ||
| }, [searchParams]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
성능 최적화가 필요합니다
useEffect 내부의 로직이 초기화 로직과 중복되며, 불필요한 상태 업데이트가 발생할 수 있습니다.
다음과 같이 최적화하는 것을 제안합니다:
useEffect(() => {
const group = validateUrlParam(
searchParams.get('group'),
OptionKeys.TALK_PICK,
isValidOptionKey,
);
const option = searchParams.get('option') ?? optionSets[group]?.[0]?.value;
- setSelectedGroup(group);
- setSelectedOption(option);
+ if (group !== selectedGroup) {
+ setSelectedGroup(group);
+ }
+ if (option !== selectedOption) {
+ setSelectedOption(option);
+ }
}, [searchParams]);Committable suggestion skipped: line range outside the PR's diff.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/molecules/ActionBox/ActionBox.tsx (1)
8-16: 사용자 경험 개선을 위한 제안현재 구현에서는 다음과 같은 개선이 가능합니다:
- 페이지 이동 전에 사용자의 변경사항이 있는 경우 확인 절차 추가
- 로딩 상태 표시 기능 추가
이러한 기능을 추가하시겠습니까? 구현에 도움이 필요하시다면 말씀해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/molecules/ActionBox/ActionBox.tsx(1 hunks)src/constants/path.ts(1 hunks)
🔇 Additional comments (1)
src/components/molecules/ActionBox/ActionBox.tsx (1)
3-4: 불필요한 상태 관리 확인 필요
페이지 이동 시에도 activeButton 상태를 관리할 필요가 있는지 검토가 필요합니다. 페이지를 이동하면 컴포넌트가 언마운트되므로 상태 관리가 불필요할 수 있습니다.
Also applies to: 8-8
| @@ -15,7 +15,7 @@ export const PATH = { | |||
| }, | |||
| CHANGE: { | |||
| PASSWORD: 'change/password', | |||
| PROFILE: 'change/profile', | |||
| PROFILE: '/change/profile', | |||
There was a problem hiding this comment.
경로 형식의 일관성 검토 필요
다른 경로들과 달리 PROFILE 경로에만 선행 슬래시('/')가 추가되어 있습니다. 라우팅의 일관성을 위해 모든 경로가 동일한 형식을 따르는 것이 좋습니다.
다음과 같이 수정하는 것을 제안합니다:
- PROFILE: '/change/profile',
+ PROFILE: 'change/profile',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PROFILE: '/change/profile', | |
| PROFILE: 'change/profile', |
|
|
||
| if (buttonType === 'edit') { | ||
| navigate(PATH.CHANGE.PROFILE); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 및 타입 안전성 개선 필요
buttonType매개변수의 타입을 문자열 리터럴 유니온 타입으로 정의하여 타입 안전성을 높일 수 있습니다.- 네비게이션 실패에 대한 에러 처리가 필요합니다.
다음과 같이 수정하는 것을 제안합니다:
- const handleButtonClick = (buttonType: string) => {
+ type ButtonType = 'activity' | 'edit' | 'withdraw';
+ const handleButtonClick = (buttonType: ButtonType) => {
setActiveButton(buttonType);
if (buttonType === 'edit') {
- navigate(PATH.CHANGE.PROFILE);
+ try {
+ navigate(PATH.CHANGE.PROFILE);
+ } catch (error) {
+ console.error('페이지 이동 중 오류가 발생했습니다:', error);
+ // 에러 처리 로직 추가 (예: 토스트 메시지 표시)
+ }
}
};Committable suggestion skipped: line range outside the PR's diff.
💡 작업 내용
💡 자세한 설명
많은 일들이 있었습니다...... 💧💧
✅ 마이페이지 ToggleGroup
와 같이 fileId과 order라는 이름으로 쉼표처리되었던(ex. views,desc) 필터링 로직을 두 분류로 나누었습니다. 또한 해당 atoms 컴포넌트에서 파생되는 모든 컴포넌트에 해당 수정 사항을 반영하였습니다.
+) 지금 보니 views, createdAt이라는 속성에 fileId라는 네이밍이 그닥 적합해 보이지는 않네요...
✅ 네비게이션 처리
+) 해당 부분과 관련된
리뷰 요구 사항이 존재합니다.default.mp4
✅ 마이페이지 스켈레톤
const isQueryLoading = true;처리를 통해 로딩중인 상태로 고정해놓은 영상입니다.default.mp4
마이페이지에서 사용되는 쿼리가 로딩중 일 때 정상적으로 작동하는 것을 알 수 있습니다.
✅ 마이페이지 내부 url 처리
+) api의 마지막 엔드 포인트를 상태로 제어하는 부분은 결국 실패... 해당 부분은 대대적인 코드 수정이 필요하여 후속 작업으로 남겨놓겠습니다.....
✅ 북마크 아이콘 수정
요구 사항에 맞춰 모양과 색상이 맞는 아이콘으로 수정되었습니다.
✅ 기타
매핑 시, 쿼리를 이용한 데이터 매핑 시 구조 분해 할당을 이용하여 데이터를 표현하였습니다. (리펙토링 된 부분)
📗 참고 자료 (선택)
📢 리뷰 요구 사항
현재 카드 뉴스, 또는 박스 형태의 molecules 컴포넌트
ContentsButton,InfoBox,MyContentsBox의 경우에는 정보를 나타내는 컴포넌트 이면서, 클릭 이벤트를 전달받아 id 값과 함께 해당하는 페이지로 이동합니다.이 경우 해당 컴포넌트의 최상위 태그가 div 라면 div 에 onclick 처리를 하는 것이 굉장히 어색하다고 하네요...
현재 소나큐브에서 에러가 뜨는 것도 이 부분과 연관이 있습니다.
현재 가능한 안으로는
정도가 있는데, 1, 4 중에 가장 고민이 되네요... 해당 부분에 대한 의견을 구하고 싶습니다 :)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #211
Summary by CodeRabbit
릴리스 노트
새로운 기능
ContentsButton,InfoBox,MyContentBox등에서 사용자 상호작용을 지원합니다.SearchGameList와BalanceGameList컴포넌트에서 게임 상세 페이지로의 내비게이션 기능 추가.OptionSelector에서 옵션 구조를 개선하여 더 복잡한 데이터 표현 가능.MyPage에서 URL 검색 매개변수를 관리하는 기능 추가.useSort훅을 통해 정렬 기능 개선 및 동적 정렬 기준 적용.CommentsSection및TalkPickListSection에서 정렬 기준을 객체로 변경하여 더 명확한 데이터 구조 제공.validateUrlParam함수 추가로 URL 매개변수 검증 기능 제공.GAME상수 추가로 게임 존재하지 않을 때의 오류 메시지 제공.ToggleGroup관련 타입 및 인터페이스 추가로 정렬 및 토글 기능 개선.버그 수정
SelectGroup의 아이템 구조를 튜플로 변경하여 안정성을 높임.LandingPage에서 상태 관리 개선으로 성능 최적화.TalkPickPage및TalkPickPlacePage에서 정렬 기능의 구조적 개선.문서화
tags속성 제거 및argTypes업데이트로 더 나은 문서화 제공.스타일
SelectGroup및OptionBar의 스타일 수정으로 일관성 있는 UI 제공.InfoBox,MyContentBox의 포커스 스타일 개선으로 접근성 향상.