-
Notifications
You must be signed in to change notification settings - Fork 2
fix: tab 필터링 searchParams 기반으로 변경, 새로고침해도 필터링 유지 #243
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough이번 PR은 세 가지 주요 변경을 포함합니다. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/MainTab.tsx (1)
125-129: 상수형 데이터 분리를 고려해보세요.
SERVICE_TABS가 다른 컴포넌트나 로직에서도 재사용될 가능성이 있다면 별도의 상수 파일로 분리하면 유지보수에 유리합니다.src/components/ServiceTab.tsx (1)
42-55: 디버그 로그 확인 및 제거 여부 검토
console.log를 통한 디버그 로그가 그대로 남아 있습니다. 배포용 코드에서는 디버그 로그 정리가 필요할 수 있으므로, 유지 여부를 재검토하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Card.tsx(1 hunks)src/components/MainTab.tsx(1 hunks)src/components/ServiceTab.tsx(6 hunks)
🔇 Additional comments (10)
src/components/Card.tsx (1)
73-73: 애니메이션 전환 시간 단축, UX 개선에 적절합니다.
더 빠른 전환을 통해 사용자 경험을 향상시킬 수 있으므로 적절한 변경으로 보이며, 추가적인 기능 충돌은 없어 보입니다.src/components/MainTab.tsx (4)
6-11: Context 타입 정의가 명확합니다.
Context 형태와 필드가 직관적으로 정의되어 있어, 이후 유지보수 및 확장이 용이합니다.
15-21: useTabContext의 예외 처리 문구가 명확합니다.
직관적인 오류 메시지를 통해 컴포넌트 사용 실수를 빠르게 파악할 수 있습니다.
44-51: URL 파라미터 기반 탭 상태 동기화 로직이 적절합니다.
searchParams에서type을 읽어activeIndex를 갱신하는 방식으로 뒤로 가기 및 새로고침을 자연스럽게 처리할 수 있습니다.
74-75: URL 변경 시 버전 호환성 및 링크 공유 가능성을 확인하세요.
라우터 푸시(router.push)로 검색 파라미터를 변경하는 로직은 의도대로 동작하더라도, 새 url이 북마크나 공유에 사용될 경우를 고려해 완전한 테스트를 권장합니다.src/components/ServiceTab.tsx (5)
4-6:useSearchParams를 통한 prop 단순화가 좋습니다.
별도searchParamsprop 없이 내부에서 직접 훅을 사용하여 코드가 간결해졌습니다.
28-28: 컴포넌트 시그니처 변경 확인
ServiceTab에서searchParams파라미터를 제거하고 훅을 사용하는 구조로 변경되어, 불필요한 props가 줄어들었습니다.
30-36: 초기 상태 설정이 명확합니다.
selectedTab과selectedCategory가 URL의 파라미터를 잘 반영하도록 초기값이 설정되어 있습니다.
94-94:MainTab도입으로 코드 일관성이 향상되었습니다.
분리된 탭 컴포넌트를 재사용함으로써 유지보수 편의성이 높아졌습니다.
110-110:MainTab.Item사용으로 상태 동기화가 단순화되었습니다.
클릭 시activeIndex갱신이 자동으로 처리되어, 로직이 간결해졌습니다.
| const currentType = searchParams.get("type") || tabType; // 없으면 클릭한 탭을 기본값으로 | ||
| console.log("핸들러 실행됨11:", currentType); | ||
| setSelectedTab(currentType); | ||
|
|
||
| setSelectedTab(tabType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleTabChange에서 중복으로 setSelectedTab을 호출하는 구조를 점검하세요.
setSelectedTab(currentType);
...
setSelectedTab(tabType);최종적으로 selectedTab이 tabType으로 설정되어 currentType 할당이 무의미해질 수 있습니다. 실제 의도에 맞는지 확인해 주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/ServiceTab.tsx (1)
21-25: 🛠️ Refactor suggestion타입 정의를 업데이트해야 합니다.
searchParams프로퍼티가 더 이상 사용되지 않지만 타입 정의에 여전히 남아있습니다.type ServiceTabProps = { - searchParams: URLSearchParams; // 부모에서 받은 searchParams onCategoryChange: (type: string | undefined) => void; isFilteringLoading?: boolean; // 필터링 중인지 판단하는 변수 };
♻️ Duplicate comments (1)
src/components/ServiceTab.tsx (1)
60-64:⚠️ Potential issue
setSelectedTab중복 호출 문제
setSelectedTab이 두 번 호출되어 첫 번째 호출이 두 번째 호출에 의해 덮어쓰여집니다.// URL의 type 값을 가져와서 selectedTab 업데이트 const currentType = searchParams.get("type") || tabType; // 없으면 클릭한 탭을 기본값으로 - setSelectedTab(currentType); setSelectedTab(tabType);
🧹 Nitpick comments (2)
src/components/MainTab.tsx (2)
44-51: 기본 탭 값을 하드코딩하지 않는 것이 좋습니다.현재 "DALLAEMFIT"이 하드코딩되어 있는데, 향후 탭 항목이 변경될 경우 유지보수가 어려울 수 있습니다.
- const currentType = searchParams.get("type") || "DALLAEMFIT"; + const currentType = searchParams.get("type") || SERVICE_TABS[0].type;
74-76: 기존 URL 매개변수를 보존하는 것이 좋습니다.현재 구현은 URL을 업데이트할 때 다른 매개변수를 덮어씁니다. URLSearchParams를 사용하여 기존 매개변수를 유지하는 것이 좋습니다.
- router.push(`${pathname}?type=${tabType}`); + const params = new URLSearchParams(searchParams.toString()); + params.set('type', tabType); + router.push(`${pathname}?${params.toString()}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/MainTab.tsx(1 hunks)src/components/ServiceTab.tsx(6 hunks)
🔇 Additional comments (2)
src/components/MainTab.tsx (1)
1-129: 새로운 MainTab 컴포넌트 구현이 잘 되었습니다.URL 매개변수 기반으로 탭 상태를 관리하는 구현이 잘 되어 있습니다. React Context API를 활용하여 상태 관리를 효율적으로 처리하고 있습니다.
src/components/ServiceTab.tsx (1)
86-114: MainTab 컴포넌트의 활용이 효과적입니다.기존 Tab 컴포넌트에서 새로운 MainTab 컴포넌트로의 전환이 잘 이루어졌습니다. URL 기반 상태 관리와 함께 사용자 경험이 향상될 것입니다.
| const [selectedTab, setSelectedTab] = useState<string>( | ||
| () => SERVICE_TABS.find((t) => t.type === searchParams.get("type"))?.name || "DALLAEMFIT", | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selectedTab 상태 관리가 일관되지 않습니다.
selectedTab이 탭 이름으로 초기화되지만 코드 일부분에서는 탭 타입으로 취급됩니다. 상태 관리의 일관성을 유지해야 합니다.
const [selectedTab, setSelectedTab] = useState<string>(
- () => SERVICE_TABS.find((t) => t.type === searchParams.get("type"))?.name || "DALLAEMFIT",
+ () => SERVICE_TABS.find((t) => t.type === searchParams.get("type"))?.type || "DALLAEMFIT",
);📝 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 [selectedTab, setSelectedTab] = useState<string>( | |
| () => SERVICE_TABS.find((t) => t.type === searchParams.get("type"))?.name || "DALLAEMFIT", | |
| ); | |
| const [selectedTab, setSelectedTab] = useState<string>( | |
| () => SERVICE_TABS.find((t) => t.type === searchParams.get("type"))?.type || "DALLAEMFIT", | |
| ); |
| if (currentType) { | ||
| const tabName = SERVICE_TABS.find((t) => t.type === currentType)?.name; | ||
|
|
||
| if (tabName && tabName == selectedTab) { | ||
| setSelectedTab(tabName); | ||
|
|
||
| handleTabChange(tabName); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useEffect의 조건부 로직이 문제가 있습니다.
현재 로직은 새 탭이 현재 탭과 같을 때만 업데이트하므로, 탭이 변경될 때는 업데이트되지 않습니다. 또한 setSelectedTab과 handleTabChange를 모두 호출하는데, handleTabChange 내에서도 setSelectedTab을 호출합니다.
if (currentType) {
const tabName = SERVICE_TABS.find((t) => t.type === currentType)?.name;
- if (tabName && tabName == selectedTab) {
+ if (tabName && tabName !== selectedTab) {
setSelectedTab(tabName);
-
- handleTabChange(tabName);
}
}📝 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.
| if (currentType) { | |
| const tabName = SERVICE_TABS.find((t) => t.type === currentType)?.name; | |
| if (tabName && tabName == selectedTab) { | |
| setSelectedTab(tabName); | |
| handleTabChange(tabName); | |
| } | |
| } | |
| if (currentType) { | |
| const tabName = SERVICE_TABS.find((t) => t.type === currentType)?.name; | |
| if (tabName && tabName !== selectedTab) { | |
| setSelectedTab(tabName); | |
| } | |
| } |
|
|
Description
tab 필터링 searchParams 기반으로 변경했습니다.
새로고침해도 필터링이 유지되도록 수정하여 사용자 경험을 향상시켰습니다.
Changes Made
To Reviewers
기존 Tab 컴포넌트를 그대로 가져와 searchParam 부분을 추가하였습니다. 같은 코드가 반복되는 부분이 있지만 필요한 기능이라 판단되어 추가하게되었습니다..!
중복된 코드를 줄이는 것을 추후 작업에서 진행하겠습니다!
Screenshots (선택)
IssueNumber
[#이슈번호]
Summary by CodeRabbit
새로운 기능
스타일
리팩토링