-
Notifications
You must be signed in to change notification settings - Fork 0
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
[design] 가이드 찾아요페이지 퍼블리싱 #107
Conversation
Walkthrough이 풀 리퀘스트는 새로운 Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
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: 4
🧹 Outside diff range and nitpick comments (8)
src/data/travelFindGuideMockData.ts (2)
6-6
: 가격 형식의 표준화가 필요합니다.현재 가격이 문자열로 저장되어 있어 정렬이나 필터링 시 문제가 될 수 있습니다.
다음과 같이 개선하는 것을 제안드립니다:
- travelPrice: '123,000원', + priceAmount: 123000, + currency: 'KRW',Also applies to: 13-13
20-20
: 날짜 형식의 표준화가 필요합니다.현재 사용된 날짜 형식(YYYY.MM.DD)은 국제 표준이 아닙니다.
ISO 8601 형식(YYYY-MM-DD)을 사용하는 것을 권장드립니다:
- updateAt: '2024.01.05', + updateAt: '2024-01-05',src/components/myPage/MyPageSideMenu.tsx (1)
Line range hint
5-21
: 메뉴 아이템에 대한 타입 정의 추가를 제안드립니다.메뉴 아이템의 타입 안정성을 위해 인터페이스를 정의하는 것이 좋을 것 같습니다.
+interface SideMenuItem { + name: string; + to: string; +} -const sideMeunItems = [ +const sideMeunItems: SideMenuItem[] = [src/components/findGuide/FindGuideContent.tsx (3)
6-13
: 상태 업데이트 로직 최적화 필요handleEnableCard 함수의 구현에 대해 몇 가지 개선사항이 있습니다:
- 불필요한 렌더링을 방지하기 위해 이미 활성화된 카드에 대한 체크 필요
- 다른 카드들의 활성화 상태를 해제하는 로직이 필요할 수 있음
다음과 같이 수정을 제안드립니다:
const handleEnableCard = (index: number) => { setTrips((prevTrips) => { if (prevTrips[index].travelActive) return prevTrips; return prevTrips.map((trip, i) => ({ ...trip, travelActive: i === index })); }); };
32-41
: 반응형 디자인 개선 필요스타일링에서 다음 개선사항들이 필요합니다:
- 다양한 화면 크기에 대한 대응
- 그리드 레이아웃의 최소/최대 너비 설정
- 스크롤 사용성 개선
다음과 같이 수정을 제안드립니다:
const ScrollableContainer = styled.div` max-width: 100%; overflow-x: auto; -webkit-overflow-scrolling: touch; padding: 16px; `; const MyMadeTripsContainer = styled.div` display: grid; grid-template-columns: repeat(auto-fit, minmax(280px, 1fr)); gap: 40px; max-width: 1200px; margin: 0 auto; `;
1-43
: 테스트 코드 작성 필요컴포넌트의 안정성을 위해 다음 테스트 케이스들이 필요합니다:
- 컴포넌트 렌더링 테스트
- 카드 활성화 기능 테스트
- 상태 업데이트 테스트
테스트 코드 작성에 도움이 필요하시다면 말씀해 주세요.
src/components/findGuide/FindGuideCard.tsx (2)
5-10
: 인터페이스 정의를 개선하세요props 인터페이스에 다음과 같은 개선사항을 제안합니다:
- 각 속성에 대한 JSDoc 문서 추가
- 콜백 함수의 반환 타입 명시
- 필수 props와 선택적 props를 구분하는 주석 추가
+/** + * 가이드 카드 컴포넌트의 props 인터페이스 + */ interface IFindGuideCardProps { + /** 가이드 제목 */ title: string; + /** 리뷰 수 */ reviews: number; + /** 카드 비활성화 여부 */ isDisabled?: boolean; + /** 카드 활성화 시 호출되는 콜백 함수 */ onEnable?: () => void; }
119-148
: 버튼 스타일 개선이 필요합니다버튼 컴포넌트에 다음과 같은 개선사항을 제안합니다:
- 호버 효과 추가
- 트랜지션 효과 추가
- 비활성화 상태의 시각적 피드백 개선
const ManageButton = styled.button<{ isDisabled: boolean }>` background-color: #4a95f2; color: white; border: none; padding: 8px; height: 30px; border-radius: 8px; cursor: pointer; position: relative; flex-grow: 1; display: flex; justify-content: center; align-items: center; + transition: all 0.2s ease-in-out; pointer-events: ${(props) => (props.isDisabled ? 'none' : 'auto')}; + opacity: ${(props) => (props.isDisabled ? 0.5 : 1)}; + + &:hover:not(:disabled) { + background-color: #357abd; + transform: translateY(-1px); + } + + &:active:not(:disabled) { + transform: translateY(0); + } `; const DeleteButton = styled.button<{ isDisabled: boolean }>` color: #ff5757; border: 1px solid #ff5757; padding: 8px; border-radius: 8px; margin-left: 10px; height: 30px; cursor: pointer; flex-grow: 1; display: flex; justify-content: center; align-items: center; + transition: all 0.2s ease-in-out; pointer-events: ${(props) => (props.isDisabled ? 'none' : 'auto')}; + opacity: ${(props) => (props.isDisabled ? 0.5 : 1)}; + + &:hover:not(:disabled) { + background-color: #ff5757; + color: white; + transform: translateY(-1px); + } + + &:active:not(:disabled) { + transform: translateY(0); + } `;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
src/components/findGuide/FindGuideCard.tsx
(1 hunks)src/components/findGuide/FindGuideContent.tsx
(1 hunks)src/components/myPage/MyPageSideMenu.tsx
(1 hunks)src/data/travelFindGuideMockData.ts
(1 hunks)src/pages/MyFindGuide.tsx
(1 hunks)src/routes/router.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/pages/MyFindGuide.tsx
🔇 Additional comments (4)
src/components/myPage/MyPageSideMenu.tsx (1)
18-21
: 새로운 메뉴 항목이 올바르게 추가되었습니다.
기존 메뉴 구조와 일관성있게 구현되었으며, 라우팅 경로도 적절하게 설정되었습니다.
src/routes/router.tsx (2)
17-17
: 새로운 컴포넌트 import 구문이 적절히 추가되었습니다.
컴포넌트 import 구문이 알파벳 순서대로 정렬되어 있어 코드의 일관성이 유지되었습니다.
86-89
: 라우트 구성이 적절하게 추가되었습니다.
마이페이지 하위 경로에 새로운 가이드 찾기 페이지가 올바르게 구성되었습니다. 다만, 다음 사항들을 확인해 주시기 바랍니다:
- 해당 경로에 대한 접근 권한 설정이 필요한지 검토해 주세요.
- 페이지 이동 시 필요한 데이터 로딩 처리가 구현되어 있는지 확인해 주세요.
라우터 구성 검증을 위해 다음 스크립트를 실행합니다:
src/components/findGuide/FindGuideCard.tsx (1)
12-19
: 구현이 깔끔합니다!
컴포넌트 정의와 상태 관리가 적절하게 구현되어 있습니다.
export const travelFindGuideMockData = [ | ||
{ | ||
travelInfo: { | ||
travelTitle: '대한민국 고궁 투어', | ||
travelReviewCount: 23, | ||
travelPrice: '123,000원', | ||
}, | ||
}, | ||
{ | ||
travelInfo: { | ||
travelTitle: '부산 국밥 투어', | ||
travelReviewCount: 28, | ||
travelPrice: '50,000원', | ||
}, | ||
}, | ||
{ | ||
travelInfo: { | ||
travelTitle: '대한민국 고궁 투어', | ||
travelReviewCount: 23, | ||
updateAt: '2024.01.05', | ||
}, | ||
}, | ||
]; |
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.
🛠️ Refactor suggestion
데이터 구조 및 타입 안전성 개선이 필요합니다.
다음과 같은 문제점들이 발견되었습니다:
-
데이터 구조의 일관성이 부족합니다:
updateAt
이 마지막 객체에만 존재합니다- '대한민국 고궁 투어' 데이터가 중복되어 있습니다
travelInfo
래퍼가 불필요하게 중첩되어 있습니다
-
타입 정의가 누락되었습니다
다음과 같이 개선하는 것을 제안드립니다:
+interface TravelInfo {
+ travelTitle: string;
+ travelReviewCount: number;
+ travelPrice: string;
+ updateAt?: string;
+}
+
-export const travelFindGuideMockData = [
+export const travelFindGuideMockData: TravelInfo[] = [
{
- travelInfo: {
travelTitle: '대한민국 고궁 투어',
travelReviewCount: 23,
travelPrice: '123,000원',
- },
+ updateAt: '2024-01-05', // ISO 날짜 형식 사용
},
{
- travelInfo: {
travelTitle: '부산 국밥 투어',
travelReviewCount: 28,
travelPrice: '50,000원',
- },
+ updateAt: '2024-01-05',
},
- {
- travelInfo: {
- travelTitle: '대한민국 고궁 투어',
- travelReviewCount: 23,
- updateAt: '2024.01.05',
- },
- },
];
📝 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.
export const travelFindGuideMockData = [ | |
{ | |
travelInfo: { | |
travelTitle: '대한민국 고궁 투어', | |
travelReviewCount: 23, | |
travelPrice: '123,000원', | |
}, | |
}, | |
{ | |
travelInfo: { | |
travelTitle: '부산 국밥 투어', | |
travelReviewCount: 28, | |
travelPrice: '50,000원', | |
}, | |
}, | |
{ | |
travelInfo: { | |
travelTitle: '대한민국 고궁 투어', | |
travelReviewCount: 23, | |
updateAt: '2024.01.05', | |
}, | |
}, | |
]; | |
interface TravelInfo { | |
travelTitle: string; | |
travelReviewCount: number; | |
travelPrice: string; | |
updateAt?: string; | |
} | |
export const travelFindGuideMockData: TravelInfo[] = [ | |
{ | |
travelTitle: '대한민국 고궁 투어', | |
travelReviewCount: 23, | |
travelPrice: '123,000원', | |
updateAt: '2024-01-05', // ISO 날짜 형식 사용 | |
}, | |
{ | |
travelTitle: '부산 국밥 투어', | |
travelReviewCount: 28, | |
travelPrice: '50,000원', | |
updateAt: '2024-01-05', | |
}, | |
]; |
import FindGuideCard from '@/components/findGuide/FindGuideCard'; | ||
import { travelFindGuideMockData } from '@/data/travelFindGuideMockData'; | ||
import styled from '@emotion/styled'; | ||
import { useState } from 'react'; |
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.
🛠️ Refactor suggestion
타입 정의 추가 필요
컴포넌트의 안정성을 위해 다음 사항들의 타입 정의가 필요합니다:
- travelFindGuideMockData의 타입 정의
- trips 상태의 타입 정의
다음과 같이 타입을 추가해주세요:
interface TravelInfo {
travelTitle: string;
travelReviewCount: number;
}
interface Trip {
travelInfo: TravelInfo;
travelActive: boolean;
}
return ( | ||
<ScrollableContainer> | ||
<MyMadeTripsContainer> | ||
{trips.map((trip, index) => ( | ||
<FindGuideCard | ||
key={index} | ||
title={trip.travelInfo.travelTitle} | ||
reviews={trip.travelInfo.travelReviewCount} | ||
onEnable={() => handleEnableCard(index)} | ||
/> | ||
))} | ||
</MyMadeTripsContainer> | ||
</ScrollableContainer> | ||
); | ||
}; |
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.
🛠️ Refactor suggestion
접근성 및 성능 개선 필요
렌더링 부분에서 다음 개선사항들이 필요합니다:
- key prop으로 index 대신 고유 식별자 사용
- 접근성을 위한 의미있는 HTML 구조 필요
- 많은 데이터 처리를 위한 가상화 고려
다음과 같이 수정해주세요:
return (
<ScrollableContainer role="main" aria-label="여행 가이드 목록">
<MyMadeTripsContainer>
{trips.map((trip) => (
<FindGuideCard
key={trip.id} {/* 고유 ID 필요 */}
title={trip.travelInfo.travelTitle}
reviews={trip.travelInfo.travelReviewCount}
onEnable={() => handleEnableCard(trip.id)}
/>
))}
</MyMadeTripsContainer>
</ScrollableContainer>
);
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.
mockData 부분이라 일단 저렇게 처리한거고 실제 데이터들어올때 적용할게
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.
@Sonseongoh, 이해했습니다. 실제 데이터 적용 시 개선사항을 반영하실 계획이시군요. 해당 사항을 추적하기 위해 새로운 GitHub 이슈를 생성해 드릴까요?
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.
아니 괜찮아
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.
@Sonseongoh, 알겠습니다. 따로 이슈를 생성하지 않겠습니다.
return ( | ||
<TripCardContainer | ||
onMouseEnter={() => setIsHovered(true)} | ||
onMouseLeave={() => setIsHovered(false)} | ||
> | ||
<TripInfo isDisabled={isDisabled}> | ||
<TitleContainer> | ||
<Title>{title}</Title> | ||
<Comment>댓글 수:{reviews}</Comment> | ||
</TitleContainer> | ||
<Team max={7} mbtiList={['ENFP']} /> | ||
<Buttons> | ||
<ManageButtonContainer> | ||
<ManageButton isDisabled={isDisabled}>수정</ManageButton> | ||
</ManageButtonContainer> | ||
<DeleteButton isDisabled={isDisabled}>삭제</DeleteButton> | ||
</Buttons> | ||
</TripInfo> | ||
{isDisabled && ( | ||
<Overlay onClick={onEnable}> | ||
<DisabledText isHovered={isHovered}>{isHovered ? '활성화' : '비활성화'}</DisabledText> | ||
</Overlay> | ||
)} | ||
</TripCardContainer> | ||
); | ||
}; |
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.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
다음과 같은 접근성 개선사항을 제안합니다:
<TripCardContainer
+ role="article"
+ aria-disabled={isDisabled}
onMouseEnter={() => setIsHovered(true)}
onMouseLeave={() => setIsHovered(false)}
>
<TripInfo isDisabled={isDisabled}>
<TitleContainer>
- <Title>{title}</Title>
+ <Title role="heading" aria-level={3}>{title}</Title>
<Comment>댓글 수:{reviews}</Comment>
</TitleContainer>
<Team max={7} mbtiList={['ENFP']} />
<Buttons>
<ManageButtonContainer>
- <ManageButton isDisabled={isDisabled}>수정</ManageButton>
+ <ManageButton
+ isDisabled={isDisabled}
+ aria-disabled={isDisabled}
+ aria-label="가이드 수정하기"
+ >
+ 수정
+ </ManageButton>
</ManageButtonContainer>
- <DeleteButton isDisabled={isDisabled}>삭제</DeleteButton>
+ <DeleteButton
+ isDisabled={isDisabled}
+ aria-disabled={isDisabled}
+ aria-label="가이드 삭제하기"
+ >
+ 삭제
+ </DeleteButton>
</Buttons>
</TripInfo>
📝 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.
return ( | |
<TripCardContainer | |
onMouseEnter={() => setIsHovered(true)} | |
onMouseLeave={() => setIsHovered(false)} | |
> | |
<TripInfo isDisabled={isDisabled}> | |
<TitleContainer> | |
<Title>{title}</Title> | |
<Comment>댓글 수:{reviews}</Comment> | |
</TitleContainer> | |
<Team max={7} mbtiList={['ENFP']} /> | |
<Buttons> | |
<ManageButtonContainer> | |
<ManageButton isDisabled={isDisabled}>수정</ManageButton> | |
</ManageButtonContainer> | |
<DeleteButton isDisabled={isDisabled}>삭제</DeleteButton> | |
</Buttons> | |
</TripInfo> | |
{isDisabled && ( | |
<Overlay onClick={onEnable}> | |
<DisabledText isHovered={isHovered}>{isHovered ? '활성화' : '비활성화'}</DisabledText> | |
</Overlay> | |
)} | |
</TripCardContainer> | |
); | |
}; | |
return ( | |
<TripCardContainer | |
role="article" | |
aria-disabled={isDisabled} | |
onMouseEnter={() => setIsHovered(true)} | |
onMouseLeave={() => setIsHovered(false)} | |
> | |
<TripInfo isDisabled={isDisabled}> | |
<TitleContainer> | |
<Title role="heading" aria-level={3}>{title}</Title> | |
<Comment>댓글 수:{reviews}</Comment> | |
</TitleContainer> | |
<Team max={7} mbtiList={['ENFP']} /> | |
<Buttons> | |
<ManageButtonContainer> | |
<ManageButton | |
isDisabled={isDisabled} | |
aria-disabled={isDisabled} | |
aria-label="가이드 수정하기" | |
> | |
수정 | |
</ManageButton> | |
</ManageButtonContainer> | |
<DeleteButton | |
isDisabled={isDisabled} | |
aria-disabled={isDisabled} | |
aria-label="가이드 삭제하기" | |
> | |
삭제 | |
</DeleteButton> | |
</Buttons> | |
</TripInfo> | |
{isDisabled && ( | |
<Overlay onClick={onEnable}> | |
<DisabledText isHovered={isHovered}>{isHovered ? '활성화' : '비활성화'}</DisabledText> | |
</Overlay> | |
)} | |
</TripCardContainer> | |
); | |
}; |
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.
aria-label 이거쓰면 뭐가 좋은거야?
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.
aria-label
은 스크린 리더와 같은 보조 기술을 사용하는 사용자들에게 요소의 의미를 전달하기 위해 사용되는 속성입니다. 특히 버튼이나 링크에 시각적으로는 표시되지 않지만 보조 기기가 이해해야 하는 추가적인 설명이 필요할 때 유용합니다.
하지만 현재의 경우 버튼에 이미 텍스트('수정', '삭제')가 포함되어 있으므로, 스크린 리더는 이 텍스트를 인식할 수 있습니다. 따라서 이 상황에서는 aria-label
을 추가하지 않아도 접근성에 문제가 없을 수 있습니다.
제안이 혼란을 드려 죄송하며, 필요하지 않은 경우 aria-label
을 생략하셔도 괜찮습니다.
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.
components/findGuide도 components/myTravel처럼 components/myFindGuide로 수정하는게 좋을것같습니다
🚀 풀 리퀘스트 제안
가이드 찾아요페이지 퍼블리싱
📋 작업 세부 사항
가이드 찾아요페이지 퍼블리싱
🔧 변경 사항 요약
📸 스크린샷 (선택 사항)
## 📄 추가 정보
Summary by CodeRabbit
릴리스 노트
FindGuideCard
컴포넌트 추가.FindGuideContent
컴포넌트 추가.MyFindGuide
컴포넌트 추가 및 라우팅 설정.이 업데이트는 사용자 경험을 향상시키고 가이드 탐색 기능을 강화합니다.